[7.x] [APM] Optimize API test order (#88654) (#89398)

Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
elastic · Jan 27, 2021 · 4345068 · 4345068
1 parent a893d34
commit 4345068
Show file tree

Hide file tree

Showing 87 changed files with 3,447 additions and 3,533 deletions.
diff --git a/packages/kbn-test/src/functional_test_runner/fake_mocha_types.d.ts b/packages/kbn-test/src/functional_test_runner/fake_mocha_types.d.ts
@@ -20,6 +20,8 @@ export interface Suite {
   title: string;
   file?: string;
   parent?: Suite;
+  eachTest: (cb: (test: Test) => void) => void;
+  root: boolean;
 }
 
 export interface Test {

diff --git a/packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.test.ts b/packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.test.ts
@@ -6,21 +6,44 @@
  * Public License, v 1.
  */
 
-import { Test } from '../../fake_mocha_types';
+import { Suite, Test } from '../../fake_mocha_types';
 import { Lifecycle } from '../lifecycle';
 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 suite = {
+    tests,
+    root,
+    eachTest: (cb: (test: Test) => void) => {
+      suite.tests.forEach((test) => cb(test));
+    },
+  } as Suite;
+
+  return suite;
+};
+
 const createMockTest = ({
   title = 'Test',
   passed = true,
-}: { title?: string; passed?: boolean } = {}) => {
-  return {
+  filename = __filename,
+  parent,
+}: { title?: string; passed?: boolean; filename?: string; parent?: Suite } = {}) => {
+  const test = ({
+    file: filename,
     fullTitle: () => title,
     isPassed: () => passed,
-    parent: {},
-  } as Test;
+  } as unknown) as Test;
+
+  if (parent) {
+    parent.tests.push(test);
+    test.parent = parent;
+  } else {
+    test.parent = createMockSuite({ tests: [test] });
+  }
+
+  return test;
 };
 
 describe('decorateSnapshotUi', () => {
@@ -211,7 +234,7 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('bar').toMatch();
         }).not.toThrow();
 
-        const afterTestSuite = lifecycle.afterTestSuite.trigger({});
+        const afterTestSuite = lifecycle.afterTestSuite.trigger(test.parent);
 
         await expect(afterTestSuite).resolves.toBe(undefined);
       });
@@ -225,7 +248,7 @@ exports[\`Test2 1\`] = \`"bar"\`;
           expectSnapshot('foo').toMatch();
         }).not.toThrow();
 
-        const afterTestSuite = lifecycle.afterTestSuite.trigger({});
+        const afterTestSuite = lifecycle.afterTestSuite.trigger(test.parent);
 
         await expect(afterTestSuite).rejects.toMatchInlineSnapshot(`
                 [Error: 1 obsolete snapshot(s) found:
@@ -234,6 +257,32 @@ exports[\`Test2 1\`] = \`"bar"\`;
                 Run tests again with \`--updateSnapshots\` to remove them.]
               `);
       });
+
+      it('does not throw on unused when some tests are skipped', async () => {
+        const root = createMockSuite({ tests: [] });
+
+        const test = createMockTest({
+          title: 'Test',
+          parent: root,
+          passed: true,
+        });
+
+        createMockTest({
+          title: 'Test2',
+          parent: root,
+          passed: false,
+        });
+
+        await lifecycle.beforeEachTest.trigger(test);
+
+        expect(() => {
+          expectSnapshot('foo').toMatch();
+        }).not.toThrow();
+
+        const afterTestSuite = lifecycle.afterTestSuite.trigger(root);
+
+        await expect(afterTestSuite).resolves.toBeUndefined();
+      });
     });
   });
 });
diff --git a/packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.ts b/packages/kbn-test/src/functional_test_runner/lib/snapshots/decorate_snapshot_ui.ts
@@ -16,9 +16,8 @@ import path from 'path';
 import prettier from 'prettier';
 import babelTraverse from '@babel/traverse';
 import { once } from 'lodash';
-import callsites from 'callsites';
 import { Lifecycle } from '../lifecycle';
-import { Test } from '../../fake_mocha_types';
+import { Suite, Test } from '../../fake_mocha_types';
 
 type ISnapshotState = InstanceType<typeof SnapshotState>;
 
