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

rejects.toThrowError again #8140

Closed
nshaikhinurov opened this issue Mar 17, 2019 · 11 comments
Closed

rejects.toThrowError again #8140

nshaikhinurov opened this issue Mar 17, 2019 · 11 comments

Comments

@nshaikhinurov
Copy link

nshaikhinurov commented Mar 17, 2019

💥 Regression Report

Related to #3601. I was reading .rejects and .toThrow docs and found inconsistent example imho. Google got me to #3601. Please could anyone help me to understand this issue if i misunderstood anything.

Last working version

Worked up to version: 20.0.3 I think

Stopped working in version: after #4884 i think

To Reproduce

Steps to reproduce the behavior:

sorry for broken code block. Couldn't fix

test("rejects-toThrow test", async () => {
		const myError = new Error("error message")

		// (1) [passes]
		// rejects unwraps the reason to the string literal
		await expect(Promise.reject('octopus')).rejects.toBe('octopus')

		// (2) [passes]
		// rejects unwraps the reason to myError object
		await expect(Promise.reject(myError)).rejects.toBe(myError)

		// (3) [passes]
		// rejects unwraps the reason to the function which throws when called
		// prettier-ignore
		await expect(Promise.reject(() => {throw myError})).rejects.toThrow(myError)

		// (4) [passes] why?
		// INCONSISTENT BEHAVIOUR IMHO
		// from test case (2) we know that rejects unwraps the reason to myError object
		// toThrow supposed to work with functions and not objects that's why test cases (5),(5.1) fail as expected
		// P.S. toThrow with error object as an argument checks error message equality
		await expect(Promise.reject(myError)).rejects.toThrow(myError)

		// (5) [fails]
		// --------------------------------
		// Matcher error: received value must be a function
		// Received has type:  object
		// Received has value: [Error: error message]
		// --------------------------------
		expect(myError).toThrow(myError)

		// (5.1) [fails]
		// --------------------------------
		// Matcher error: received value must be a function
		// Received has type:  string
		// Received has value: "octopus"
		// --------------------------------
		expect('octopus').toThrow('octopus')

		// (6) [fails]
		// So what behaviour should be in this test case?
		// A BUG IMHO
		// from test case (1) we know that rejects unwraps the reason to the string literal
		// from test case (5.1) we know that toThrow receiving a string fails with a Matcher error
		// but here there is no Matcher error. Jest says:
		// --------------------------------
		// Expected substring: "octopus"
		// Received function did not throw
		// --------------------------------
		// Why it now considers unwraped reason from rejects to be a function?
		// P.P.S. toThrow with string as an argument checks error message includes the substring
		await expect(Promise.reject('octopus')).rejects.toThrow('octopus')
	})

Expected behavior

As stated by @BetterCallSKY in #3601 in his 1st message before mergin PR #4884

Link to repl or repo (highly encouraged)

repl.it demo

Run npx envinfo --preset jest

Paste the results here:

System:
    OS: Windows 10
    CPU: (8) x64 AMD Ryzen 5 2500U with Radeon Vega Mobile Gfx
  Binaries:
    Node: 10.13.0 - C:\Program Files\nodejs\node.EXE
    npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD

"jest": "^24.5.0"

@SimenB
Copy link
Member

SimenB commented Mar 17, 2019

@pedrottimark do you wanna unwrap this one and comment on the inconsistencies?

@cspotcode
Copy link
Contributor

.rejects.toThrow doesn't make sense and it's not good for the API to make it magically work with special-casing. There are easier ways to write clearer assertions. .rejects, as described, should return asyncExpect(unwrapRejectionValue(promise)) which can then be matched using any normal matcher.

.rejects.toThrow() implies that the rejection value is itself a function that will then be invoked by toThrow. It implies bizarre usage like expect(Promise.reject(function() {throw new Error()})).rejects.toThrow(Error) (this is shown in the example from @nshaikhinurov)

As is, it's difficult to figure out exactly when .rejects and/or .toThrow will behave and when they'll do something bizarre. This is not the kind of behavior anyone wants from an assertion library.

The ideal API should have:

  • expect(fn).toThrow() invokes function; asserts that it throws; returns expect(thrownValue); does not accept any arguments. (chained matchers take care of the rest)
  • expect(asyncFn).toThrowAsync() invokes function; asserts that it returns rejected promise; returns expectAsync(unwrapRejectionValue(asyncFn()))
  • expect(promise).rejects asserts that value is a promise that rejects; returns expectAsync(unwrapRejectionValue(promise)))

The return value of .rejects and .resolves should also be a valid PromiseLike so they can be awaited directly without chaining a matcher. await expect(promise).rejects;

expect(fn).toThrow().toMatchObject({code: 'ENOENT'});

await expect(asyncFn).toThrowAsync().toMatchObject({code: 'ENOENT'});

await expect(asyncFn()).rejects.toMatchObject({code: 'ENOENT'});

@DanielHreben
Copy link

For others who struggle with this issue for years, here is my workaround - https://github.com/DanielHreben/jest-matcher-specific-error

@adrian-gierakowski
Copy link

@SimenB @pedrottimark any updates on this?

@karlismelderis
Copy link

I would love to be able to write such tests:

expect(fn).toThrow().toMatchObject({code: 'ENOENT'});

@twelve17
Copy link

Ava has a nice API that allows checking multiple properties of the error, in various ways:

https://github.com/avajs/ava/blob/master/docs/03-assertions.md#throwsasyncthrower-expectation-message

It would be nice to have something similar in jest.

@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 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@cspotcode
Copy link
Contributor

cspotcode commented Mar 1, 2022

IMO, this comment #8140 (comment) outlines a clean API that's still relevant. There's a question of versioning since it'll probably be a breaking change.

EDIT removed an incorrect paragraph about confusing docs

@github-actions github-actions bot removed the Stale label Mar 1, 2022
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

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 Mar 2, 2023
@github-actions
Copy link

github-actions bot commented Apr 1, 2023

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2023
@github-actions
Copy link

github-actions bot commented May 2, 2023

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 May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants