Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Iterable equality within object #8359

Merged
merged 15 commits into from Feb 24, 2022
Merged

Iterable equality within object #8359

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9bc6a08
iterableEquality when given iterator within inequal object
d7my11 Apr 21, 2019
86312c9
remove unnecessary object check
d7my11 Apr 22, 2019
7e13ba4
check equality again only if object has keys
d7my11 Apr 22, 2019
8ec8ab5
ignore iterators while checking other properties
d7my11 Apr 22, 2019
53408bd
remove unnecessary emptyObject check
d7my11 Apr 22, 2019
87c4e62
update CHANGELOG.md
d7my11 Apr 22, 2019
1e5d3b7
Merge branch 'master' of github.com:d7my11/jest into iterable-equalit…
d7my11 Apr 22, 2019
84f8bda
add getObjectEntries instead of Object.entries
d7my11 Apr 22, 2019
8068054
import in ascending order
d7my11 Apr 22, 2019
3cd2b4e
better type for getObjectEntries - fix linting issue
d7my11 Apr 22, 2019
c4d40b9
Merge branch 'master' of github.com:facebook/jest into iterable-equal…
d7my11 Oct 15, 2019
2c3a862
Merge branch 'master' of github.com:facebook/jest into iterable-equal…
d7my11 Nov 23, 2019
416f5b3
replace getObjectEntries with Object.entries
d7my11 Nov 23, 2019
e50e95e
Merge branch 'main' into iterable-equality-within-object
SimenB Feb 24, 2022
5a46db1
move changelog
SimenB Feb 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/expect/src/__tests__/utils.test.js
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ const {stringify} = require('jest-matcher-utils');
const {
emptyObject,
getObjectSubset,
getObjectEntries,
getPath,
hasOwnProperty,
subsetEquality,
Expand Down Expand Up @@ -166,6 +167,18 @@ describe('getObjectSubset()', () => {
});
});

describe('getObjectEntries', () => {
test('returns an empty array when called with null', () => {
expect(getObjectEntries(null)).toEqual([]);
});
test('returns an empty array when called with undefined', () => {
expect(getObjectEntries(undefined)).toEqual([]);
});
test('returns object entries', () => {
expect(getObjectEntries({ a: 1, b: 2 })).toEqual([['a', 1], ['b', 2]]);
});
})

describe('emptyObject()', () => {
test('matches an empty object', () => {
expect(emptyObject({})).toBe(true);
Expand Down
7 changes: 5 additions & 2 deletions packages/expect/src/utils.ts
View file
Open in desktop
Expand Up @@ -133,6 +133,9 @@ export const getObjectSubset = (object: any, subset: any): any => {
return object;
};

export const getObjectEntries = (object: any): Array<any> =>
d7my11 marked this conversation as resolved.
Show resolved Hide resolved
object == null ? [] : Object.keys(object).map((key) => [key, object[key]])

const IteratorSymbol = Symbol.iterator;

const hasIterator = (object: any) =>
Expand Down Expand Up @@ -251,8 +254,8 @@ export const iterableEquality = (
return false;
}

const aEntries = Object.entries(a);
const bEntries = Object.entries(b);
const aEntries = getObjectEntries(a);
const bEntries = getObjectEntries(b);
if (!equals(aEntries, bEntries)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add [iterableEquality] as optional customTesters argument to equals call in case any of these property values have iterators? All the calls in matchers.ts include it. I am asking, not telling.

This illustrates a limitation that we recently noticed: it is not (yet) possible to for a custom tester to know about sibling testers (for example, toStrictEqual matcher needs more testers than toEqual does, and iterableEquality has no way to provide them in this recursive call).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pedrottimark is there an extra test case you're thinking of we should add?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so sorry for the very late follow up.
I'm not familiar with the code base but I see the point and the limitation.

This illustrates a limitation that we recently noticed: it is not (yet) possible to for a custom tester to know about sibling testers

IMO customTesters should be kept like this so each custom tester should test only one thing, so my changes in iterableEquality method will be reverted and will include non-enumerable members again in keys method in jasmineUtils.ts
I'm not sure of the consequences of this change but will try it out.

return false;
}
Expand Down