@@ -33,12 +32,12 @@ const globalState: {
   updateSnapshot: SnapshotUpdateState;
   registered: boolean;
   currentTest: Test | null;
-  snapshots: Array<{ tests: Test[]; file: string; snapshotState: ISnapshotState }>;
+  snapshotStates: Record<string, ISnapshotState>;
 } = {
   updateSnapshot: 'none',
   registered: false,
   currentTest: null,
-  snapshots: [],
+  snapshotStates: {},
 };
 
 const modifyStackTracePrepareOnce = once(() => {
@@ -73,7 +72,7 @@ export function decorateSnapshotUi({
   isCi: boolean;
 }) {
   globalState.registered = true;
-  globalState.snapshots.length = 0;
+  globalState.snapshotStates = {};
   globalState.currentTest = null;
 
   if (isCi) {
@@ -102,32 +101,36 @@ export function decorateSnapshotUi({
     globalState.currentTest = test;
   });
 
-  lifecycle.afterTestSuite.add(function (testSuite) {
+  lifecycle.afterTestSuite.add(function (testSuite: Suite) {
     // save snapshot & check unused after top-level test suite completes
-    if (testSuite.parent?.parent) {
+    if (!testSuite.root) {
       return;
     }
 
-    const unused: string[] = [];
+    testSuite.eachTest((test) => {
+      const file = test.file;
 
-    globalState.snapshots.forEach((snapshot) => {
-      const { tests, snapshotState } = snapshot;
-      tests.forEach((test) => {
-        const title = test.fullTitle();
-        // If test is failed or skipped, mark snapshots as used. Otherwise,
-        // running a test in isolation will generate false positives.
-        if (!test.isPassed()) {
-          snapshotState.markSnapshotsAsCheckedForTest(title);
-        }
-      });
+      if (!file) {
+        return;
+      }
+
+      const snapshotState = globalState.snapshotStates[file];
+
+      if (snapshotState && !test.isPassed()) {
+        snapshotState.markSnapshotsAsCheckedForTest(test.fullTitle());
+      }
+    });
+
+    const unused: string[] = [];
 
-      if (globalState.updateSnapshot !== 'all') {
-        unused.push(...snapshotState.getUncheckedKeys());
-      } else {
-        snapshotState.removeUncheckedKeys();
+    Object.values(globalState.snapshotStates).forEach((state) => {
+      if (globalState.updateSnapshot === 'all') {
+        state.removeUncheckedKeys();
       }
 
-      snapshotState.save();
+      unused.push(...state.getUncheckedKeys());
+
+      state.save();
     });
 
     if (unused.length) {
@@ -138,7 +141,7 @@ export function decorateSnapshotUi({
       );
     }
 
-    globalState.snapshots.length = 0;
+    globalState.snapshotStates = {};
   });
 }
 
@@ -161,43 +164,29 @@ function getSnapshotState(file: string, updateSnapshot: SnapshotUpdateState) {
 
 export function expectSnapshot(received: any) {
   if (!globalState.registered) {
-    throw new Error(
-      'Mocha hooks were not registered before expectSnapshot was used. Call `registerMochaHooksForSnapshots` in your top-level describe().'
-    );
-  }
-
-  if (!globalState.currentTest) {
-    throw new Error('expectSnapshot can only be called inside of an it()');
+    throw new Error('expectSnapshot UI was not initialized before calling expectSnapshot()');
   }
 
-  const [, fileOfTest] = callsites().map((site) => site.getFileName());
+  const test = globalState.currentTest;
 
-  if (!fileOfTest) {
-    throw new Error("Couldn't infer a filename for the current test");
+  if (!test) {
+    throw new Error('expectSnapshot can only be called inside of an it()');
   }
 
-  let snapshot = globalState.snapshots.find(({ file }) => file === fileOfTest);
-
-  if (!snapshot) {
-    snapshot = {
-      file: fileOfTest,
-      tests: [],
-      snapshotState: getSnapshotState(fileOfTest, globalState.updateSnapshot),
-    };
-    globalState.snapshots.unshift(snapshot!);
+  if (!test.file) {
+    throw new Error('File for test not found');
   }
 
-  if (!snapshot) {
-    throw new Error('Snapshot is undefined');
-  }
+  let snapshotState = globalState.snapshotStates[test.file];
 
-  if (!snapshot.tests.includes(globalState.currentTest)) {
-    snapshot.tests.push(globalState.currentTest);
+  if (!snapshotState) {
+    snapshotState = getSnapshotState(test.file, globalState.updateSnapshot);
+    globalState.snapshotStates[test.file] = snapshotState;
   }
 
   const context: SnapshotContext = {
-    snapshotState: snapshot.snapshotState,
-    currentTestName: globalState.currentTest.fullTitle(),
+    snapshotState,
+    currentTestName: test.fullTitle(),
   };
 
   return {

diff --git a/x-pack/test/apm_api_integration/basic/config.ts b/x-pack/test/apm_api_integration/basic/config.ts
@@ -4,10 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { createTestConfig } from '../common/config';
+import { configs } from '../configs';
 
-export default createTestConfig({
-  license: 'basic',
-  name: 'X-Pack APM API integration tests (basic)',
-  testFiles: [require.resolve('./tests')],
-});
+export default configs.basic;