Skip to content

expect: Improve report when matcher fails, part 12#8033

Merged
SimenB merged 3 commits intojestjs:masterfrom
pedrottimark:improve-report-12
Mar 5, 2019
Merged

expect: Improve report when matcher fails, part 12#8033
SimenB merged 3 commits intojestjs:masterfrom
pedrottimark:improve-report-12

Conversation

@pedrottimark
Copy link
Copy Markdown
Contributor

Summary

For .toThrow and .toThrowError matchers:

  • Display not following expected label
  • Inverse highlight not-expected substring or pattern in received error message
  • Omit received error message for object instance because criterion is === strict equality

Residue

  • Improve report for error class if received error.name is not the same as error.constructor.name probably together with .toBeInstanceOf matcher
  • Breaking change to require .exec method in addition to .test method for RegExp

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

Test plan

Change error message to more realistic "Invalid array length" in 6 .not snapshots

Chore in snapshot names:

  • replace .toThrow() and .toThrowError() with toThrow and toThrowError
  • replace strings with substring

Updated existing tests: all 60 names because of chore and snapshots:

expected
asymmetric 8
error-message 2
regexp 4
substring 4

See also pictures in following comment.

@pedrottimark
Copy link
Copy Markdown
Contributor Author

Example pictures baseline at left and improved at right

error message includes the substring:

substring

error message matches the pattern:

regexp

error message is equal to the message property of the object:

error-message

thrown value matches the asymmetric matcher:

asymmetric

Comment thread packages/expect/src/toThrowMatchers.ts Outdated
'\n'
);
}
} else if (expected !== undefined) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

instanceof RegExp?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question, as usual.

Some expect code tries to avoid instanceof but is it still a problem?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that might be due to e.g. Error being weird across sandbox boundaries. But here we want you to pass something to us that we run.

I confused myself with that sentence 😛 Did it make sense?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

matcher error if not string or regexp? maybe next major

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A place to change in next major is in createMatcher function, replace:

    } else if (expected !== null && typeof expected.test === 'function') {
      return toThrowExpectedRegExp(matcherName, options, thrown, expected);

with

    } else if (expected instanceof RegExp) {
      return toThrowExpectedRegExp(matcherName, options, thrown, expected);

Copy link
Copy Markdown
Collaborator

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

nice work, LGTM 👍

@github-actions
Copy link
Copy Markdown

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.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants