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

Correctly merge property matchers with the rest of the snapshot in toMatchSnapshot. #6528

Merged
merged 12 commits into from
Aug 9, 2018
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- `[jest-cli]` Don't report promises as open handles ([#6716](https://github.com/facebook/jest/pull/6716))
- `[jest-each]` Add timeout support to parameterised tests ([#6660](https://github.com/facebook/jest/pull/6660))
- `[jest-cli]` Improve the message when running coverage while there are no files matching global threshold ([#6334](https://github.com/facebook/jest/pull/6334))
- `[jest-snapshot]` Correctly merge property matchers with the rest of the snapshot in `toMatchSnapshot`. ([#6528](https://github.com/facebook/jest/pull/6528))
- `[jest-snapshot]` Add error messages for invalid property matchers. ([#6528](https://github.com/facebook/jest/pull/6528))

## 23.4.2

Expand Down
67 changes: 61 additions & 6 deletions e2e/__tests__/to_match_snapshot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,49 @@ test('handles property matchers', () => {
}
});

test('handles invalid property matchers', () => {
const filename = 'handle-property-matchers.test.js';
{
writeFiles(TESTS_DIR, {
[filename]: `test('invalid property matchers', () => {
expect({foo: 'bar'}).toMatchSnapshot(null);
});
`,
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Property matchers must be an object.');
expect(status).toBe(1);
}
{
writeFiles(TESTS_DIR, {
[filename]: `test('invalid property matchers', () => {
expect({foo: 'bar'}).toMatchSnapshot(null, 'test-name');
});
`,
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Property matchers must be an object.');
expect(stderr).toMatch(
'To provide a snapshot test name without property matchers, use: toMatchSnapshot("name")',
);
expect(status).toBe(1);
}
{
writeFiles(TESTS_DIR, {
[filename]: `test('invalid property matchers', () => {
expect({foo: 'bar'}).toMatchSnapshot(undefined, 'test-name');
});
`,
});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('Property matchers must be an object.');
expect(stderr).toMatch(
'To provide a snapshot test name without property matchers, use: toMatchSnapshot("name")',
);
expect(status).toBe(1);
}
});

test('handles property matchers with custom name', () => {
const filename = 'handle-property-matchers-with-name.test.js';
const template = makeTemplate(`test('handles property matchers with name', () => {
Expand All @@ -217,20 +260,21 @@ test('handles property matchers with custom name', () => {
expect(stderr).toMatch(
'Received value does not match snapshot properties for "handles property matchers with name: custom-name 1".',
);
expect(stderr).toMatch('Expected snapshot to match properties:');
expect(stderr).toMatch('Snapshots: 1 failed, 1 total');
expect(status).toBe(1);
}
});

test('handles property matchers with deep expect.objectContaining', () => {
test('handles property matchers with deep properties', () => {
const filename = 'handle-property-matchers-with-name.test.js';
const template = makeTemplate(`test('handles property matchers with deep expect.objectContaining', () => {
expect({ user: { createdAt: $1, name: 'Jest' }}).toMatchSnapshot({ user: expect.objectContaining({ createdAt: expect.any(Date) }) });
const template = makeTemplate(`test('handles property matchers with deep properties', () => {
expect({ user: { createdAt: $1, name: $2 }}).toMatchSnapshot({ user: { createdAt: expect.any(Date), name: $2 }});
});
`);

{
writeFiles(TESTS_DIR, {[filename]: template(['new Date()'])});
writeFiles(TESTS_DIR, {[filename]: template(['new Date()', '"Jest"'])});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch('1 snapshot written from 1 test suite.');
expect(status).toBe(0);
Expand All @@ -243,10 +287,21 @@ test('handles property matchers with deep expect.objectContaining', () => {
}

{
writeFiles(TESTS_DIR, {[filename]: template(['"string"'])});
writeFiles(TESTS_DIR, {[filename]: template(['"string"', '"Jest"'])});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch(
'Received value does not match snapshot properties for "handles property matchers with deep properties 1".',
);
expect(stderr).toMatch('Expected snapshot to match properties:');
expect(stderr).toMatch('Snapshots: 1 failed, 1 total');
expect(status).toBe(1);
}

{
writeFiles(TESTS_DIR, {[filename]: template(['new Date()', '"CHANGED"'])});
const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
expect(stderr).toMatch(
'Received value does not match snapshot properties for "handles property matchers with deep expect.objectContaining 1".',
'Received value does not match stored snapshot "handles property matchers with deep properties 1"',
);
expect(stderr).toMatch('Snapshots: 1 failed, 1 total');
expect(status).toBe(1);
Expand Down
13 changes: 13 additions & 0 deletions packages/jest-snapshot/src/__tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const {
saveSnapshotFile,
serialize,
testNameToKey,
deepMerge,
SNAPSHOT_GUIDE_LINK,
SNAPSHOT_VERSION,
SNAPSHOT_VERSION_WARNING,
Expand Down Expand Up @@ -198,3 +199,15 @@ test('serialize handles \\r\\n', () => {

expect(serializedData).toBe('\n"<div>\n</div>"\n');
});

describe('DeepMerge', () => {
it('Correctly merges objects with property matchers', () => {
const target = {data: {bar: 'bar', foo: 'foo'}};
const matcher = expect.any(String);
const propertyMatchers = {data: {foo: matcher}};
const mergedOutput = deepMerge(target, propertyMatchers);

expect(mergedOutput).toStrictEqual({data: {bar: 'bar', foo: matcher}});
expect(target).toStrictEqual({data: {bar: 'bar', foo: 'foo'}});
Copy link
Member

Choose a reason for hiding this comment

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

@swcisel I updated your tests here, does this make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks good to me. thanks!

});
});
11 changes: 10 additions & 1 deletion packages/jest-snapshot/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ const toMatchSnapshot = function(
propertyMatchers?: any,
testName?: string,
) {
if (arguments.length === 3 && !propertyMatchers) {
throw new Error(
'Property matchers must be an object.\n\nTo provide a snapshot test name without property matchers, use: toMatchSnapshot("name")',
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Added the checks here to close #6581


return _toMatchSnapshot({
context: this,
propertyMatchers,
Expand Down Expand Up @@ -118,6 +124,9 @@ const _toMatchSnapshot = ({
: currentTestName || '';

if (typeof propertyMatchers === 'object') {
if (propertyMatchers === null) {
throw new Error(`Property matchers must be an object.`);
}
const propertyPass = context.equals(received, propertyMatchers, [
context.utils.iterableEquality,
context.utils.subsetEquality,
Expand All @@ -144,7 +153,7 @@ const _toMatchSnapshot = ({
report,
};
} else {
Object.assign(received, propertyMatchers);
received = utils.deepMerge(received, propertyMatchers);
}
}

Expand Down
19 changes: 19 additions & 0 deletions packages/jest-snapshot/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ const validateSnapshotVersion = (snapshotContents: string) => {
return null;
};

function isObject(item) {
return item && typeof item === 'object' && !Array.isArray(item);
}

export const testNameToKey = (testName: string, count: number) =>
testName + ' ' + count;

Expand Down Expand Up @@ -180,3 +184,18 @@ export const saveSnapshotFile = (
writeSnapshotVersion() + '\n\n' + snapshots.join('\n\n') + '\n',
);
};

export const deepMerge = (target: any, source: any) => {
const mergedOutput = Object.assign({}, target);
if (isObject(target) && isObject(source)) {
Object.keys(source).forEach(key => {
if (isObject(source[key]) && !source[key].$$typeof) {
if (!(key in target)) Object.assign(mergedOutput, {[key]: source[key]});
else mergedOutput[key] = deepMerge(target[key], source[key]);
} else {
Object.assign(mergedOutput, {[key]: source[key]});
}
});
}
return mergedOutput;
};