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.

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

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

Merged
merged 3 commits into from
Mar 5, 2019

Conversation

pedrottimark
Copy link
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
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

'\n'
);
}
} else if (expected !== undefined) {
Copy link
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
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
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
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
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
Contributor

@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

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