Skip to content

Commit

Permalink
fix: handle circular references correctly in objects (closes #8663)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasfcosta committed Jul 14, 2019
1 parent b76f01b commit b0f9514
Show file tree
Hide file tree
Showing 5 changed files with 258 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -24,6 +24,7 @@
- `[jest-core]` Fix incorrect `passWithNoTests` warning ([#8595](https://github.com/facebook/jest/pull/8595))
- `[jest-snapshots]` Fix test retries that contain snapshots ([#8629](https://github.com/facebook/jest/pull/8629))
- `[jest-mock]` Fix incorrect assignments when restoring mocks in instances where they originally didn't exist ([#8631](https://github.com/facebook/jest/pull/8631))
- `[expect]` Fix stack overflow when matching objects with circular references ([#8687](https://github.com/facebook/jest/pull/8687))

### Chore & Maintenance

Expand Down
86 changes: 86 additions & 0 deletions packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
Expand Up @@ -4092,6 +4092,92 @@ Expected: not <green>Set {2, 1}</>
Received: <red>Set {1, 2}</>"
`;

exports[`toMatchObject() circular references simple circular references {pass: false} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "world", "ref": [Circular]}) 1`] = `"Maximum call stack size exceeded"`;

exports[`toMatchObject() circular references simple circular references {pass: false} expect({"ref": "not a ref"}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>

<green>- Expected</>
<red>+ Received</>

<dim> Object {</>
<green>- \\"a\\": \\"hello\\",</>
<green>- \\"ref\\": [Circular],</>
<red>+ \\"ref\\": \\"not a ref\\",</>
<dim> }</>"
`;

exports[`toMatchObject() circular references simple circular references {pass: false} expect({}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>

<green>- Expected</>
<red>+ Received</>

<green>- Object {</>
<green>- \\"a\\": \\"hello\\",</>
<green>- \\"ref\\": [Circular],</>
<green>- }</>
<red>+ Object {}</>"
`;

exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>

Expected: not <green>{\\"a\\": \\"hello\\", \\"ref\\": [Circular]}</>"
`;

exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>

Expected: not <green>{}</>
Received: <red>{\\"a\\": \\"hello\\", \\"ref\\": [Circular]}</>"
`;

exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"a": "world", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `"Maximum call stack size exceeded"`;

exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"nestedObj": {"parentObj": "not the parent ref"}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>

<green>- Expected</>
<red>+ Received</>

<dim> Object {</>
<green>- \\"a\\": \\"hello\\",</>
<dim> \\"nestedObj\\": Object {</>
<green>- \\"parentObj\\": [Circular],</>
<red>+ \\"parentObj\\": \\"not the parent ref\\",</>
<dim> },</>
<dim> }</>"
`;

exports[`toMatchObject() circular references transitive circular references {pass: false} expect({}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>

<green>- Expected</>
<red>+ Received</>

<green>- Object {</>
<green>- \\"a\\": \\"hello\\",</>
<green>- \\"nestedObj\\": Object {</>
<green>- \\"parentObj\\": [Circular],</>
<green>- },</>
<green>- }</>
<red>+ Object {}</>"
`;

exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>

Expected: not <green>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
`;

exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>

Expected: not <green>{}</>
Received: <red>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
`;

exports[`toMatchObject() throws expect("44").toMatchObject({}) 1`] = `
"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>

Expand Down
110 changes: 88 additions & 22 deletions packages/expect/src/__tests__/matchers.test.js
Expand Up @@ -1537,7 +1537,91 @@ describe('toMatchObject()', () => {
}
}

[
const testNotToMatchSnapshots = tuples => {
tuples.forEach(([n1, n2]) => {
it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify(
n2,
)})`, () => {
jestExpect(n1).toMatchObject(n2);
expect(() =>
jestExpect(n1).not.toMatchObject(n2),
).toThrowErrorMatchingSnapshot();
});
});
};

const testToMatchSnapshots = tuples => {
tuples.forEach(([n1, n2]) => {
it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify(
n2,
)})`, () => {
jestExpect(n1).not.toMatchObject(n2);
expect(() =>
jestExpect(n1).toMatchObject(n2),
).toThrowErrorMatchingSnapshot();
});
});
};

describe('circular references', () => {
describe('simple circular references', () => {
const circularObj = {a: 'hello'};
circularObj.ref = circularObj;

const differentCircularObj = {a: 'world'};
differentCircularObj.ref = differentCircularObj;

const otherCircularObj = {a: 'hello'};
otherCircularObj.ref = otherCircularObj;

const primitiveInsteadOfRef = {};
primitiveInsteadOfRef.ref = 'not a ref';

testNotToMatchSnapshots([
[circularObj, {}],
[otherCircularObj, circularObj],
]);

testToMatchSnapshots([
[{}, circularObj],
[circularObj, differentCircularObj],
[primitiveInsteadOfRef, circularObj],
]);
});

describe('transitive circular references', () => {
const transitiveCircularObj = {a: 'hello'};
transitiveCircularObj.nestedObj = {parentObj: transitiveCircularObj};

const otherTransitiveCircularObj = {a: 'hello'};
otherTransitiveCircularObj.nestedObj = {
parentObj: otherTransitiveCircularObj,
};

const differentTransitiveCircularObj = {a: 'world'};
differentTransitiveCircularObj.nestedObj = {
parentObj: differentTransitiveCircularObj,
};

const primitiveInsteadOfRef = {};
primitiveInsteadOfRef.nestedObj = {
parentObj: 'not the parent ref',
};

testNotToMatchSnapshots([
[transitiveCircularObj, {}],
[otherTransitiveCircularObj, transitiveCircularObj],
]);

testToMatchSnapshots([
[{}, transitiveCircularObj],
[differentTransitiveCircularObj, transitiveCircularObj],
[primitiveInsteadOfRef, transitiveCircularObj],
]);
});
});

testNotToMatchSnapshots([
[{a: 'b', c: 'd'}, {a: 'b'}],
[{a: 'b', c: 'd'}, {a: 'b', c: 'd'}],
[{a: 'b', t: {x: {r: 'r'}, z: 'z'}}, {a: 'b', t: {z: 'z'}}],
Expand All @@ -1560,18 +1644,9 @@ describe('toMatchObject()', () => {
[new Error('bar'), {message: 'bar'}],
[new Foo(), {a: undefined, b: 'b'}],
[Object.assign(Object.create(null), {a: 'b'}), {a: 'b'}],
].forEach(([n1, n2]) => {
it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify(
n2,
)})`, () => {
jestExpect(n1).toMatchObject(n2);
expect(() =>
jestExpect(n1).not.toMatchObject(n2),
).toThrowErrorMatchingSnapshot();
});
});
]);

