[ftr/lifecycle] refactor to be typesafe (#52453)

* [ftr/lifecycle] refactor to be typesafe

* update test fixture

docs/developer/core/development-functional-tests.asciidoc

@@ -282,7 +282,7 @@ The `FunctionalTestRunner` comes with three built-in services:
 * Source: {blob}src/functional_test_runner/lib/lifecycle.ts[src/functional_test_runner/lib/lifecycle.ts]
 * Designed primary for use in services
 * Exposes lifecycle events for basic coordination. Handlers can return a promise and resolve/fail asynchronously
-* Phases include: `beforeLoadTests`, `beforeTests`, `beforeEachTest`, `cleanup`, `phaseStart`, `phaseEnd`
+* Phases include: `beforeLoadTests`, `beforeTests`, `beforeEachTest`, `cleanup`
 
 [float]
 ===== Kibana Services

packages/kbn-test/src/functional_test_runner/__tests__/fixtures/failure_hooks/config.js

@@ -29,18 +29,19 @@ export default function () {
     services: {
       hookIntoLIfecycle({ getService }) {
         const log = getService('log');
+        const lifecycle = getService('lifecycle')
 
-        getService('lifecycle')
-          .on('testFailure', async (err, test) => {
-            log.info('testFailure %s %s', err.message, test.fullTitle());
-            await delay(10);
-            log.info('testFailureAfterDelay %s %s', err.message, test.fullTitle());
-          })
-          .on('testHookFailure', async (err, test) => {
-            log.info('testHookFailure %s %s', err.message, test.fullTitle());
-            await delay(10);
-            log.info('testHookFailureAfterDelay %s %s', err.message, test.fullTitle());
-          });
+        lifecycle.testFailure.add(async (err, test) => {
+          log.info('testFailure %s %s', err.message, test.fullTitle());
+          await delay(10);
+          log.info('testFailureAfterDelay %s %s', err.message, test.fullTitle());
+        });
+
+        lifecycle.testHookFailure.add(async (err, test) => {
+          log.info('testHookFailure %s %s', err.message, test.fullTitle());
+          await delay(10);
+          log.info('testHookFailureAfterDelay %s %s', err.message, test.fullTitle());
+        });
       }
     },
     mochaReporter: {

packages/kbn-test/src/functional_test_runner/functional_test_runner.ts

@@ -18,10 +18,11 @@
  */
 
 import { ToolingLog } from '@kbn/dev-utils';
-import { Suite, Test } from './fake_mocha_types';
 
+import { Suite, Test } from './fake_mocha_types';
 import {
-  createLifecycle,
+  Lifecycle,
+  LifecyclePhase,
   readConfigFile,
   ProviderCollection,
   readProviderSpec,
@@ -31,7 +32,7 @@ import {
 } from './lib';
 
 export class FunctionalTestRunner {
-  public readonly lifecycle = createLifecycle();
+  public readonly lifecycle = new Lifecycle();
   private closed = false;
 
   constructor(
@@ -39,13 +40,12 @@ export class FunctionalTestRunner {
     private readonly configFile: string,
     private readonly configOverrides: any
   ) {
-    this.lifecycle.on('phaseStart', name => {
-      log.verbose('starting %j lifecycle phase', name);
-    });
-
-    this.lifecycle.on('phaseEnd', name => {
-      log.verbose('ending %j lifecycle phase', name);
-    });
+    for (const [key, value] of Object.entries(this.lifecycle)) {
+      if (value instanceof LifecyclePhase) {
+        value.before$.subscribe(() => log.verbose('starting %j lifecycle phase', key));
+        value.after$.subscribe(() => log.verbose('starting %j lifecycle phase', key));
+      }
+    }
   }
 
   async run() {
@@ -59,7 +59,7 @@ export class FunctionalTestRunner {
       await providers.loadAll();
 
       const mocha = await setupMocha(this.lifecycle, this.log, config, providers);
-      await this.lifecycle.trigger('beforeTests');
+      await this.lifecycle.beforeTests.trigger();
       this.log.info('Starting tests');
 
       return await runTests(this.lifecycle, mocha);
@@ -140,6 +140,6 @@ export class FunctionalTestRunner {
     if (this.closed) return;
 
     this.closed = true;
-    await this.lifecycle.trigger('cleanup');
+    await this.lifecycle.cleanup.trigger();
   }
 }

packages/kbn-test/src/functional_test_runner/lib/index.ts

@@ -17,7 +17,8 @@
  * under the License.
  */
 
-export { createLifecycle, Lifecycle } from './lifecycle';
+export { Lifecycle } from './lifecycle';
+export { LifecyclePhase } from './lifecycle_phase';
 export { readConfigFile, Config } from './config';
 export { readProviderSpec, ProviderCollection, Provider } from './providers';
 export { runTests, setupMocha } from './mocha';

packages/kbn-test/src/functional_test_runner/lib/lifecycle.ts

@@ -17,64 +17,22 @@
  * under the License.
  */
 
-import * as Rx from 'rxjs';
+import { LifecyclePhase } from './lifecycle_phase';
 
-type Listener = (...args: any[]) => Promise<void> | void;
-export type Lifecycle = ReturnType<typeof createLifecycle>;
+// mocha's global types mean we can't import Mocha or it will override the global jest types..............
+type ItsASuite = any;
+type ItsATest = any;
 
-export function createLifecycle() {
-  const listeners = {
-    beforeLoadTests: [] as Listener[],
-    beforeTests: [] as Listener[],
-    beforeTestSuite: [] as Listener[],
-    beforeEachTest: [] as Listener[],
-    afterTestSuite: [] as Listener[],
-    testFailure: [] as Listener[],
-    testHookFailure: [] as Listener[],
-    cleanup: [] as Listener[],
-    phaseStart: [] as Listener[],
-    phaseEnd: [] as Listener[],
-  };
-
-  const cleanup$ = new Rx.ReplaySubject<undefined>(1);
-
-  return {
-    cleanup$: cleanup$.asObservable(),
-
-    on(name: keyof typeof listeners, fn: Listener) {
-      if (!listeners[name]) {
-        throw new TypeError(`invalid lifecycle event "${name}"`);
-      }
-
-      listeners[name].push(fn);
-      return this;
-    },
-
-    async trigger(name: keyof typeof listeners, ...args: any[]) {
-      if (!listeners[name]) {
-        throw new TypeError(`invalid lifecycle event "${name}"`);
-      }
-
-      if (name === 'cleanup') {
-        if (cleanup$.closed) {
-          return;
-        }
-
-        cleanup$.next();
-        cleanup$.complete();
-      }
-
-      try {
-        if (name !== 'phaseStart' && name !== 'phaseEnd') {
-          await this.trigger('phaseStart', name);
-        }
-
-        await Promise.all(listeners[name].map(async fn => await fn(...args)));
-      } finally {
-        if (name !== 'phaseStart' && name !== 'phaseEnd') {
-          await this.trigger('phaseEnd', name);
-        }
-      }
-    },
-  };
+export class Lifecycle {
+  public readonly beforeTests = new LifecyclePhase<[]>({
+    singular: true,
+  });
+  public readonly beforeTestSuite = new LifecyclePhase<[ItsASuite]>();
+  public readonly beforeEachTest = new LifecyclePhase<[ItsATest]>();
+  public readonly afterTestSuite = new LifecyclePhase<[ItsASuite]>();
+  public readonly testFailure = new LifecyclePhase<[Error, ItsATest]>();
+  public readonly testHookFailure = new LifecyclePhase<[Error, ItsATest]>();
+  public readonly cleanup = new LifecyclePhase<[]>({
+    singular: true,
+  });
 }

packages/kbn-test/src/functional_test_runner/lib/lifecycle_phase.test.ts

@@ -0,0 +1,206 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import * as Rx from 'rxjs';
+import { materialize, toArray } from 'rxjs/operators';
+
+import { LifecyclePhase } from './lifecycle_phase';
+
+describe('with randomness', () => {
+  beforeEach(() => {
+    const randomOrder = [0, 0.75, 0.5, 0.25, 1];
+    jest.spyOn(Math, 'random').mockImplementation(() => {
+      const n = randomOrder.shift()!;
+      randomOrder.push(n);
+      return n;
+    });
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('calls handlers in random order', async () => {
+    const phase = new LifecyclePhase();
+    const order: string[] = [];
+
+    phase.add(
+      jest.fn(() => {
+        order.push('one');
+      })
+    );
+
+    phase.add(
+      jest.fn(() => {
+        order.push('two');
+      })
+    );
+
+    phase.add(
+      jest.fn(() => {
+        order.push('three');
+      })
+    );
+
+    await phase.trigger();
+    expect(order).toMatchInlineSnapshot(`
+      Array [
+        "one",
+        "three",
+        "two",
+      ]
+    `);
+  });
+});
+
+describe('without randomness', () => {
+  beforeEach(() => jest.spyOn(Math, 'random').mockImplementation(() => 0));
+  afterEach(() => jest.restoreAllMocks());
+
+  it('calls all handlers and throws first error', async () => {
+    const phase = new LifecyclePhase();
+    const fn1 = jest.fn();
+    phase.add(fn1);
+
+    const fn2 = jest.fn(() => {
+      throw new Error('foo');
+    });
+    phase.add(fn2);
+
+    const fn3 = jest.fn();
+    phase.add(fn3);
+
+    await expect(phase.trigger()).rejects.toThrowErrorMatchingInlineSnapshot(`"foo"`);
+    expect(fn1).toHaveBeenCalled();
+    expect(fn2).toHaveBeenCalled();
+    expect(fn3).toHaveBeenCalled();
+  });
+
+  it('triggers before$ just before calling handler and after$ once it resolves', async () => {
+    const phase = new LifecyclePhase();
+    const order: string[] = [];
+
+    const beforeSub = jest.fn(() => order.push('before'));
+    phase.before$.subscribe(beforeSub);
+
+    const afterSub = jest.fn(() => order.push('after'));
+    phase.after$.subscribe(afterSub);
+
+    const handler = jest.fn(async () => {
+      order.push('handler start');
+      await new Promise(resolve => setTimeout(resolve, 100));
+      order.push('handler done');
+    });
+    phase.add(handler);
+
+    await phase.trigger();
+    expect(order).toMatchInlineSnapshot(`
+      Array [
+        "before",
+        "handler start",
+        "handler done",
+        "after",
+      ]
+    `);
+  });
+
+  it('completes before$ and after$ if phase is singular', async () => {
+    const phase = new LifecyclePhase({ singular: true });
+
+    const beforeNotifs: Array<Rx.Notification<unknown>> = [];
+    phase.before$.pipe(materialize()).subscribe(n => beforeNotifs.push(n));
+
+    const afterNotifs: Array<Rx.Notification<unknown>> = [];
+    phase.after$.pipe(materialize()).subscribe(n => afterNotifs.push(n));
+
+    await phase.trigger();
+    expect(beforeNotifs).toMatchInlineSnapshot(`
+      Array [
+        Notification {
+          "error": undefined,
+          "hasValue": true,
+          "kind": "N",
+          "value": undefined,
+        },
+        Notification {
+          "error": undefined,
+          "hasValue": false,
+          "kind": "C",
+          "value": undefined,
+        },
+      ]
+    `);
+    expect(afterNotifs).toMatchInlineSnapshot(`
+      Array [
+        Notification {
+          "error": undefined,
+          "hasValue": true,
+          "kind": "N",
+          "value": undefined,
+        },
+        Notification {
+          "error": undefined,
+          "hasValue": false,
+          "kind": "C",
+          "value": undefined,
+        },
+      ]
+    `);
+  });
+
+  it('completes before$ subscribers after trigger of singular phase', async () => {
+    const phase = new LifecyclePhase({ singular: true });
+    await phase.trigger();
+
+    await expect(phase.before$.pipe(materialize(), toArray()).toPromise()).resolves
+      .toMatchInlineSnapshot(`
+            Array [
+              Notification {
+                "error": undefined,
+                "hasValue": false,
+                "kind": "C",
+                "value": undefined,
+              },
+            ]
+          `);
+  });
+
+  it('replays after$ event subscribers after trigger of singular phase', async () => {
+    const phase = new LifecyclePhase({ singular: true });
+    await phase.trigger();
+
+    await expect(phase.after$.pipe(materialize(), toArray()).toPromise()).resolves
+      .toMatchInlineSnapshot(`
+            Array [
+              Notification {
+                "error": undefined,
+                "hasValue": true,
+                "kind": "N",
+                "value": undefined,
+              },
+              Notification {
+                "error": undefined,
+                "hasValue": false,
+                "kind": "C",
+                "value": undefined,
+              },
+            ]
+          `);
+  });
+});

packages/kbn-test/src/functional_test_runner/lib/lifecycle_phase.ts

@@ -0,0 +1,82 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import * as Rx from 'rxjs';
+
+const shuffle = <T>(arr: T[]) => arr.slice().sort(() => (Math.random() > 0.5 ? 1 : -1));
+
+export type GetArgsType<T extends LifecyclePhase<any>> = T extends LifecyclePhase<infer X>
+  ? X
+  : never;
+
+export class LifecyclePhase<Args extends readonly any[]> {
+  private readonly handlers: Array<(...args: Args) => Promise<void> | void> = [];
+
+  private readonly beforeSubj = new Rx.Subject<void>();
+  public readonly before$ = this.beforeSubj.asObservable();
+
+  private readonly afterSubj = this.options.singular
+    ? new Rx.ReplaySubject<void>(1)
+    : new Rx.Subject<void>();
+  public readonly after$ = this.afterSubj.asObservable();
+
+  constructor(
+    private readonly options: {
+      singular?: boolean;
+    } = {}
+  ) {}
+
+  public add(fn: (...args: Args) => Promise<void> | void) {
+    this.handlers.push(fn);
+  }
+
+  public async trigger(...args: Args) {
+    if (this.beforeSubj.isStopped) {
+      throw new Error(`singular lifecycle event can only be triggered once`);
+    }
+
+    this.beforeSubj.next(undefined);
+    if (this.options.singular) {
+      this.beforeSubj.complete();
+    }
+
+    // catch the first error but still execute all handlers
+    let error;
+
+    // shuffle the handlers to prevent relying on their order
+    for (const fn of shuffle(this.handlers)) {
+      try {
+        await fn(...args);
+      } catch (_error) {
+        if (!error) {
+          error = _error;
+        }
+      }
+    }
+
+    this.afterSubj.next(undefined);
+    if (this.options.singular) {
+      this.afterSubj.complete();
+    }
+
+    if (error) {
+      throw error;
+    }
+  }
+}

packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js

@@ -58,7 +58,7 @@ export function decorateMochaUi(lifecycle, context) {
 
         argumentsList[1] = function() {
           before(async () => {
-            await lifecycle.trigger('beforeTestSuite', this);
+            await lifecycle.beforeTestSuite.trigger(this);
           });
 
           this.tags = tags => {
@@ -68,7 +68,7 @@ export function decorateMochaUi(lifecycle, context) {
           provider.call(this);
 
           after(async () => {
-            await lifecycle.trigger('afterTestSuite', this);
+            await lifecycle.afterTestSuite.trigger(this);
           });
         };
 
@@ -94,7 +94,7 @@ export function decorateMochaUi(lifecycle, context) {
     return wrapNonSuiteFunction(
       name,
       wrapRunnableArgsWithErrorHandler(fn, async (err, test) => {
-        await lifecycle.trigger('testFailure', err, test);
+        await lifecycle.testFailure.trigger(err, test);
       })
     );
   }
@@ -112,7 +112,7 @@ export function decorateMochaUi(lifecycle, context) {
     return wrapNonSuiteFunction(
       name,
       wrapRunnableArgsWithErrorHandler(fn, async (err, test) => {
-        await lifecycle.trigger('testHookFailure', err, test);
+        await lifecycle.testHookFailure.trigger(err, test);
       })
     );
   }

packages/kbn-test/src/functional_test_runner/lib/mocha/run_tests.ts

@@ -35,7 +35,7 @@ export async function runTests(lifecycle: Lifecycle, mocha: Mocha) {
     runComplete = true;
   });
 
-  lifecycle.on('cleanup', () => {
+  lifecycle.cleanup.add(() => {
     if (!runComplete) runner.abort();
   });
 

packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js

@@ -41,7 +41,7 @@ export async function setupMocha(lifecycle, log, config, providers) {
 
   // global beforeEach hook in root suite triggers before all others
   mocha.suite.beforeEach('global before each', async function() {
-    await lifecycle.trigger('beforeEachTest', this.currentTest);
+    await lifecycle.beforeEachTest.trigger(this.currentTest);
   });
 
   loadTestFiles({

test/common/services/kibana_server/kibana_server.ts

@@ -32,7 +32,7 @@ export function KibanaServerProvider({ getService }: FtrProviderContext) {
   const kbn = new KbnClient(log, [url], defaults);
 
   if (defaults) {
-    lifecycle.on('beforeTests', async () => {
+    lifecycle.beforeTests.add(async () => {
       await kbn.uiSettings.update(defaults);
     });
   }

test/functional/services/failure_debugging.ts

@@ -65,5 +65,6 @@ export async function FailureDebuggingProvider({ getService }: FtrProviderContex
     await Promise.all([screenshots.takeForFailure(name), logCurrentUrl(), savePageHtml(name)]);
   }
 
-  lifecycle.on('testFailure', onFailure).on('testHookFailure', onFailure);
+  lifecycle.testFailure.add(onFailure);
+  lifecycle.testHookFailure.add(onFailure);
 }

test/functional/services/remote/poll_for_log_entry.ts

@@ -29,7 +29,7 @@ export function pollForLogEntry$(
   driver: WebDriver,
   type: string,
   ms: number,
-  stop$: Rx.Observable<undefined>
+  stop$: Rx.Observable<void>
 ) {
   const logCtrl = driver.manage().logs();
   const poll$ = new Rx.BehaviorSubject(undefined);

test/functional/services/remote/remote.ts

@@ -80,7 +80,7 @@ export async function RemoteProvider({ getService }: FtrProviderContext) {
         driver,
         logging.Type.BROWSER,
         config.get('browser.logPollingMs'),
-        lifecycle.cleanup$ as any
+        lifecycle.cleanup.after$
       )
         .pipe(
           mergeMap(logEntry => {
@@ -110,7 +110,7 @@ export async function RemoteProvider({ getService }: FtrProviderContext) {
     }
   }
 
-  lifecycle.on('beforeTests', async () => {
+  lifecycle.beforeTests.add(async () => {
     // hard coded default, can be overridden per suite using `browser.setWindowSize()`
     // and will be automatically reverted after each suite
     await driver
@@ -120,7 +120,7 @@ export async function RemoteProvider({ getService }: FtrProviderContext) {
   });
 
   const windowSizeStack: Array<{ width: number; height: number }> = [];
-  lifecycle.on('beforeTestSuite', async () => {
+  lifecycle.beforeTestSuite.add(async () => {
     windowSizeStack.unshift(
       await driver
         .manage()
@@ -129,11 +129,11 @@ export async function RemoteProvider({ getService }: FtrProviderContext) {
     );
   });
 
-  lifecycle.on('beforeEachTest', async () => {
+  lifecycle.beforeEachTest.add(async () => {
     await driver.manage().setTimeouts({ implicit: config.get('timeouts.find') });
   });
 
-  lifecycle.on('afterTestSuite', async () => {
+  lifecycle.afterTestSuite.add(async () => {
     const { width, height } = windowSizeStack.shift()!;
     await driver
       .manage()
@@ -143,7 +143,7 @@ export async function RemoteProvider({ getService }: FtrProviderContext) {
     await clearBrowserStorage('localStorage');
   });
 
-  lifecycle.on('cleanup', async () => {
+  lifecycle.cleanup.add(async () => {
     if (logSubscription) {
       await new Promise(r => logSubscription!.add(r));
     }

test/functional/services/remote/webdriver.ts

@@ -115,9 +115,9 @@ async function attemptToCreateCommand(
             session,
             logging.Type.BROWSER,
             logPollingMs,
-            lifecycle.cleanup$
+            lifecycle.cleanup.after$
           ).pipe(
-            takeUntil(lifecycle.cleanup$),
+            takeUntil(lifecycle.cleanup.after$),
             map(({ message, level: { name: level } }) => ({
               message: message.replace(/\\n/g, '\n'),
               level,
@@ -151,7 +151,7 @@ async function attemptToCreateCommand(
         }
 
         const { input, chunk$, cleanup } = await createStdoutSocket();
-        lifecycle.on('cleanup', cleanup);
+        lifecycle.cleanup.add(cleanup);
 
         const session = await new Builder()
           .forBrowser(browserType)

test/visual_regression/services/visual_testing/visual_testing.ts

@@ -54,7 +54,7 @@ export async function VisualTestingProvider({ getService }: FtrProviderContext)
   const lifecycle = getService('lifecycle');
 
   let currentTest: Test | undefined;
-  lifecycle.on('beforeEachTest', (test: Test) => {
+  lifecycle.beforeEachTest.add(test => {
     currentTest = test;
   });