Correctly merge property matchers with the rest of the snapshot in `t…

…oMatchSnapshot`. (#6528)
jestjs · Aug 9, 2018 · 3c3f9f2 · 3c3f9f2
1 parent f8f966b
commit 3c3f9f2
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 

diff --git a/e2e/__tests__/to_match_snapshot.test.js b/e2e/__tests__/to_match_snapshot.test.js
@@ -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', () => {
@@ -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);
@@ -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);

diff --git a/packages/jest-snapshot/src/__tests__/utils.test.js b/packages/jest-snapshot/src/__tests__/utils.test.js
@@ -18,6 +18,7 @@ const {
   saveSnapshotFile,
   serialize,
   testNameToKey,
+  deepMerge,
   SNAPSHOT_GUIDE_LINK,
   SNAPSHOT_VERSION,
   SNAPSHOT_VERSION_WARNING,
@@ -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'}});
+  });
+});
diff --git a/packages/jest-snapshot/src/index.js b/packages/jest-snapshot/src/index.js
@@ -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")',
+    );
+  }
+
   return _toMatchSnapshot({
     context: this,
     propertyMatchers,
@@ -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,
@@ -144,7 +153,7 @@ const _toMatchSnapshot = ({
         report,
       };
     } else {
-      Object.assign(received, propertyMatchers);
+      received = utils.deepMerge(received, propertyMatchers);
     }
   }
 

diff --git a/packages/jest-snapshot/src/utils.js b/packages/jest-snapshot/src/utils.js
@@ -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;
 
@@ -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;
+};