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

Fix test retries that contain snapshots #8629

Merged
merged 5 commits into from Jul 4, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -19,6 +19,7 @@
- `[jest-snapshot]` Prevent inline snapshots from drifting when inline snapshots are updated ([#8492](https://github.com/facebook/jest/pull/8492))
- `[jest-haste-map]` Don't throw on missing mapper in Node crawler ([#8558](https://github.com/facebook/jest/pull/8558))
- `[jest-core]` Fix incorrect `passWithNoTests` warning ([#8595](https://github.com/facebook/jest/pull/8595))
- `[jest-snapshots]` Fix test retries that contain snapshots ([#8629](https://github.com/facebook/jest/pull/8629))

### Chore & Maintenance

Expand Down
141 changes: 141 additions & 0 deletions e2e/__tests__/toMatchInlineSnapshotWithRetries.test.ts
@@ -0,0 +1,141 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import path from 'path';
import {cleanup, makeTemplate, writeFiles} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(__dirname, '../to-match-inline-snapshot-with-retries');
const TESTS_DIR = path.resolve(DIR, '__tests__');

beforeEach(() => cleanup(TESTS_DIR));
afterAll(() => cleanup(TESTS_DIR));

test('works with a single snapshot', () => {
const filename = 'basic-support.test.js';
const template = makeTemplate(`
let index = 0;
afterEach(() => {
index += 1;
});
jest.retryTimes($2);
test('snapshots', () => expect($1).toMatchInlineSnapshot(\`3\`));
`);

{
writeFiles(TESTS_DIR, {
[filename]: template(['3', '1' /* retries */]),
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(0);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Received: 2');
expect(stderr).toMatch('1 snapshot failed from 1 test suite.');
expect(status).toBe(1);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '4' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(0);
}
});

test('works a different assertion is failing', () => {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
const filename = 'basic-support.test.js';
const template = makeTemplate(`
jest.retryTimes($1);
test('snapshots', () => {
expect(3).toMatchInlineSnapshot(\`3\`);
expect(false).toBe(true);
});
`);

{
writeFiles(TESTS_DIR, {
[filename]: template(['4']),
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Test Suites: 1 failed, 1 total');
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(1);
}
});

test('works when multiple tests have snapshots but only one of them failed multiple times', () => {
const filename = 'basic-support.test.js';
const template = makeTemplate(`
test('passing snapshots', () => expect(1).toMatchInlineSnapshot(\`1\`));
describe('with retries', () => {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
let index = 0;
afterEach(() => {
index += 1;
});
jest.retryTimes($2);
test('snapshots', () => expect($1).toMatchInlineSnapshot(\`3\`));
});
`);

{
writeFiles(TESTS_DIR, {
[filename]: template(['3', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Snapshots: 2 passed, 2 total');
expect(status).toBe(0);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Snapshot name: `with retries snapshots 1`');
expect(stderr).toMatch('Received: 2');
expect(stderr).toMatch('1 snapshot failed from 1 test suite.');
expect(status).toBe(1);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '4' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(0);
}
});
119 changes: 119 additions & 0 deletions e2e/__tests__/toMatchSnapshotWithRetries.test.ts
@@ -0,0 +1,119 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import path from 'path';
import {cleanup, makeTemplate, writeFiles} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(__dirname, '../to-match-snapshot-with-retries');
const TESTS_DIR = path.resolve(DIR, '__tests__');

beforeEach(() => cleanup(TESTS_DIR));
afterAll(() => cleanup(TESTS_DIR));

test('works with a single snapshot', () => {
const filename = 'basic-support.test.js';
const template = makeTemplate(`
let index = 0;
afterEach(() => {
index += 1;
});
jest.retryTimes($2);
test('snapshots', () => expect($1).toMatchSnapshot());
`);

{
writeFiles(TESTS_DIR, {
[filename]: template(['3', '1' /* retries */]),
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('1 snapshot written from 1 test suite.');
expect(status).toBe(0);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Received: 2');
expect(stderr).toMatch('1 snapshot failed from 1 test suite.');
expect(status).toBe(1);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '4' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(0);
}
});

test('works when multiple tests have snapshots but only one of them failed multiple times', () => {
const filename = 'basic-support.test.js';
const template = makeTemplate(`
test('passing snapshots', () => expect('foo').toMatchSnapshot());
describe('with retries', () => {
let index = 0;
afterEach(() => {
index += 1;
});
jest.retryTimes($2);
test('snapshots', () => expect($1).toMatchSnapshot());
});
`);

{
writeFiles(TESTS_DIR, {
[filename]: template(['3', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('2 snapshots written from 1 test suite.');
expect(status).toBe(0);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '2' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Received: 2');
expect(stderr).toMatch('1 snapshot failed from 1 test suite.');
expect(status).toBe(1);
}

{
writeFiles(TESTS_DIR, {
[filename]: template(['index', '4' /* retries */]),
});
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
'--testRunner=jest-circus/runner',
filename,
]);
expect(stderr).toMatch('Snapshots: 1 passed, 1 total');
expect(status).toBe(0);
}
});
5 changes: 5 additions & 0 deletions e2e/to-match-inline-snapshot-with-retries/package.json
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
5 changes: 5 additions & 0 deletions e2e/to-match-snapshot-with-retries/package.json
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
2 changes: 2 additions & 0 deletions packages/expect/src/index.ts
Expand Up @@ -43,6 +43,7 @@ import {
INTERNAL_MATCHER_FLAG,
getState,
setState,
clearState,
getMatchers,
setMatchers,
} from './jestMatchersObject';
Expand Down Expand Up @@ -402,6 +403,7 @@ expect.assertions = assertions;
expect.hasAssertions = hasAssertions;
expect.getState = getState;
expect.setState = setState;
expect.clearState = clearState;
expect.extractExpectedAssertionsErrors = extractExpectedAssertionsErrors;

const expectExport = expect as Expect;
Expand Down
4 changes: 4 additions & 0 deletions packages/expect/src/jestMatchersObject.ts
Expand Up @@ -37,6 +37,10 @@ export const setState = (state: object) => {
Object.assign((global as any)[JEST_MATCHERS_OBJECT].state, state);
};

export const clearState = () => {
getState().snapshotState.clear();
};

export const getMatchers = () => (global as any)[JEST_MATCHERS_OBJECT].matchers;

export const setMatchers = (
Expand Down
1 change: 1 addition & 0 deletions packages/expect/src/types.ts
Expand Up @@ -44,6 +44,7 @@ export type MatcherState = {
isExpectingAssertions?: boolean;
isNot: boolean;
promise: string;
snapshotState: any;
SimenB marked this conversation as resolved.
Show resolved Hide resolved
suppressedErrors: Array<Error>;
testPath?: Config.Path;
utils: typeof jestMatcherUtils & {
Expand Down
4 changes: 4 additions & 0 deletions packages/jest-circus/src/eventHandler.ts
Expand Up @@ -148,7 +148,11 @@ const eventHandler: Circus.EventHandler = (event, state): void => {
break;
}
case 'test_retry': {
// Clear errors so tests can be retried (and not immediately fail)
event.test.errors = [];
// Clear any snapshot data that occurred in previous test run
global.expect.clearState();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This forces expect to be available, while circus is so far not tightly coupled to it (and in theory built to be used anywhere, the legacy-code directory aside). Can we make it gracefully handle expect/clearState not being available and move it to a separate event handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great point, you are right we should find to prevent jest-circus of being coupled with expect.


break;
}
case 'run_start': {
Expand Down
17 changes: 17 additions & 0 deletions packages/jest-snapshot/src/State.ts
Expand Up @@ -47,6 +47,7 @@ export default class SnapshotState {
private _uncheckedKeys: Set<string>;
private _getBabelTraverse: () => Function;
private _getPrettier: () => null | any;
private _reinitializeData: () => void;

added: number;
expand: boolean;
Expand All @@ -60,6 +61,7 @@ export default class SnapshotState {
this._snapshotPath,
options.updateSnapshot,
);

this._snapshotData = data;
this._dirty = dirty;
this._getBabelTraverse = options.getBabelTraverse;
Expand All @@ -74,6 +76,17 @@ export default class SnapshotState {
this.unmatched = 0;
this._updateSnapshot = options.updateSnapshot;
this.updated = 0;

this._reinitializeData = () => {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
this._snapshotData = data;
this._inlineSnapshots = [];
this._counters = new Map();
this._index = 0;
this.added = 0;
this.matched = 0;
this.unmatched = 0;
this.updated = 0;
};
}

markSnapshotsAsCheckedForTest(testName: string) {
Expand Down Expand Up @@ -108,6 +121,10 @@ export default class SnapshotState {
}
}

clear() {
this._reinitializeData();
}

save() {
const hasExternalSnapshots = Object.keys(this._snapshotData).length;
const hasInlineSnapshots = this._inlineSnapshots.length;
Expand Down