[
testToMatchSnapshots([
[{a: 'b', c: 'd'}, {e: 'b'}],
[{a: 'b', c: 'd'}, {a: 'b!', c: 'd'}],
[{a: 'a', c: 'd'}, {a: jestExpect.any(Number)}],
Expand All @@ -1597,16 +1672,7 @@ describe('toMatchObject()', () => {
[[1, 2, 3], [1, 2, 2]],
[new Error('foo'), new Error('bar')],
[Object.assign(Object.create(null), {a: 'b'}), {c: 'd'}],
].forEach(([n1, n2]) => {
it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify(
n2,
)})`, () => {
jestExpect(n1).not.toMatchObject(n2);
expect(() =>
jestExpect(n1).toMatchObject(n2),
).toThrowErrorMatchingSnapshot();
});
});
]);

[
[null, {}],
Expand Down
54 changes: 54 additions & 0 deletions packages/expect/src/__tests__/utils.test.js
Expand Up @@ -202,6 +202,60 @@ describe('subsetEquality()', () => {
test('undefined does not return errors', () => {
expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy();
});

describe('matching subsets with circular references', () => {
test('simple circular references', () => {
const circularObj = {a: 'hello'};
circularObj.ref = circularObj;

const otherCircularObj = {a: 'hello'};
otherCircularObj.ref = otherCircularObj;

const differentCircularObj = {a: 'world'};
differentCircularObj.ref = differentCircularObj;

const primitiveInsteadOfRef = {};
primitiveInsteadOfRef.ref = 'not a ref';

expect(subsetEquality(circularObj, {})).toBe(true);
expect(subsetEquality({}, circularObj)).toBe(false);
expect(subsetEquality(otherCircularObj, circularObj)).toBe(true);
expect(subsetEquality(differentCircularObj, circularObj)).toBe(false);
expect(subsetEquality(primitiveInsteadOfRef, circularObj)).toBe(false);
});

test('transitive circular references', () => {
const transitiveCircularObj = {a: 'hello'};
transitiveCircularObj.nestedObj = {parentObj: transitiveCircularObj};

const otherTransitiveCircularObj = {a: 'hello'};
otherTransitiveCircularObj.nestedObj = {
parentObj: otherTransitiveCircularObj,
};

const differentTransitiveCircularObj = {a: 'world'};
differentTransitiveCircularObj.nestedObj = {
parentObj: differentTransitiveCircularObj,
};

const primitiveInsteadOfRef = {};
primitiveInsteadOfRef.nestedObj = {
parentObj: 'not the parent ref',
};

expect(subsetEquality(transitiveCircularObj, {})).toBe(true);
expect(subsetEquality({}, transitiveCircularObj)).toBe(false);
expect(
subsetEquality(otherTransitiveCircularObj, transitiveCircularObj),
).toBe(true);
expect(
subsetEquality(differentTransitiveCircularObj, transitiveCircularObj),
).toBe(false);
expect(subsetEquality(primitiveInsteadOfRef, transitiveCircularObj)).toBe(
false,
);
});
});
});

describe('iterableEquality', () => {
Expand Down
38 changes: 29 additions & 9 deletions packages/expect/src/utils.ts
Expand Up @@ -268,16 +268,36 @@ export const subsetEquality = (
object: any,
subset: any,
): undefined | boolean => {
if (!isObjectWithKeys(subset)) {
return undefined;
}
// subsetEquality needs to keep track of the references
// it has already visited to avoid infinite loops in case
// there are circular references in the subset passed to it.
const subsetEqualityWithContext = (
seenReferences: WeakMap<object, boolean> = new WeakMap(),
) => (object: any, subset: any): undefined | boolean => {
if (!isObjectWithKeys(subset)) {
return undefined;
}

return Object.keys(subset).every(
key =>
object != null &&
hasOwnProperty(object, key) &&
equals(object[key], subset[key], [iterableEquality, subsetEquality]),
);
return Object.keys(subset).every(key => {
if (isObjectWithKeys(subset[key])) {
if (seenReferences.get(subset[key])) {
return equals(object[key], subset[key], [iterableEquality]);
}
seenReferences.set(subset[key], true);
}

return (
object != null &&
hasOwnProperty(object, key) &&
equals(object[key], subset[key], [
iterableEquality,
subsetEqualityWithContext(seenReferences),
])
);
});
};

return subsetEqualityWithContext()(object, subset);
};

export const typeEquality = (a: any, b: any) => {
Expand Down

0 comments on commit b0f9514

Please sign in to comment.