From 6534d09784dd73c851c00baab23da5edcf0cefcd Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 2 Oct 2018 12:57:10 -0400 Subject: [PATCH 1/9] add support for arrays when merging snapshots --- .../jest-snapshot/src/__tests__/utils.test.js | 53 +++++++++++++++++-- packages/jest-snapshot/src/utils.js | 9 ++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.js b/packages/jest-snapshot/src/__tests__/utils.test.js index ea6d2191f430..6a9ca6b3eefc 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.js +++ b/packages/jest-snapshot/src/__tests__/utils.test.js @@ -195,12 +195,57 @@ test('serialize handles \\r\\n', () => { describe('DeepMerge', () => { it('Correctly merges objects with property matchers', () => { - const target = {data: {bar: 'bar', foo: 'foo'}}; + /* eslint-disable sort-keys */ + // to keep keys in numerical order rather than alphabetical + const target = { + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + ], + }, + }; const matcher = expect.any(String); - const propertyMatchers = {data: {foo: matcher}}; + const propertyMatchers = { + data: { + two: matcher, + three: [ + { + four: matcher, + }, + ], + }, + }; const mergedOutput = deepMerge(target, propertyMatchers); - expect(mergedOutput).toStrictEqual({data: {bar: 'bar', foo: matcher}}); - expect(target).toStrictEqual({data: {bar: 'bar', foo: 'foo'}}); + expect(mergedOutput).toStrictEqual({ + data: { + one: 'one', + two: matcher, + three: [ + { + four: matcher, + five: 'five', + }, + ], + }, + }); + expect(target).toStrictEqual({ + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + ], + }, + }); + /* eslint-enable sort-keys */ }); }); diff --git a/packages/jest-snapshot/src/utils.js b/packages/jest-snapshot/src/utils.js index 13f7b76a4465..65461796c897 100644 --- a/packages/jest-snapshot/src/utils.js +++ b/packages/jest-snapshot/src/utils.js @@ -185,6 +185,15 @@ export const deepMerge = (target: any, source: any) => { 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 if (Array.isArray(source[key])) { + // Convert arrays to objects, merge, convert back to array + const mergedArrayObject = deepMerge( + Object.assign({}, target[key]), + Object.assign({}, source[key]), + ); + mergedOutput[key] = Object.keys(mergedArrayObject).map( + arrKey => mergedArrayObject[arrKey], + ); } else { Object.assign(mergedOutput, {[key]: source[key]}); } From 4fb1c1a8046188707f7e85609933420edff0c482 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 2 Oct 2018 13:07:50 -0400 Subject: [PATCH 2/9] update README --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 925911127054..17ad3f1e8002 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ - `[jest-haste-map]` Remove legacy condition for duplicate module detection ([#7333](https://github.com/facebook/jest/pull/7333)) - `[jest-haste-map]` Fix `require` detection with trailing commas and ignore `import typeof` modules ([#7385](https://github.com/facebook/jest/pull/7385)) - `[jest-cli]` Fix to set prettierPath via config file ([#7412](https://github.com/facebook/jest/pull/7412)) +- `[jest-snapshot]` Handle arrays when merging snapshots ([#7089](https://github.com/facebook/jest/pull/7089)) ### Chore & Maintenance From 079fa50738093948540013dce9f6934d6d63d3f2 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 3 Dec 2018 14:45:36 -0500 Subject: [PATCH 3/9] compare array by keys without converting to object --- .../jest-snapshot/src/__tests__/utils.test.js | 21 +++++++++++++-- packages/jest-snapshot/src/utils.js | 26 +++++++++++++------ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.js b/packages/jest-snapshot/src/__tests__/utils.test.js index 6a9ca6b3eefc..b82615f42bee 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.js +++ b/packages/jest-snapshot/src/__tests__/utils.test.js @@ -206,10 +206,17 @@ describe('DeepMerge', () => { four: 'four', five: 'five', }, + // Include an array element not present in the propertyMatchers + { + six: 'six', + seven: 'seven', + }, ], }, }; - const matcher = expect.any(String); + // Don't use `expect.any(string)` since that will cause a false positive + // if deepMerge incorrectly keeps two as 'two' from the target + const matcher = '--matcher--'; const propertyMatchers = { data: { two: matcher, @@ -220,8 +227,8 @@ describe('DeepMerge', () => { ], }, }; - const mergedOutput = deepMerge(target, propertyMatchers); + const mergedOutput = deepMerge(target, propertyMatchers); expect(mergedOutput).toStrictEqual({ data: { one: 'one', @@ -231,9 +238,15 @@ describe('DeepMerge', () => { four: matcher, five: 'five', }, + { + six: 'six', + seven: 'seven', + }, ], }, }); + + // Ensure original target is not modified expect(target).toStrictEqual({ data: { one: 'one', @@ -243,6 +256,10 @@ describe('DeepMerge', () => { four: 'four', five: 'five', }, + { + six: 'six', + seven: 'seven', + }, ], }, }); diff --git a/packages/jest-snapshot/src/utils.js b/packages/jest-snapshot/src/utils.js index 65461796c897..c5686ac24baa 100644 --- a/packages/jest-snapshot/src/utils.js +++ b/packages/jest-snapshot/src/utils.js @@ -178,6 +178,23 @@ export const saveSnapshotFile = ( ); }; +const deepMergeArray = (target, source) => { + // Clone target + const mergedOutput = target.slice(); + + source.forEach((element, index) => { + if (typeof mergedOutput[index] === 'undefined') { + mergedOutput[index] = element; + } else if (typeof element === 'undefined') { + mergedOutput.push(target[index]); + } else { + mergedOutput[index] = deepMerge(target[index], element); + } + }); + + return mergedOutput; +}; + export const deepMerge = (target: any, source: any) => { const mergedOutput = Object.assign({}, target); if (isObject(target) && isObject(source)) { @@ -186,14 +203,7 @@ export const deepMerge = (target: any, source: any) => { if (!(key in target)) Object.assign(mergedOutput, {[key]: source[key]}); else mergedOutput[key] = deepMerge(target[key], source[key]); } else if (Array.isArray(source[key])) { - // Convert arrays to objects, merge, convert back to array - const mergedArrayObject = deepMerge( - Object.assign({}, target[key]), - Object.assign({}, source[key]), - ); - mergedOutput[key] = Object.keys(mergedArrayObject).map( - arrKey => mergedArrayObject[arrKey], - ); + mergedOutput[key] = deepMergeArray(target[key], source[key]); } else { Object.assign(mergedOutput, {[key]: source[key]}); } From 7317c0da9acbab73658f69e41a4c1fefae1d809c Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 2 Oct 2018 12:57:10 -0400 Subject: [PATCH 4/9] add support for arrays when merging snapshots --- .../jest-snapshot/src/__tests__/utils.test.ts | 53 +++++++++++++++++-- packages/jest-snapshot/src/utils.ts | 9 ++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index 981070691777..a7adeb41a782 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -201,12 +201,57 @@ test('serialize handles \\r\\n', () => { describe('DeepMerge', () => { it('Correctly merges objects with property matchers', () => { - const target = {data: {bar: 'bar', foo: 'foo'}}; + /* eslint-disable sort-keys */ + // to keep keys in numerical order rather than alphabetical + const target = { + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + ], + }, + }; const matcher = expect.any(String); - const propertyMatchers = {data: {foo: matcher}}; + const propertyMatchers = { + data: { + two: matcher, + three: [ + { + four: matcher, + }, + ], + }, + }; const mergedOutput = deepMerge(target, propertyMatchers); - expect(mergedOutput).toStrictEqual({data: {bar: 'bar', foo: matcher}}); - expect(target).toStrictEqual({data: {bar: 'bar', foo: 'foo'}}); + expect(mergedOutput).toStrictEqual({ + data: { + one: 'one', + two: matcher, + three: [ + { + four: matcher, + five: 'five', + }, + ], + }, + }); + expect(target).toStrictEqual({ + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + ], + }, + }); + /* eslint-enable sort-keys */ }); }); diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index 992fa72926d0..cfb9360b3a16 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -185,6 +185,15 @@ export const deepMerge = (target: any, source: any) => { 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 if (Array.isArray(source[key])) { + // Convert arrays to objects, merge, convert back to array + const mergedArrayObject = deepMerge( + Object.assign({}, target[key]), + Object.assign({}, source[key]), + ); + mergedOutput[key] = Object.keys(mergedArrayObject).map( + arrKey => mergedArrayObject[arrKey], + ); } else { Object.assign(mergedOutput, {[key]: source[key]}); } From ac1974cdfea0c8be98d84dfaed5e86af7e1fcce9 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 2 Oct 2018 13:07:50 -0400 Subject: [PATCH 5/9] update README --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4d5fe700a5b..63707a66008c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - `[jest-haste-map]` Fix the `mapper` option which was incorrectly ignored ([#8299](https://github.com/facebook/jest/pull/8299)) - `[jest-jasmine2]` Fix describe return value warning being shown if the describe function throws ([#8335](https://github.com/facebook/jest/pull/8335)) - `[jest-environment-jsdom]` Re-declare global prototype of JSDOMEnvironment ([#8352](https://github.com/facebook/jest/pull/8352)) +- `[jest-snapshot]` Handle arrays when merging snapshots ([#7089](https://github.com/facebook/jest/pull/7089)) ### Chore & Maintenance From 1942422dbad819313042506cbde5cac6bf1a274e Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 3 Dec 2018 14:45:36 -0500 Subject: [PATCH 6/9] compare array by keys without converting to object --- .../jest-snapshot/src/__tests__/utils.test.ts | 21 +++++++++++++-- packages/jest-snapshot/src/utils.ts | 26 +++++++++++++------ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index a7adeb41a782..631f1fc45dfd 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -212,10 +212,17 @@ describe('DeepMerge', () => { four: 'four', five: 'five', }, + // Include an array element not present in the propertyMatchers + { + six: 'six', + seven: 'seven', + }, ], }, }; - const matcher = expect.any(String); + // Don't use `expect.any(string)` since that will cause a false positive + // if deepMerge incorrectly keeps two as 'two' from the target + const matcher = '--matcher--'; const propertyMatchers = { data: { two: matcher, @@ -226,8 +233,8 @@ describe('DeepMerge', () => { ], }, }; - const mergedOutput = deepMerge(target, propertyMatchers); + const mergedOutput = deepMerge(target, propertyMatchers); expect(mergedOutput).toStrictEqual({ data: { one: 'one', @@ -237,9 +244,15 @@ describe('DeepMerge', () => { four: matcher, five: 'five', }, + { + six: 'six', + seven: 'seven', + }, ], }, }); + + // Ensure original target is not modified expect(target).toStrictEqual({ data: { one: 'one', @@ -249,6 +262,10 @@ describe('DeepMerge', () => { four: 'four', five: 'five', }, + { + six: 'six', + seven: 'seven', + }, ], }, }); diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index cfb9360b3a16..386f4856f900 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -178,6 +178,23 @@ export const saveSnapshotFile = ( ); }; +const deepMergeArray = (target, source) => { + // Clone target + const mergedOutput = target.slice(); + + source.forEach((element, index) => { + if (typeof mergedOutput[index] === 'undefined') { + mergedOutput[index] = element; + } else if (typeof element === 'undefined') { + mergedOutput.push(target[index]); + } else { + mergedOutput[index] = deepMerge(target[index], element); + } + }); + + return mergedOutput; +}; + export const deepMerge = (target: any, source: any) => { const mergedOutput = {...target}; if (isObject(target) && isObject(source)) { @@ -186,14 +203,7 @@ export const deepMerge = (target: any, source: any) => { if (!(key in target)) Object.assign(mergedOutput, {[key]: source[key]}); else mergedOutput[key] = deepMerge(target[key], source[key]); } else if (Array.isArray(source[key])) { - // Convert arrays to objects, merge, convert back to array - const mergedArrayObject = deepMerge( - Object.assign({}, target[key]), - Object.assign({}, source[key]), - ); - mergedOutput[key] = Object.keys(mergedArrayObject).map( - arrKey => mergedArrayObject[arrKey], - ); + mergedOutput[key] = deepMergeArray(target[key], source[key]); } else { Object.assign(mergedOutput, {[key]: source[key]}); } From a22721f43118fbb9c8e88c056f8b1ac7cb3e1add Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 22 Apr 2019 12:10:43 -0400 Subject: [PATCH 7/9] jest-snapshot: fix ts errors in utils --- CHANGELOG.md | 1 - packages/jest-snapshot/src/utils.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44aa4974a708..63707a66008c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -415,7 +415,6 @@ We skipped 24.2.0 because a draft was accidentally published. Please use `24.3.0 - `[jest-runtime]` Pass `watchPathIgnorePatterns` to Haste instance ([#7585](https://github.com/facebook/jest/pull/7585)) - `[jest-runtime]` Resolve mock files via Haste when using `require.resolve` ([#7687](https://github.com/facebook/jest/pull/7687)) - ### Chore & Maintenance - `[*]` [**BREAKING**] Require Node.js 6+ for all packages ([#7258](https://github.com/facebook/jest/pull/7258)) diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index 386f4856f900..dc6587afff7b 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -178,7 +178,7 @@ export const saveSnapshotFile = ( ); }; -const deepMergeArray = (target, source) => { +const deepMergeArray = (target: Array, source: Array) => { // Clone target const mergedOutput = target.slice(); From 1dada661af6b71607efc877cc66e3a77a932dd5e Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 22 Apr 2019 12:22:02 -0400 Subject: [PATCH 8/9] jest-snapshot: use assert.deepStrictEqual to allow expect.any(String) instead of a dummy matcher --- packages/jest-snapshot/src/__tests__/utils.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index 631f1fc45dfd..6d89ff96fd38 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -10,6 +10,7 @@ jest.mock('fs'); import fs from 'fs'; import path from 'path'; import chalk from 'chalk'; +import assert from 'assert'; import { deepMerge, @@ -220,9 +221,8 @@ describe('DeepMerge', () => { ], }, }; - // Don't use `expect.any(string)` since that will cause a false positive - // if deepMerge incorrectly keeps two as 'two' from the target - const matcher = '--matcher--'; + + const matcher = expect.any(String); const propertyMatchers = { data: { two: matcher, @@ -235,7 +235,11 @@ describe('DeepMerge', () => { }; const mergedOutput = deepMerge(target, propertyMatchers); - expect(mergedOutput).toStrictEqual({ + + // Use assert.deepStrictEqual() instead of expect().toStrictEqual() + // since we want to actually validate that we got the matcher + // rather than treat it specially the way that expect() does + assert.deepStrictEqual(mergedOutput, { data: { one: 'one', two: matcher, From 092635722069835aca08033ee5bff4574d9384bd Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 22 Apr 2019 15:10:25 -0400 Subject: [PATCH 9/9] jest-snapshot: add test for more array elements in source than target --- packages/jest-snapshot/src/__tests__/utils.test.ts | 10 +++++++++- packages/jest-snapshot/src/utils.ts | 2 -- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index 6d89ff96fd38..f28efd12128d 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -9,8 +9,8 @@ jest.mock('fs'); import fs from 'fs'; import path from 'path'; -import chalk from 'chalk'; import assert from 'assert'; +import chalk from 'chalk'; import { deepMerge, @@ -219,6 +219,7 @@ describe('DeepMerge', () => { seven: 'seven', }, ], + eight: [{nine: 'nine'}], }, }; @@ -231,6 +232,11 @@ describe('DeepMerge', () => { four: matcher, }, ], + eight: [ + {nine: matcher}, + // Include an array element not present in the target + {ten: matcher}, + ], }, }; @@ -253,6 +259,7 @@ describe('DeepMerge', () => { seven: 'seven', }, ], + eight: [{nine: matcher}, {ten: matcher}], }, }); @@ -271,6 +278,7 @@ describe('DeepMerge', () => { seven: 'seven', }, ], + eight: [{nine: 'nine'}], }, }); /* eslint-enable sort-keys */ diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index dc6587afff7b..d0e4b20eb049 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -185,8 +185,6 @@ const deepMergeArray = (target: Array, source: Array) => { source.forEach((element, index) => { if (typeof mergedOutput[index] === 'undefined') { mergedOutput[index] = element; - } else if (typeof element === 'undefined') { - mergedOutput.push(target[index]); } else { mergedOutput[index] = deepMerge(target[index], element); }