diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cf5560c5db9..fa8546a16c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### Features ### Fixes + +- `[jest-matcher-utils]` Replace accessors with values to avoid calling setters in object descriptors when computing diffs for error reporting ([#9757](https://github.com/facebook/jest/pull/9757)) - `[@jest/watcher]` Correct return type of `shouldRunTestSuite` for `JestHookEmitter` ([#9753](https://github.com/facebook/jest/pull/9753)) ### Chore & Maintenance diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index 4da9c13f4d09..bc1abc19adf6 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -2127,6 +2127,76 @@ exports[`.toEqual() {pass: false} expect({"foo": {"bar": 1}}).toEqual({"foo": {} } `; +exports[`.toEqual() {pass: false} expect({"frozenGetter": {}}).toEqual({"frozenGetter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "frozenGetter": Object { +- "foo": "bar", +- }, ++ "frozenGetter": Object {}, + } +`; + +exports[`.toEqual() {pass: false} expect({"frozenGetterAndSetter": {}}).toEqual({"frozenGetterAndSetter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "frozenGetterAndSetter": Object { +- "foo": "bar", +- }, ++ "frozenGetterAndSetter": Object {}, + } +`; + +exports[`.toEqual() {pass: false} expect({"frozenSetter": undefined}).toEqual({"frozenSetter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "frozenSetter": Object { +- "foo": "bar", +- }, ++ "frozenSetter": undefined, + } +`; + +exports[`.toEqual() {pass: false} expect({"getter": {}}).toEqual({"getter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "getter": Object { +- "foo": "bar", +- }, ++ "getter": Object {}, + } +`; + +exports[`.toEqual() {pass: false} expect({"getterAndSetter": {}}).toEqual({"getterAndSetter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "getterAndSetter": Object { +- "foo": "bar", +- }, ++ "getterAndSetter": Object {}, + } +`; + exports[`.toEqual() {pass: false} expect({"nodeName": "div", "nodeType": 1}).toEqual({"nodeName": "p", "nodeType": 1}) 1`] = ` expect(received).toEqual(expected) // deep equality @@ -2140,6 +2210,20 @@ exports[`.toEqual() {pass: false} expect({"nodeName": "div", "nodeType": 1}).toE } `; +exports[`.toEqual() {pass: false} expect({"setter": undefined}).toEqual({"setter": {"foo": "bar"}}) 1`] = ` +expect(received).toEqual(expected) // deep equality + +- Expected - 3 ++ Received + 1 + + Object { +- "setter": Object { +- "foo": "bar", +- }, ++ "setter": undefined, + } +`; + exports[`.toEqual() {pass: false} expect({"target": {"nodeType": 1, "value": "a"}}).toEqual({"target": {"nodeType": 1, "value": "b"}}) 1`] = ` expect(received).toEqual(expected) // deep equality diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 7bb6676b2b24..8d1ec4fbea8c 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -435,6 +435,62 @@ describe('.toEqual()', () => { [{a: 1}, {a: 2}], [{a: 5}, {b: 6}], [Object.freeze({foo: {bar: 1}}), {foo: {}}], + [ + { + get getterAndSetter() { + return {}; + }, + set getterAndSetter(value) { + throw new Error('noo'); + }, + }, + {getterAndSetter: {foo: 'bar'}}, + ], + [ + Object.freeze({ + get frozenGetterAndSetter() { + return {}; + }, + set frozenGetterAndSetter(value) { + throw new Error('noo'); + }, + }), + {frozenGetterAndSetter: {foo: 'bar'}}, + ], + [ + { + get getter() { + return {}; + }, + }, + {getter: {foo: 'bar'}}, + ], + [ + Object.freeze({ + get frozenGetter() { + return {}; + }, + }), + {frozenGetter: {foo: 'bar'}}, + ], + [ + { + // eslint-disable-next-line accessor-pairs + set setter(value) { + throw new Error('noo'); + }, + }, + {setter: {foo: 'bar'}}, + ], + [ + Object.freeze({ + // eslint-disable-next-line accessor-pairs + set frozenSetter(value) { + throw new Error('noo'); + }, + }), + {frozenSetter: {foo: 'bar'}}, + ], ['banana', 'apple'], ['1\u{00A0}234,57\u{00A0}$', '1 234,57 $'], // issues/6881 [ diff --git a/packages/jest-matcher-utils/src/Replaceable.ts b/packages/jest-matcher-utils/src/Replaceable.ts index 6d1e8a2776c1..8f518fef1494 100644 --- a/packages/jest-matcher-utils/src/Replaceable.ts +++ b/packages/jest-matcher-utils/src/Replaceable.ts @@ -36,10 +36,7 @@ export default class Replaceable { cb(value, key, this.object); }); Object.getOwnPropertySymbols(this.object).forEach(key => { - const descriptor = Object.getOwnPropertyDescriptor(this.object, key); - if ((descriptor as PropertyDescriptor).enumerable) { - cb(this.object[key], key, this.object); - } + cb(this.object[key], key, this.object); }); } else { this.object.forEach(cb); diff --git a/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts b/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts index aa724bfe968e..890f1ba38b96 100644 --- a/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts +++ b/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts @@ -104,28 +104,17 @@ describe('Replaceable', () => { const replaceable = new Replaceable(object); const cb = jest.fn(); replaceable.forEach(cb); - expect(cb.mock.calls.length).toBe(3); + expect(cb).toHaveBeenCalledTimes(3); expect(cb.mock.calls[0]).toEqual([1, 'a', object]); expect(cb.mock.calls[1]).toEqual([2, 'b', object]); expect(cb.mock.calls[2]).toEqual([3, symbolKey, object]); }); - test('object forEach do not iterate none enumerable symbol key', () => { - const symbolKey = Symbol('jest'); - const object = {a: 1, b: 2}; - Object.defineProperty(object, symbolKey, {enumerable: false}); - const replaceable = new Replaceable(object); - const cb = jest.fn(); - replaceable.forEach(cb); - expect(cb.mock.calls.length).toBe(2); - expect(cb.mock.calls[0]).toEqual([1, 'a', object]); - expect(cb.mock.calls[1]).toEqual([2, 'b', object]); - }); - test('array forEach', () => { const replaceable = new Replaceable([1, 2, 3]); const cb = jest.fn(); replaceable.forEach(cb); + expect(cb).toHaveBeenCalledTimes(3); expect(cb.mock.calls[0]).toEqual([1, 0, [1, 2, 3]]); expect(cb.mock.calls[1]).toEqual([2, 1, [1, 2, 3]]); expect(cb.mock.calls[2]).toEqual([3, 2, [1, 2, 3]]); @@ -139,6 +128,7 @@ describe('Replaceable', () => { const replaceable = new Replaceable(map); const cb = jest.fn(); replaceable.forEach(cb); + expect(cb).toHaveBeenCalledTimes(2); expect(cb.mock.calls[0]).toEqual([1, 'a', map]); expect(cb.mock.calls[1]).toEqual([2, 'b', map]); }); diff --git a/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts index 3063db1f76f7..63af3a7dce3d 100644 --- a/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts +++ b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts @@ -20,18 +20,36 @@ test('returns the same value for primitive or function values', () => { expect(deepCyclicCopyReplaceable(fn)).toBe(fn); }); -test('does not execute getters/setters, but copies them', () => { - const fn = jest.fn(); +test('convert accessor descriptor into value descriptor', () => { const obj = { - // @ts-ignore + set foo(_) {}, get foo() { - fn(); + return 'bar'; }, }; + expect(Object.getOwnPropertyDescriptor(obj, 'foo')).toEqual({ + configurable: true, + enumerable: true, + get: expect.any(Function), + set: expect.any(Function), + }); const copy = deepCyclicCopyReplaceable(obj); - expect(Object.getOwnPropertyDescriptor(copy, 'foo')).toBeDefined(); - expect(fn).not.toBeCalled(); + expect(Object.getOwnPropertyDescriptor(copy, 'foo')).toEqual({ + configurable: true, + enumerable: true, + value: 'bar', + writable: true, + }); +}); + +test('skips non-enumerables', () => { + const obj = {}; + Object.defineProperty(obj, 'foo', {enumerable: false, value: 'bar'}); + + const copy = deepCyclicCopyReplaceable(obj); + + expect(Object.getOwnPropertyDescriptors(copy)).toEqual({}); }); test('copies symbols', () => { diff --git a/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts b/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts index 59f5e160b600..6849735a7f2c 100644 --- a/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts +++ b/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts @@ -49,21 +49,28 @@ export default function deepCyclicCopyReplaceable( function deepCyclicCopyObject(object: T, cycles: WeakMap): T { const newObject = Object.create(Object.getPrototypeOf(object)); - const descriptors = Object.getOwnPropertyDescriptors(object); + const descriptors: { + [x: string]: PropertyDescriptor; + } = Object.getOwnPropertyDescriptors(object); cycles.set(object, newObject); Object.keys(descriptors).forEach(key => { - const descriptor = descriptors[key]; - if (typeof descriptor.value !== 'undefined') { - descriptor.value = deepCyclicCopyReplaceable(descriptor.value, cycles); + if (descriptors[key].enumerable) { + descriptors[key] = { + configurable: true, + enumerable: true, + value: deepCyclicCopyReplaceable( + // this accesses the value or getter, depending. We just care about the value anyways, and this allows us to not mess with accessors + // it has the side effect of invoking the getter here though, rather than copying it over + (object as Record)[key], + cycles, + ), + writable: true, + }; + } else { + delete descriptors[key]; } - - if (!('set' in descriptor)) { - descriptor.writable = true; - } - - descriptor.configurable = true; }); return Object.defineProperties(newObject, descriptors);