expect: Improve report when matcher fails, part 13

[pedrottimark]

Summary

For toBeInstanceOf matcher:

• Display matcher name in regular black instead of dim color
• Display not following expected label
• Replace confusing constructor name with sentence if received value has no prototype chain
• Omit expected or received constructor name in edge cases

For more information, see discussion with @jeysal in #7795

The report is less helpful than it might be because min option of pretty-format omits class name :(

Residue: Improve report for toThrow with expected error class especially if received error.name is not the same as error.constructor.name

Test plan

• Updated 11 snapshot tests
• Added 4 snapshot tests

See also pictures in following comment.

[pedrottimark]

8 example pictures baseline at left and improved at right

EDIT: updated 3 pictures according to #8077 (comment) and 3 according to #8077 (comment)

Sentence instead of empty string for null or undefined

Sentence instead of confusing constructor name for other primitive values

Sentence instead of undefined if object does not have a prototype chain

Omit Sentence about expected if constructor has empty string as its name

Omit Sentence about received if constructor function overrides name property

Display not following expected label for literal array

Display not following expected label for instance of subclass

Omit Sentence about expected if class has static name method

[jeysal]

"Received value does not have a prototype chain" is a rather cumbersome message, can we cut it down to "Received value has no prototype"? I don't think "chain" is needed (anything that has a prototype also has a prototype chain of length >= 1), unless you wanted to express something specific with it?

[pedrottimark]

Thank you for constructive critique!

[jeysal]

In the "Omit expected if constructor has empty string as its name" case maybe we should still indicate that Expected was an unnamed constructor? "Expected unnamed constructor" or something? Or maybe it's fine because the user can identify the constructor in the code frame anyway, what do you think?

[pedrottimark]

Yes, I agree with short phrases instead of omitted lines. Bonus points to start same as the labels:

• Expected constructor blah blah blah
• Received constructor yada yada yada

[pedrottimark]

Here are two phrases so length of report is consistent positive 3 lines and negative 2 lines:

• Expected/Received constructor name is not a string
• Expected/Received constructor name is an empty string

Omit expected if class has static name method:

expect(received).not.toBeInstanceOf(expected)

Expected constructor name is not a string
Received value: {}

Omit expected if constructor has empty string as its name:

expect(received).toBeInstanceOf(expected)

Expected constructor name is an empty string
Received constructor: RegExp
Received value: /\w+/

P.S. Because pretty-format serializes strings in double quote marks, empty string is not usually a special case. For this matcher (and for toThrow with expected error class) there is a risk for report to give incorrect impression that the constructor name is the criterion.

[jeysal]

LGTM!

[pedrottimark]

For what it’s worth, I have see this warning in local yarn test with node 8.12.0:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit

[SimenB]

Yeah, it's from watch.test.js - it's not something to worry about

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.