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

Confusing output from expect.toThrow #4724

Closed
tamlyn opened this issue Oct 18, 2017 · 10 comments
Closed

Confusing output from expect.toThrow #4724

tamlyn opened this issue Oct 18, 2017 · 10 comments
Assignees
Labels

Comments

@tamlyn
Copy link
Contributor

tamlyn commented Oct 18, 2017

Do you want to request a feature or report a bug?

A feature although you could argue the current behaviour is a bug.

What is the current behavior?

expect.toThrow uses a different expected string in its output that what it matches against internally.

For example:

const error = new Error('oops');
error.name = 'ValidationError';
expect(() => {throw error}).toThrow("ValidationError: oops")

Produces this confusing error message:

Expected the function to throw an error matching:
  "ValidationError: oops"
Instead, it threw:
  ValidationError: oops

In particular, this happens with errors from joi and it has tripped me up several times.

What is the expected behavior?

Seems logical that toThrowMatchingStringOrRegexp should match against the same string that it displays as the expected output and that this should be ${error.name}: ${error.message}.

Changing the expected string would be a breaking change but breakages should be few and far between. You'd have to be explicitly expecting the name of the error to not appear in the error message.

As a bonus, being able to assert against the error name as a string instead of a constructor would be useful in cases where the error constructor is hidden away in a library.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

$ npx jest --version; node -v
v21.2.1
v8.6.0
@LiaRowan
Copy link

LiaRowan commented Feb 23, 2018

Yeah, this is really annoying, and I would consider it a bug since it says what it expects to be matching but doesn't actually match against that. It appears to be only matching against error.message. I really would not consider this a "breaking change" since it's more of a bug fix.

Or, if worried about breaking changes, you could add a new expect such as, expect().toThrowWithName() or something like that.

This is pretty frustrating since it's very useful in many cases to just check that the correct "type" of error is being thrown rather than the exact error message - just like ValidationError's in the original post.

@diversemix
Copy link

Maybe the function signature toThrow() should take errorType and errorString ? That way you could match on just the ValidationError or the complete message? Either way as it is at the minute is HIGHLY confusing

@nhnicwaller
Copy link

Is there any workaround for this, or is it really not possible to check error.name using expect.toThrow?

What's the holdup on getting this fixed? I can submit a pull request if the maintainers agree on what should be the correct, desired behaviour. I think @tamlyn original suggestion of matching against ${error.name}: ${error.message} is very good.

@SimenB
Copy link
Member

SimenB commented Dec 13, 2018

@pedrottimark are you fixing this? Seems like it would show the type error first, right, not just the stringified thing?

@pedrottimark
Copy link
Contributor

Reducing confusion will be a part of the “Improve report when assertion fails” series of PR.

@SimenB To improve the matcher criterion, what do you think about the following:

Concerning the proposed solution to implicitly include ${error.name}: ${error.message} in the current string argument, I have some concerns. A few more realistic (that is, not "oops" :) examples of failing tests would be welcome for me to prepare prototypes of reports when test fail for y’all to critique /cc @tamlyn for example, from joi also @nhnicwaller

@pedrottimark
Copy link
Contributor

pedrottimark commented Dec 15, 2018

Update to previous comment, recommend object literal to match error.name in one arg:

  • toThrow({name, message})
  • toThrow({name})

parallel to error instance object, instead of as an optional second argument.

EDIT: doesn’t support matching name as string and message as regexp. Is that criterion needed?

@natealcedo
Copy link
Contributor

natealcedo commented Jan 2, 2019

For those interested, I implemented something similar actually in jest-extended. https://github.com/jest-community/jest-extended#tothrowwithmessagetype-message I have a pull request to handle the .not cases there since I didnt handle it during the initial implementation. Just waiting for @mattphillips to merge that in. jest-community/jest-extended#173

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 26, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

This issue 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 Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants