decorate_snapshot_ui: make sure snapshots are saved (#89694)

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

@@ -18,10 +18,11 @@ export interface Suite {
   suites: Suite[];
   tests: Test[];
   title: string;
-  file?: string;
+  file: string;
   parent?: Suite;
   eachTest: (cb: (test: Test) => void) => void;
   root: boolean;
+  suiteTag: string;
 }
 
 export interface Test {

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

@@ -62,7 +62,7 @@ export class FunctionalTestRunner {
       }
 
       const mocha = await setupMocha(this.lifecycle, this.log, config, providers);
-      await this.lifecycle.beforeTests.trigger();
+      await this.lifecycle.beforeTests.trigger(mocha.suite);
       this.log.info('Starting tests');
 
       return await runTests(this.lifecycle, mocha);

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

@@ -8,12 +8,13 @@
 
 import { Lifecycle } from './lifecycle';
 import { FailureMetadata } from './failure_metadata';
+import { Test } from '../fake_mocha_types';
 
 it('collects metadata for the current test', async () => {
   const lifecycle = new Lifecycle();
   const failureMetadata = new FailureMetadata(lifecycle);
 
-  const test1 = {};
+  const test1 = {} as Test;
   await lifecycle.beforeEachRunnable.trigger(test1);
   failureMetadata.add({ foo: 'bar' });
 
@@ -23,7 +24,7 @@ it('collects metadata for the current test', async () => {
     }
   `);
 
-  const test2 = {};
+  const test2 = {} as Test;
   await lifecycle.beforeEachRunnable.trigger(test2);
   failureMetadata.add({ test: 2 });
 
@@ -43,7 +44,7 @@ it('adds messages to the messages state', () => {
   const lifecycle = new Lifecycle();
   const failureMetadata = new FailureMetadata(lifecycle);
 
-  const test1 = {};
+  const test1 = {} as Test;
   lifecycle.beforeEachRunnable.trigger(test1);
   failureMetadata.addMessages(['foo', 'bar']);
   failureMetadata.addMessages(['baz']);

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

@@ -8,21 +8,18 @@
 
 import { LifecyclePhase } from './lifecycle_phase';
 
-// mocha's global types mean we can't import Mocha or it will override the global jest types..............
-type ItsASuite = any;
-type ItsATest = any;
-type ItsARunnable = any;
+import { Suite, Test } from '../fake_mocha_types';
 
 export class Lifecycle {
-  public readonly beforeTests = new LifecyclePhase<[]>({
+  public readonly beforeTests = new LifecyclePhase<[Suite]>({
     singular: true,
   });
-  public readonly beforeEachRunnable = new LifecyclePhase<[ItsARunnable]>();
-  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 beforeEachRunnable = new LifecyclePhase<[Test]>();
+  public readonly beforeTestSuite = new LifecyclePhase<[Suite]>();
+  public readonly beforeEachTest = new LifecyclePhase<[Test]>();
+  public readonly afterTestSuite = new LifecyclePhase<[Suite]>();
+  public readonly testFailure = new LifecyclePhase<[Error, Test]>();
+  public readonly testHookFailure = new LifecyclePhase<[Error, Test]>();
   public readonly cleanup = new LifecyclePhase<[]>({
     singular: true,
   });

packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.test.ts

@@ -12,36 +12,36 @@ import { decorateSnapshotUi, expectSnapshot } from './decorate_snapshot_ui';
 import path from 'path';
 import fs from 'fs';
 
-const createMockSuite = ({ tests, root = true }: { tests: Test[]; root?: boolean }) => {
+const createRootSuite = () => {
   const suite = {
-    tests,
-    root,
-    eachTest: (cb: (test: Test) => void) => {
+    tests: [] as Test[],
+    root: true,
+    eachTest: (cb) => {
       suite.tests.forEach((test) => cb(test));
     },
+    parent: undefined,
   } as Suite;
 
   return suite;
 };
 
-const createMockTest = ({
+const registerTest = ({
+  parent,
   title = 'Test',
   passed = true,
-  filename = __filename,
-  parent,
-}: { title?: string; passed?: boolean; filename?: string; parent?: Suite } = {}) => {
+}: {
+  parent: Suite;
+  title?: string;
+  passed?: boolean;
+}) => {
   const test = ({
-    file: filename,
+    file: __filename,
     fullTitle: () => title,
     isPassed: () => passed,
   } as unknown) as Test;
 
-  if (parent) {
-    parent.tests.push(test);
-    test.parent = parent;
-  } else {
-    test.parent = createMockSuite({ tests: [test] });
-  }
+  parent.tests.push(test);
+  test.parent = parent;
 
   return test;
 };
@@ -63,34 +63,41 @@ describe('decorateSnapshotUi', () => {
 
   describe('when running a test', () => {
     let lifecycle: Lifecycle;
-    beforeEach(() => {
+    let rootSuite: Suite;
+    beforeEach(async () => {
       lifecycle = new Lifecycle();
+      rootSuite = createRootSuite();
       decorateSnapshotUi({ lifecycle, updateSnapshots: false, isCi: false });
+
+      await lifecycle.beforeTests.trigger(rootSuite);
     });
 
     it('passes when the snapshot matches the actual value', async () => {
-      const test = createMockTest();
+      const test = registerTest({ parent: rootSuite });
 
       await lifecycle.beforeEachTest.trigger(test);
 
       expect(() => {
         expectSnapshot('foo').toMatchInline(`"foo"`);
       }).not.toThrow();
+
+      await lifecycle.cleanup.trigger();
     });
 
     it('throws when the snapshot does not match the actual value', async () => {
-      const test = createMockTest();
+      const test = registerTest({ parent: rootSuite });
 
       await lifecycle.beforeEachTest.trigger(test);
 
       expect(() => {
         expectSnapshot('foo').toMatchInline(`"bar"`);
       }).toThrow();
+
+      await lifecycle.cleanup.trigger();
     });
 
     it('writes a snapshot to an external file if it does not exist', async () => {
-      const test: Test = createMockTest();
-
+      const test = registerTest({ parent: rootSuite });
       await lifecycle.beforeEachTest.trigger(test);
 
       expect(fs.existsSync(snapshotFile)).toBe(false);
@@ -99,7 +106,7 @@ describe('decorateSnapshotUi', () => {
         expectSnapshot('foo').toMatch();
       }).not.toThrow();
 
-      await lifecycle.afterTestSuite.trigger(test.parent);
+      await lifecycle.cleanup.trigger();
 
       expect(fs.existsSync(snapshotFile)).toBe(true);
     });
@@ -107,9 +114,13 @@ describe('decorateSnapshotUi', () => {
 
   describe('when writing multiple snapshots to a single file', () => {
     let lifecycle: Lifecycle;
-    beforeEach(() => {
+    let rootSuite: Suite;
+    beforeEach(async () => {
       lifecycle = new Lifecycle();
+      rootSuite = createRootSuite();
       decorateSnapshotUi({ lifecycle, updateSnapshots: false, isCi: false });
+
+      await lifecycle.beforeTests.trigger(rootSuite);
     });
 
     beforeEach(() => {
@@ -127,7 +138,7 @@ exports[\`Test2 1\`] = \`"bar"\`;
     });
 
     it('compares to an existing snapshot', async () => {
-      const test1 = createMockTest({ title: 'Test1' });
+      const test1 = registerTest({ parent: rootSuite, title: 'Test1' });
 
       await lifecycle.beforeEachTest.trigger(test1);
 
@@ -135,7 +146,7 @@ exports[\`Test2 1\`] = \`"bar"\`;
         expectSnapshot('foo').toMatch();
       }).not.toThrow();
 
-      const test2 = createMockTest({ title: 'Test2' });
+      const test2 = registerTest({ parent: rootSuite, title: 'Test2' });
 
       await lifecycle.beforeEachTest.trigger(test2);
 
@@ -143,19 +154,23 @@ exports[\`Test2 1\`] = \`"bar"\`;
         expectSnapshot('foo').toMatch();
       }).toThrow();
 
-      await lifecycle.afterTestSuite.trigger(test1.parent);
+      await lifecycle.cleanup.trigger();
     });
   });
 
   describe('when updating snapshots', () => {
     let lifecycle: Lifecycle;
-    beforeEach(() => {
+    let rootSuite: Suite;
+    beforeEach(async () => {
       lifecycle = new Lifecycle();
+      rootSuite = createRootSuite();
       decorateSnapshotUi({ lifecycle, updateSnapshots: true, isCi: false });
+
+      await lifecycle.beforeTests.trigger(rootSuite);
     });
 
     it("doesn't throw if the value does not match", async () => {
-      const test = createMockTest();
+      const test = registerTest({ parent: rootSuite });
 
       await lifecycle.beforeEachTest.trigger(test);
 
@@ -163,23 +178,64 @@ exports[\`Test2 1\`] = \`"bar"\`;
         expectSnapshot('bar').toMatchInline(`"foo"`);
       }).not.toThrow();
     });
+
+    describe('writing to disk', () => {
+      beforeEach(() => {
+        fs.mkdirSync(path.resolve(__dirname, '__snapshots__'));
+        fs.writeFileSync(
+          snapshotFile,
+          `// Jest Snapshot v1, https://goo.gl/fbAQLP
+  
+  exports[\`Test 1\`] = \`"foo"\`;
+        `,
+          { encoding: 'utf-8' }
+        );
+      });
+
+      it('updates existing external snapshots', async () => {
+        const test = registerTest({ parent: rootSuite });
+
+        await lifecycle.beforeEachTest.trigger(test);
+
+        expect(() => {
+          expectSnapshot('bar').toMatch();
+        }).not.toThrow();
+
+        await lifecycle.cleanup.trigger();
+
+        const file = fs.readFileSync(snapshotFile, { encoding: 'utf-8' });
+
+        expect(file).toMatchInlineSnapshot(`
+          "// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+          exports[\`Test 1\`] = \`\\"bar\\"\`;
+          "
+        `);
+      });
+    });
   });
 
   describe('when running on ci', () => {
     let lifecycle: Lifecycle;
-    beforeEach(() => {
+    let rootSuite: Suite;
+    beforeEach(async () => {
       lifecycle = new Lifecycle();
+      rootSuite = createRootSuite();
       decorateSnapshotUi({ lifecycle, updateSnapshots: false, isCi: true });
+
+      await lifecycle.beforeTests.trigger(rootSuite);
     });
 
     it('throws on new snapshots', async () => {
-      const test = createMockTest();
+      const test = registerTest({ parent: rootSuite });
 
       await lifecycle.beforeEachTest.trigger(test);
 
       expect(() => {
         expectSnapshot('bar').toMatchInline();
       }).toThrow();
+
+      await lifecycle.cleanup.trigger();
     });
 
     describe('when adding to an existing file', () => {
@@ -198,17 +254,27 @@ exports[\`Test2 1\`] = \`"bar"\`;
       });
 
       it('does not throw on an existing test', async () => {
-        const test = createMockTest({ title: 'Test' });
+        const test = registerTest({ parent: rootSuite });
 
         await lifecycle.beforeEachTest.trigger(test);
 
         expect(() => {
           expectSnapshot('foo').toMatch();
         }).not.toThrow();
+
+        const test2 = registerTest({ parent: rootSuite, title: 'Test2' });
+
+        await lifecycle.beforeEachTest.trigger(test2);
+
+        expect(() => {
+          expectSnapshot('bar').toMatch();
+        }).not.toThrow();
+
+        await lifecycle.cleanup.trigger();
       });
 
       it('throws on a new test', async () => {
-        const test = createMockTest({ title: 'New test' });
+        const test = registerTest({ parent: rootSuite, title: 'New test' });
 
         await lifecycle.beforeEachTest.trigger(test);
 
@@ -217,8 +283,8 @@ exports[\`Test2 1\`] = \`"bar"\`;
         }).toThrow();
       });
 
-      it('does not throw when all snapshots are used ', async () => {
-        const test = createMockTest({ title: 'Test' });
+      it('does not throw when all snapshots are used', async () => {
+        const test = registerTest({ parent: rootSuite });
 
         await lifecycle.beforeEachTest.trigger(test);
 
@@ -226,7 +292,7 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('foo').toMatch();
         }).not.toThrow();
 
-        const test2 = createMockTest({ title: 'Test2' });
+        const test2 = registerTest({ parent: rootSuite, title: 'Test2' });
 
         await lifecycle.beforeEachTest.trigger(test2);
 
@@ -234,13 +300,13 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('bar').toMatch();
         }).not.toThrow();
 
-        const afterTestSuite = lifecycle.afterTestSuite.trigger(test.parent);
+        const afterCleanup = lifecycle.cleanup.trigger();
 
-        await expect(afterTestSuite).resolves.toBe(undefined);
+        await expect(afterCleanup).resolves.toBe(undefined);
       });
 
       it('throws on unused snapshots', async () => {
-        const test = createMockTest({ title: 'Test' });
+        const test = registerTest({ parent: rootSuite });
 
         await lifecycle.beforeEachTest.trigger(test);
 
@@ -248,9 +314,9 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('foo').toMatch();
         }).not.toThrow();
 
-        const afterTestSuite = lifecycle.afterTestSuite.trigger(test.parent);
+        const afterCleanup = lifecycle.cleanup.trigger();
 
-        await expect(afterTestSuite).rejects.toMatchInlineSnapshot(`
+        await expect(afterCleanup).rejects.toMatchInlineSnapshot(`
                 [Error: 1 obsolete snapshot(s) found:
                 Test2 1.
 
@@ -259,17 +325,11 @@ exports[\`Test2 1\`] = \`"bar"\`;
       });
 
       it('does not throw on unused when some tests are skipped', async () => {
-        const root = createMockSuite({ tests: [] });
+        const test = registerTest({ parent: rootSuite, passed: true });
 
-        const test = createMockTest({
-          title: 'Test',
-          parent: root,
-          passed: true,
-        });
-
-        createMockTest({
+        registerTest({
           title: 'Test2',
-          parent: root,
+          parent: rootSuite,
           passed: false,
         });
 
@@ -279,9 +339,9 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('foo').toMatch();
         }).not.toThrow();
 
-        const afterTestSuite = lifecycle.afterTestSuite.trigger(root);
+        const afterCleanup = lifecycle.cleanup.trigger();
 
-        await expect(afterTestSuite).resolves.toBeUndefined();
+        await expect(afterCleanup).resolves.toBeUndefined();
       });
     });
   });

packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.ts

@@ -71,43 +71,51 @@ export function decorateSnapshotUi({
   updateSnapshots: boolean;
   isCi: boolean;
 }) {
-  globalState.registered = true;
-  globalState.snapshotStates = {};
-  globalState.currentTest = null;
+  let rootSuite: Suite | undefined;
 
-  if (isCi) {
-    // make sure snapshots that have not been committed
-    // are not written to file on CI, passing the test
-    globalState.updateSnapshot = 'none';
-  } else {
-    globalState.updateSnapshot = updateSnapshots ? 'all' : 'new';
-  }
+  lifecycle.beforeTests.add((root) => {
+    if (!root) {
+      throw new Error('Root suite was not set');
+    }
+    rootSuite = root;
 
-  modifyStackTracePrepareOnce();
+    globalState.registered = true;
+    globalState.snapshotStates = {};
+    globalState.currentTest = null;
 
-  addSerializer({
-    serialize: (num: number) => {
-      return String(parseFloat(num.toPrecision(15)));
-    },
-    test: (value: any) => {
-      return typeof value === 'number';
-    },
+    if (isCi) {
+      // make sure snapshots that have not been committed
+      // are not written to file on CI, passing the test
+      globalState.updateSnapshot = 'none';
+    } else {
+      globalState.updateSnapshot = updateSnapshots ? 'all' : 'new';
+    }
+
+    modifyStackTracePrepareOnce();
+
+    addSerializer({
+      serialize: (num: number) => {
+        return String(parseFloat(num.toPrecision(15)));
+      },
+      test: (value: any) => {
+        return typeof value === 'number';
+      },
+    });
+
+    // @ts-expect-error
+    global.expectSnapshot = expectSnapshot;
   });
 
-  // @ts-expect-error
-  global.expectSnapshot = expectSnapshot;
-
   lifecycle.beforeEachTest.add((test: Test) => {
     globalState.currentTest = test;
   });
 
-  lifecycle.afterTestSuite.add(function (testSuite: Suite) {
-    // save snapshot & check unused after top-level test suite completes
-    if (!testSuite.root) {
+  lifecycle.cleanup.add(() => {
+    if (!rootSuite) {
       return;
     }
 
-    testSuite.eachTest((test) => {
+    rootSuite.eachTest((test) => {
       const file = test.file;
 
       if (!file) {

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

@@ -17,6 +17,7 @@ jest.mock('@kbn/utils', () => {
 import { REPO_ROOT } from '@kbn/dev-utils';
 import { Lifecycle } from './lifecycle';
 import { SuiteTracker } from './suite_tracker';
+import { Suite } from '../fake_mocha_types';
 
 const DEFAULT_TEST_METADATA_PATH = join(REPO_ROOT, 'target', 'test_metadata.json');
 const MOCK_CONFIG_PATH = join('test', 'config.js');
@@ -47,18 +48,18 @@ describe('SuiteTracker', () => {
     jest.resetAllMocks();
   });
 
-  let MOCKS: Record<string, object>;
+  let MOCKS: Record<string, Suite>;
 
   const createMock = (overrides = {}) => {
-    return {
+    return ({
       file: resolve(REPO_ROOT, MOCK_TEST_PATH),
       title: 'A Test',
       suiteTag: MOCK_TEST_PATH,
       ...overrides,
-    };
+    } as unknown) as Suite;
   };
 
-  const runLifecycleWithMocks = async (mocks: object[], fn: (objs: any) => any = () => {}) => {
+  const runLifecycleWithMocks = async (mocks: Suite[], fn: (objs: any) => any = () => {}) => {
     const lifecycle = new Lifecycle();
     const suiteTracker = SuiteTracker.startTracking(
       lifecycle,

packages/kbn-test/types/ftr.d.ts

@@ -13,6 +13,7 @@ import {
   FailureMetadata,
   DockerServersService,
 } from '../src/functional_test_runner/lib';
+import { Test, Suite } from '../src/functional_test_runner/fake_mocha_types';
 
 export { Lifecycle, Config, FailureMetadata };
 
@@ -91,3 +92,5 @@ export interface FtrConfigProviderContext {
   log: ToolingLog;
   readConfigFile(path: string): Promise<Config>;
 }
+
+export { Test, Suite };

test/visual_regression/services/visual_testing/visual_testing.ts

@@ -7,10 +7,8 @@
  */
 
 import { postSnapshot } from '@percy/agent/dist/utils/sdk-utils';
-import { Test } from 'mocha';
-
 import testSubjSelector from '@kbn/test-subj-selector';
-
+import { Test } from '@kbn/test/types/ftr';
 import { pkg } from '../../../../src/core/server/utils';
 import { FtrProviderContext } from '../../ftr_provider_context';