Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Optimize API test order #88654

Merged
merged 17 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export interface Suite {
title: string;
file?: string;
parent?: Suite;
eachTest: (cb: (test: Test) => void) => void;
root: boolean;
}

export interface Test {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
Expand All @@ -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:
Expand All @@ -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();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand All @@ -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(() => {
Expand Down Expand Up @@ -73,7 +72,7 @@ export function decorateSnapshotUi({
isCi: boolean;
}) {
globalState.registered = true;
globalState.snapshots.length = 0;
globalState.snapshotStates = {};
globalState.currentTest = null;

if (isCi) {
Expand Down Expand Up @@ -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) {
Expand All @@ -138,7 +141,7 @@ export function decorateSnapshotUi({
);
}

globalState.snapshots.length = 0;
globalState.snapshotStates = {};
});
}

Expand All @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions x-pack/test/apm_api_integration/basic/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading