Skip to content

Commit

Permalink
Use iterableEquality in spy matchers
Browse files Browse the repository at this point in the history
While working on custom matchers to solve #3574, I found out that the cause for
not seeing this issue in `expect().toEqual()` comes from the fact that this
matcher passes the `iterableEquality` to the `equals()` function.

When I added this to the equal calls for our spy matchers as well, Immutable.js
types were properly suppored.

I'm considering this is a bug since the `toBeCalledWith()` matchers should
behave the same as the `equals()` matcher.
  • Loading branch information
philipp-spiess committed Jun 27, 2017
1 parent 98a6d9d commit f43a04a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ Expected mock function to have been last called with:
But it was <red>not called</>."
`;
exports[`lastCalledWith works with Immutable.js objects 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).not.lastCalledWith(<green>expected</><dim>)
Expected mock function to not have been last called with:
<green>[Immutable.Map {a: {\\"b\\": \\"c\\"}}, Immutable.Map {a: {\\"b\\": \\"c\\"}}]</>"
`;
exports[`lastCalledWith works with arguments that don't match 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).lastCalledWith(<green>expected</><dim>)
Expand Down Expand Up @@ -205,6 +212,13 @@ Expected mock function to have been called with:
But it was <red>not called</>."
`;
exports[`toHaveBeenCalledWith works with Immutable.js objects 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).not.toHaveBeenCalledWith(<green>expected</><dim>)
Expected mock function not to have been called with:
<green>[Immutable.Map {a: {\\"b\\": \\"c\\"}}, Immutable.Map {a: {\\"b\\": \\"c\\"}}]</>"
`;
exports[`toHaveBeenCalledWith works with arguments that don't match 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).toHaveBeenCalledWith(<green>expected</><dim>)
Expand Down Expand Up @@ -253,6 +267,13 @@ Expected mock function to have been last called with:
But it was <red>not called</>."
`;
exports[`toHaveBeenLastCalledWith works with Immutable.js objects 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).not.toHaveBeenLastCalledWith(<green>expected</><dim>)
Expected mock function to not have been last called with:
<green>[Immutable.Map {a: {\\"b\\": \\"c\\"}}, Immutable.Map {a: {\\"b\\": \\"c\\"}}]</>"
`;
exports[`toHaveBeenLastCalledWith works with arguments that don't match 1`] = `
"<dim>expect(<red>jest.fn()</><dim>).toHaveBeenLastCalledWith(<green>expected</><dim>)
Expand Down
14 changes: 14 additions & 0 deletions packages/jest-matchers/src/__tests__/spy_matchers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @emails oncall+jsinfra
*/

const Immutable = require('immutable');
const jestExpect = require('../');

['toHaveBeenCalled', 'toBeCalled'].forEach(called => {
Expand Down Expand Up @@ -167,4 +168,17 @@ describe('toHaveBeenCalledTimes', () => {
jestExpect(fn).not[calledWith]('foo', 'bar'),
).toThrowErrorMatchingSnapshot();
});

test(`${calledWith} works with Immutable.js objects`, () => {
const fn = jest.fn();
const directlyCreated = new Immutable.Map([['a', {b: 'c'}]]);
const indirectlyCreated = new Immutable.Map().set('a', {b: 'c'});
fn(directlyCreated, indirectlyCreated);

jestExpect(fn)[calledWith](indirectlyCreated, directlyCreated);

expect(() =>
jestExpect(fn).not[calledWith](indirectlyCreated, directlyCreated),
).toThrowErrorMatchingSnapshot();
});
});
37 changes: 6 additions & 31 deletions packages/jest-matchers/src/matchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ import {
printExpected,
printWithType,
} from 'jest-matcher-utils';
import {getObjectSubset, getPath, hasOwnProperty} from './utils';
import {
getObjectSubset,
getPath,
hasOwnProperty,
iterableEquality,
} from './utils';
import {equals} from './jasmine_utils';

type ContainIterable =
Expand All @@ -33,36 +38,6 @@ type ContainIterable =
| DOMTokenList
| HTMLCollection<any>;

const IteratorSymbol = Symbol.iterator;

const hasIterator = object => !!(object != null && object[IteratorSymbol]);
const iterableEquality = (a, b) => {
if (
typeof a !== 'object' ||
typeof b !== 'object' ||
Array.isArray(a) ||
Array.isArray(b) ||
!hasIterator(a) ||
!hasIterator(b)
) {
return undefined;
}
if (a.constructor !== b.constructor) {
return false;
}
const bIterator = b[IteratorSymbol]();

for (const aValue of a) {
const nextB = bIterator.next();
if (nextB.done || !equals(aValue, nextB.value, [iterableEquality])) {
return false;
}
}
if (!bIterator.next().done) {
return false;
}
return true;
};
const isObjectWithKeys = a =>
a !== null &&
typeof a === 'object' &&
Expand Down
5 changes: 3 additions & 2 deletions packages/jest-matchers/src/spy_matchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
RECEIVED_COLOR,
} from 'jest-matcher-utils';
import {equals} from './jasmine_utils';
import {iterableEquality} from './utils';

const RECEIVED_NAME = {
'mock function': 'jest.fn()',
Expand Down Expand Up @@ -68,7 +69,7 @@ const createToBeCalledWithMatcher = matcherName => (
const calls = receivedIsSpy
? received.calls.all().map(x => x.args)
: received.mock.calls;
const pass = calls.some(call => equals(call, expected));
const pass = calls.some(call => equals(call, expected, [iterableEquality]));

const message = pass
? () =>
Expand Down Expand Up @@ -97,7 +98,7 @@ const createLastCalledWithMatcher = matcherName => (
const calls = receivedIsSpy
? received.calls.all().map(x => x.args)
: received.mock.calls;
const pass = equals(calls[calls.length - 1], expected);
const pass = equals(calls[calls.length - 1], expected, [iterableEquality]);

const message = pass
? () =>
Expand Down
34 changes: 34 additions & 0 deletions packages/jest-matchers/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
* @flow
*/

import {equals} from './jasmine_utils';

type GetPath = {
hasEndProp?: boolean,
lastTraversedObject: ?Object,
Expand Down Expand Up @@ -91,8 +93,40 @@ const getObjectSubset = (object: Object, subset: Object) => {
return object;
};

const IteratorSymbol = Symbol.iterator;

const hasIterator = object => !!(object != null && object[IteratorSymbol]);
const iterableEquality = (a: any, b: any) => {
if (
typeof a !== 'object' ||
typeof b !== 'object' ||
Array.isArray(a) ||
Array.isArray(b) ||
!hasIterator(a) ||
!hasIterator(b)
) {
return undefined;
}
if (a.constructor !== b.constructor) {
return false;
}
const bIterator = b[IteratorSymbol]();

for (const aValue of a) {
const nextB = bIterator.next();
if (nextB.done || !equals(aValue, nextB.value, [iterableEquality])) {
return false;
}
}
if (!bIterator.next().done) {
return false;
}
return true;
};

module.exports = {
getObjectSubset,
getPath,
hasOwnProperty,
iterableEquality,
};

0 comments on commit f43a04a

Please sign in to comment.