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

Mark snapshots as obsolete when moved to an inline snapshot #6773

Merged
merged 2 commits into from
Aug 7, 2018
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Fixes

- `[jest-snapshot` Mark snapshots as obsolete when moved to an inline snapshot ([#6773](https://github.com/facebook/jest/pull/6773))
- `[jest-config]` Fix `--coverage` with `--findRelatedTests` overwriting `collectCoverageFrom` options ([#6736](https://github.com/facebook/jest/pull/6736))
- `[jest-config]` Update default config for testURL from 'about:blank' to 'http://localhost' to address latest JSDOM security warning. ([#6792](https://github.com/facebook/jest/pull/6792))

Expand Down
22 changes: 22 additions & 0 deletions e2e/__tests__/__snapshots__/to_match_inline_snapshot.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,28 @@ Object {
"
`;

exports[`removes obsolete external snapshots: external snapshot cleaned 1`] = `
"test('removes obsolete external snapshots', () => {
expect('1').toMatchInlineSnapshot(\`\\"1\\"\`);
});
"
`;

exports[`removes obsolete external snapshots: initial write 1`] = `
"
test('removes obsolete external snapshots', () => {
expect('1').toMatchSnapshot();
});
"
`;

exports[`removes obsolete external snapshots: inline snapshot written 1`] = `
"test('removes obsolete external snapshots', () => {
expect('1').toMatchInlineSnapshot(\`\\"1\\"\`);
});
"
`;

exports[`supports async matchers 1`] = `
"test('inline snapshots', async () => {
expect(Promise.resolve('success')).resolves.toMatchInlineSnapshot(
Expand Down
48 changes: 48 additions & 0 deletions e2e/__tests__/to_match_inline_snapshot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,54 @@ test('handles property matchers', () => {
}
});

test('removes obsolete external snapshots', () => {
const filename = 'removes-obsolete-external-snapshots.test.js';
const snapshotPath = path.join(
TESTS_DIR,
'__snapshots__',
filename + '.snap',
);
const template = makeTemplate(`
test('removes obsolete external snapshots', () => {
expect('1').$1();
});
`);

{
writeFiles(TESTS_DIR, {[filename]: template(['toMatchSnapshot'])});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
const fileAfter = readFile(filename);
expect(stderr).toMatch('1 snapshot written from 1 test suite.');
expect(status).toBe(0);
expect(fileAfter).toMatchSnapshot('initial write');
expect(fs.existsSync(snapshotPath)).toEqual(true);
}

{
writeFiles(TESTS_DIR, {[filename]: template(['toMatchInlineSnapshot'])});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
const fileAfter = readFile(filename);
expect(stderr).toMatch('Snapshots: 1 obsolete, 1 written, 1 total');
expect(status).toBe(1);
expect(fileAfter).toMatchSnapshot('inline snapshot written');
expect(fs.existsSync(snapshotPath)).toEqual(true);
}

{
const {stderr, status} = runJest(DIR, [
'-w=1',
'--ci=false',
filename,
'-u',
]);
const fileAfter = readFile(filename);
expect(stderr).toMatch('Snapshots: 1 file removed, 1 passed, 1 total');
expect(status).toBe(0);
expect(fileAfter).toMatchSnapshot('external snapshot cleaned');
expect(fs.existsSync(snapshotPath)).toEqual(false);
}
});

test('supports async matchers', () => {
const filename = 'async-matchers.test.js';
const test = `
Expand Down
7 changes: 6 additions & 1 deletion packages/jest-snapshot/src/State.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,12 @@ export default class SnapshotState {
key = testNameToKey(testName, count);
}

this._uncheckedKeys.delete(key);
// Do not mark the snapshot as "checked" if the snapshot is inline and
// there's an external snapshot. This way the external snapshot can be
// removed with `--updateSnapshot`.
if (!(isInline && this._snapshotData[key])) {
this._uncheckedKeys.delete(key);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could potentially add a check that fails the test if the inline snapshot doesn't match the external one. Thoughts?

Copy link
Member

@rickhanlonii rickhanlonii Jul 28, 2018

Choose a reason for hiding this comment

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

When a test name changes we write the new snapshot and mark the the old for removal so I think doing the same (without failing) for moving to the inline makes sense. Besides - how would you know that they changed the matcher fn instead of removing the old fn and snapshotting something else in line?


const receivedSerialized = serialize(received);
const expected = isInline ? inlineSnapshot : this._snapshotData[key];
Expand Down