Skip to content

Commit

Permalink
fix: toMatchObject works with super getters (#10381)
Browse files Browse the repository at this point in the history
  • Loading branch information
souldzin committed Aug 22, 2020
1 parent a8addf8 commit 5c6480b
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 64 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,8 @@

### Fixes

- `[expect]` Fix `toMatchObject` to work with inherited class getters ([#10381](https://github.com/facebook/jest/pull/10381))

### Chore & Maintenance

### Performance
Expand Down
33 changes: 33 additions & 0 deletions packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
Expand Up @@ -3988,6 +3988,18 @@ exports[`toMatchObject() {pass: false} expect({"a": "b"}).toMatchObject({"c": "d
<d> }</>
`;

exports[`toMatchObject() {pass: false} expect({"a": "b"}).toMatchObject({"toString": Any<Function>}) 1`] = `
<d>expect(</><r>received</><d>).</>toMatchObject<d>(</><g>expected</><d>)</>

<g>- Expected - 0</>
<r>+ Received + 1</>

<d> Object {</>
<r>+ "a": "b",</>
<d> "toString": Any<Function>,</>
<d> }</>
`;

exports[`toMatchObject() {pass: false} expect({"a": [{"a": "a", "b": "b"}]}).toMatchObject({"a": [{"a": "c"}]}) 1`] = `
<d>expect(</><r>received</><d>).</>toMatchObject<d>(</><g>expected</><d>)</>

Expand Down Expand Up @@ -4246,6 +4258,13 @@ Expected: not <g>{"t": {"x": {"r": "r"}}}</>
Received: <r>{"a": "b", "t": {"x": {"r": "r"}, "z": "z"}}</>
`;

exports[`toMatchObject() {pass: true} expect({"a": "b", "toString": [Function toString]}).toMatchObject({"toString": Any<Function>}) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expected: not <g>{"toString": Any<Function>}</>
Received: <r>{"a": "b", "toString": [Function toString]}</>
`;

exports[`toMatchObject() {pass: true} expect({"a": "b"}).toMatchObject({"a": "b"}) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expand Down Expand Up @@ -4314,13 +4333,27 @@ exports[`toMatchObject() {pass: true} expect({"a": undefined}).toMatchObject({"a
Expected: not <g>{"a": undefined}</>
`;

exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"a": undefined, "b": "b", "c": "c"}) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expected: not <g>{"a": undefined, "b": "b", "c": "c"}</>
Received: <r>{}</>
`;

exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"a": undefined, "b": "b"}) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expected: not <g>{"a": undefined, "b": "b"}</>
Received: <r>{}</>
`;

exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"d": 4}) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expected: not <g>{"d": 4}</>
Received: <r>{}</>
`;

exports[`toMatchObject() {pass: true} expect(2015-11-30T00:00:00.000Z).toMatchObject(2015-11-30T00:00:00.000Z) 1`] = `
<d>expect(</><r>received</><d>).</>not<d>.</>toMatchObject<d>(</><g>expected</><d>)</>

Expand Down
23 changes: 23 additions & 0 deletions packages/expect/src/__tests__/matchers.test.js
Expand Up @@ -1958,6 +1958,22 @@ describe('toMatchObject()', () => {
}
}

class Sub extends Foo {
get c() {
return 'c';
}
}

const withDefineProperty = (obj, key, val) => {
Object.defineProperty(obj, key, {
get() {
return val;
},
});

return obj;
};

const testNotToMatchSnapshots = tuples => {
tuples.forEach(([n1, n2]) => {
it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify(
Expand Down Expand Up @@ -2082,6 +2098,12 @@ describe('toMatchObject()', () => {
{a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'},
{a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'},
],
// These snapshots will show {} as the object because the properties
// are not enumerable. We will need to somehow make the serialization of
// these keys a little smarter before reporting accurately.
[new Sub(), {a: undefined, b: 'b', c: 'c'}],
[withDefineProperty(new Sub(), 'd', 4), {d: 4}],
[{a: 'b', toString() {}}, {toString: jestExpect.any(Function)}],
]);

testToMatchSnapshots([
Expand Down Expand Up @@ -2129,6 +2151,7 @@ describe('toMatchObject()', () => {
{a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'},
{a: 'c', [Symbol.for('jest')]: expect.any(String)},
],
[{a: 'b'}, {toString: jestExpect.any(Function)}],
]);

[
Expand Down
41 changes: 0 additions & 41 deletions packages/expect/src/__tests__/utils.test.ts
Expand Up @@ -11,7 +11,6 @@ import {
emptyObject,
getObjectSubset,
getPath,
hasOwnProperty,
iterableEquality,
subsetEquality,
} from '../utils';
Expand Down Expand Up @@ -107,46 +106,6 @@ describe('getPath()', () => {
});
});

describe('hasOwnProperty', () => {
it('does inherit getter from class', () => {
class MyClass {
get key() {
return 'value';
}
}
expect(hasOwnProperty(new MyClass(), 'key')).toBe(true);
});

it('does not inherit setter from class', () => {
class MyClass {
set key(_value: unknown) {}
}
expect(hasOwnProperty(new MyClass(), 'key')).toBe(false);
});

it('does not inherit method from class', () => {
class MyClass {
key() {}
}
expect(hasOwnProperty(new MyClass(), 'key')).toBe(false);
});

it('does not inherit property from constructor prototype', () => {
function MyClass() {}
MyClass.prototype.key = 'value';
// @ts-expect-error
expect(hasOwnProperty(new MyClass(), 'key')).toBe(false);
});

it('does not inherit __proto__ getter from Object', () => {
expect(hasOwnProperty({}, '__proto__')).toBe(false);
});

it('does not inherit toString method from Object', () => {
expect(hasOwnProperty({}, 'toString')).toBe(false);
});
});

describe('getObjectSubset', () => {
[
[{a: 'b', c: 'd'}, {a: 'd'}, {a: 'b'}],
Expand Down
36 changes: 13 additions & 23 deletions packages/expect/src/utils.ts
Expand Up @@ -21,33 +21,23 @@ type GetPath = {
value?: unknown;
};

// Return whether object instance inherits getter from its class.
const hasGetterFromConstructor = (object: object, key: string) => {
const constructor = object.constructor;
if (constructor === Object) {
// A literal object has Object as constructor.
// Therefore, it cannot inherit application-specific getters.
// Furthermore, Object has __proto__ getter which is not relevant.
// Array, Boolean, Number, String constructors don’t have any getters.
return false;
}
if (typeof constructor !== 'function') {
// Object.create(null) constructs object with no constructor nor prototype.
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create#Custom_and_Null_objects
/**
* Checks if `hasOwnProperty(object, key)` up the prototype chain, stopping at `Object.prototype`.
*/
const hasPropertyInObject = (object: object, key: string): boolean => {
const shouldTerminate =
!object || typeof object !== 'object' || object === Object.prototype;

if (shouldTerminate) {
return false;
}

const descriptor = Object.getOwnPropertyDescriptor(
constructor.prototype,
key,
return (
Object.prototype.hasOwnProperty.call(object, key) ||
hasPropertyInObject(Object.getPrototypeOf(object), key)
);
return descriptor !== undefined && typeof descriptor.get === 'function';
};

export const hasOwnProperty = (object: object, key: string): boolean =>
Object.prototype.hasOwnProperty.call(object, key) ||
hasGetterFromConstructor(object, key);

export const getPath = (
object: Record<string, any>,
propertyPath: string | Array<string>,
Expand Down Expand Up @@ -129,7 +119,7 @@ export const getObjectSubset = (
seenReferences.set(object, trimmed);

Object.keys(object)
.filter(key => hasOwnProperty(subset, key))
.filter(key => hasPropertyInObject(subset, key))
.forEach(key => {
trimmed[key] = seenReferences.has(object[key])
? seenReferences.get(object[key])
Expand Down Expand Up @@ -299,7 +289,7 @@ export const subsetEquality = (
}
const result =
object != null &&
hasOwnProperty(object, key) &&
hasPropertyInObject(object, key) &&
equals(object[key], subset[key], [
iterableEquality,
subsetEqualityWithContext(seenReferences),
Expand Down

0 comments on commit 5c6480b

Please sign in to comment.