Skip to content

Conversation

natealcedo
Copy link

What

This pull request adds in a new matcher for asserting that a callback function throws an error of a given type and given error message.

Why

This feature has been requested in core but after some discussion, it was decided that implementing it here would be the better solution.

This PR fixes the following issues

  1. make throwingMatcher test n arguments (currently zero or one) jestjs/jest#7000
  2. .toThrow() to match both error constructor and message? jestjs/jest#3659
  3. toThrow(constructor, message) #157

Notes

I've gone down the road of testing the matcher both by extending expect with this matcher and by invoking the matcher itself. Since this repository doesn't have some of the util tools that the Jest repo has, namely runJest, I am not able to test the failing scenarios since I can't capture whatever is logged to stdErr to assert that the error message is indeed correct. As such, I've just invoked the matcher itself and asserted whether it passes or not and used snapshot testing for asserting error messages.

Also, the goal for me in this matcher was to stick as close to the default toThrowErrormatcher. As such, this matcher also accepts both a string and regex for matching the error message.

The code is abit verbose right now but I'm more than happy to have a discussion on how to amend it moving forward.

Housekeeping

  • Unit tests
  • Documentation is up to date
  • No additional lint warnings
  • Add yourself to contributors list (yarn contributor)
  • Typescript definitions are added/updated where relevant

@codecov-io
Copy link

codecov-io commented Sep 22, 2018

Codecov Report

Merging #170 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #170   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         103    105    +2     
  Lines         505    536   +31     
  Branches       86     95    +9     
=====================================
+ Hits          505    536   +31
Impacted Files Coverage Δ
src/matchers/toThrowWithMessage/predicate.js 100% <100%> (ø)
src/matchers/toThrowWithMessage/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53a2c60...aa807a6. Read the comment docs.

@natealcedo natealcedo force-pushed the master branch 5 times, most recently from 4c213e6 to ee823b3 Compare September 22, 2018 16:14
README.md Outdated
#### .toThrowWithMessage(type, message)
Use `.toThrowWithMessage` when checking if a callback function throws an error with a given error type and given error message.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it's worth mentioning that the message can be either a String or RegExp?

Copy link
Author

@natealcedo natealcedo Sep 24, 2018

Choose a reason for hiding this comment

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

This sounds like a great idea! I've done the update.

if (!callback || typeof callback !== 'function') {
return {
pass: false,
message: () => `
Copy link
Member

Choose a reason for hiding this comment

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

The spacing of these validation errors is tabbed in too much for the printouts (see the snapshots)

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I've pushed in a change for this. Do take a look.

@mattphillips
Copy link
Member

@natealcedo this is looking good just a few small comments :)

@mattphillips
Copy link
Member

Thanks @natealcedo 👍

@mattphillips mattphillips merged commit 5e43791 into jest-community:master Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants