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

Call jest-diff and pretty-format more precisely in toHaveProperty matcher #4445

Merged
merged 2 commits into from
Sep 14, 2017
Merged

Call jest-diff and pretty-format more precisely in toHaveProperty matcher #4445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28e2f38
Call diff more precisely in toHaveProperty matcher
pedrottimark Sep 7, 2017
4885179
Write diff call on one line for readability
pedrottimark Sep 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2467,7 +2467,7 @@ To have a nested property:
With a value of:
<green>2</>
Received:
<red>object</>.a.b.c: <red>{\\"d\\": 1}</>"
<red>1</>"
`;

exports[`.toHaveProperty() {pass: false} expect({"a": {"b": {"c": {"d": 1}}}}).toHaveProperty('a.b.ttt.d', 1) 1`] = `
Expand Down Expand Up @@ -2517,7 +2517,8 @@ To have a nested property:
With a value of:
<green>{\\"c\\": 4}</>
Received:
<red>object</>.a: <red>{\\"b\\": {\\"c\\": 5}}</>
<red>{\\"c\\": 5}</>

Difference:

<green>- Expected</>
Expand All @@ -2539,7 +2540,8 @@ To have a nested property:
With a value of:
<green>undefined</>
Received:
<red>object</>.a: <red>{\\"b\\": 3}</>
<red>3</>

Difference:

Comparing two different types of values. Expected <green>undefined</> but received <red>number</>."
Expand Down
58 changes: 28 additions & 30 deletions packages/expect/src/matchers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
import {
getObjectSubset,
getPath,
hasOwnProperty,
iterableEquality,
subsetEquality,
} from './utils';
Expand Down Expand Up @@ -516,23 +515,10 @@ const matchers: MatchersObject = {
const result = getPath(object, keyPath);
const {lastTraversedObject, hasEndProp} = result;

let diffString;

if (valuePassed && hasOwnProperty(result, 'value')) {
diffString = diff(value, result.value, {
expand: this.expand,
});
}

const pass = valuePassed
? equals(result.value, value, [iterableEquality])
: hasEndProp;

if (hasOwnProperty(result, 'value')) {
// we don't diff numbers. So instead we'll show the object that contains the resulting value.
// And to get that object we need to go up a level.
result.traversedPath.pop();
}
const traversedPath = result.traversedPath.join('.');

const message = pass
Expand All @@ -546,22 +532,34 @@ const matchers: MatchersObject = {
`Not to have a nested property:\n` +
` ${printExpected(keyPath)}\n` +
(valuePassed ? `With a value of:\n ${printExpected(value)}\n` : '')
: () =>
matcherHint('.toHaveProperty', 'object', 'path', {
secondArgument: valuePassed ? 'value' : null,
}) +
'\n\n' +
`Expected the object:\n` +
` ${printReceived(object)}\n` +
`To have a nested property:\n` +
` ${printExpected(keyPath)}\n` +
(valuePassed ? `With a value of:\n ${printExpected(value)}\n` : '') +
(traversedPath
? `Received:\n ${RECEIVED_COLOR(
'object',
)}.${traversedPath}: ${printReceived(lastTraversedObject)}`
: '') +
(diffString ? `\nDifference:\n\n${diffString}` : '');
: () => {
const diffString =
valuePassed && hasEndProp
? diff(value, result.value, {expand: this.expand})
: '';
return (
matcherHint('.toHaveProperty', 'object', 'path', {
secondArgument: valuePassed ? 'value' : null,
}) +
'\n\n' +
`Expected the object:\n` +
` ${printReceived(object)}\n` +
`To have a nested property:\n` +
` ${printExpected(keyPath)}\n` +
(valuePassed
? `With a value of:\n ${printExpected(value)}\n`
: '') +
(hasEndProp
? `Received:\n` +
` ${printReceived(result.value)}` +
(diffString ? `\n\nDifference:\n\n${diffString}` : '')
Copy link
Collaborator

Choose a reason for hiding this comment

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

\n\nDifference

is this double newline intended? Old code has only one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I see from snapshot that it actually looks better now 👍

Copy link
Contributor Author

@pedrottimark pedrottimark Sep 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

EDIT: Good eyes! Yes, the double newline is following pattern of toEqual matcher.

: traversedPath
? `Received:\n ${RECEIVED_COLOR(
'object',
)}.${traversedPath}: ${printReceived(lastTraversedObject)}`
: '')
);
};
if (pass === undefined) {
throw new Error('pass must be initialized');
}
Expand Down