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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
toThrow should return the error #8698
Comments
Sounds like a good idea to me. @SimenB @thymikee @pedrottimark wdyt? |
While I agree the idea is sound (I love this part of Ava's API), I'm not sure we should do this. All matchers today return Note that this change will also break things like https://github.com/mattphillips/jest-chain (/cc @mattphillips who might have thoughts on this 馃檪) As for my own opinion - I think this is better solved by a custom matcher that takes predicate rather than returning the error. expect(() => { throw new Error('boom'); }).toThrowWithField(err => err.code === 'MODULE_NOT_FOUND'); or something like that. Such a matcher may or may not make sense to have in core, but that's a separate discussion |
Personally I disagree. But I also think that 90% of the matchers should be removed and should just be expressed as plain javascript with I'd prefer to use the existing matcher if anything: test('example', () => {
let errorThrown;
expect(() => {
throw new Error('boom');
}).toThrow((error) => {
errorThrown = error;
// boolean return (true passes, false fails)
return error.message === 'boom';
});
expect(errorThrown.code).toEqual('MODULE_NOT_FOUND');
}); But I think the PR as is feels a lot more natural / expected. Also I think it is best to provide the tools to never need |
Just curious, why is this considered a big change? |
Note that you can do this today: test('example', () => {
expect(() => {
throw new Error('boom');
}).toThrow(expect.objectContaining({ message: 'boom' }));
}); For |
Another big use case for this is dealing with aggregate errors. Examples: |
The type declarations say that matchers return the actual value. Is that intended to be the case, or is it a mistake? https://github.com/facebook/jest/blob/master/packages/expect/src/types.ts#L118-L122 I'm hitting the same issue where it's annoying to assert against thrown values. I already know how to use
It's still not ideal because multiple assertions means multiple function invocations, which is not the case with the promise. The assertion library should be giving the developer a straightforward way to assert that something throws and get a reference to the thrown value. |
@SimenB Is there any progress to this proposal? I really like it and would be greatly ease testing async functions. |
Upvoting this feature as well. 馃崒 that it's been sitting for a year without action while other test suites support this. |
@SimenB Is this doced anywhere? I canot find. |
This is great, but I'd like to be able to use expect(thing()).rejects.toBeInstanceOf(SomeErrorClass) and additionally check that the thrown error has the correct data added to it (ie constructed with the right message or value). |
Currently I do const promise = thing()
await expect(promise).rejects.toThrow(MyErrorClass)
await expect(promise).rejects.toThrow('Error message')
await expect(promise).rejects.toThrow(expect.objectContaining({ code: 'MY_CODE' })); But it only kind of works for async code if you want |
For the sake of keeping Jest consistent, I'm actually fine with this approach. However, I think it would be useful for expect(() => { throw new Error('boom'); }).toThrow((err: any): void => {
expect(err.code).toBe('MODULE_NOT_FOUND');
}); It's basically just a callback that enables the user to perform their own assertions on the thrown error.
Ideally, users should be able to avoid making custom matchers. That's probably its own barrier to entry. I've been using Jest for years, and I've never had to make custom matchers. I imagine this is why A) Resorting to "Make a custom matcher" kind of defeats the point and raises a number of problems. Moreover, as chrisblossom, said, leaving the The current |
Welp. For anyone who's interested, I had the (bad) idea of trying out matchers for the first time. I'm not sure whether or not people will find it useful, but I'm leaving it behind as an artifact just in case... while people are waiting to hear back from the Jest devs. My solution should solve both sync and async use cases. @Janpot could you let me know if you find this useful? You still have to write a bit of code with my solution, but hopefully the approach looks less hackish to you. const promise = thing() // `thing` is an async function
await expect(promise).rejects.toThrowWithError((error: MyErrorClass) => {
expect(error).toBeInstanceOf(MyErrorClass);
expect(error.message).toBe("Error message");
expect(error.code).toBe("MY_CODE");
}); Jest Code (TypeScript)Note that I tested my code locally, and it works in all of the anticipated use cases. So if you use my code and it doesn't work, then either A) You have an outdated version of Extend expect.extend({
toThrowWithError(received, expected) {
// Constants
const passErrorToUser = (err: unknown): jest.CustomMatcherResult => {
expected(err);
return { pass: true, message: () => "" };
};
/* -------------------- Handle Promise Modifiers -------------------- */
if (this.promise === "resolves")
return {
pass: false,
message: () =>
this.utils.matcherErrorMessage(
this.utils.matcherHint("toThrowWithError", undefined, undefined, {
promise: this.promise,
}),
`'${this.promise}' is an invalid modifier for 'toThrowWithError'`,
""
),
};
// Jest already takes care of validating `rejects`. We just need to pass the error along.
if (this.promise === "rejects") return passErrorToUser(received);
/* -------------------- Argument Validation -------------------- */
if (typeof received !== "function")
return {
pass: false,
message: () =>
this.utils.matcherErrorMessage(
this.utils.matcherHint("toThrowWithError"),
`${this.utils.RECEIVED_COLOR("received")} value must be a function`,
`Received has value: ${this.utils.RECEIVED_COLOR(typeof received)}`
),
};
if (typeof expected !== "function")
return {
pass: false,
message: () =>
this.utils.matcherErrorMessage(
this.utils.matcherHint("toThrowWithError"),
`${this.utils.EXPECTED_COLOR("expected")} value must be a function`,
`Expected has value: ${this.utils.EXPECTED_COLOR(typeof expected)}`
),
};
/* -------------------- Matcher Logic -------------------- */
try {
received();
const errorMessage = "Received function did not throw" as const; // copied from Jest's `toThrow` matcher.
return {
pass: false,
message: () =>
`${this.utils.matcherHint("toThrowWithError", undefined, undefined)}\n\n${errorMessage}`,
};
} catch (err) {
return passErrorToUser(err);
}
}
}); For TS users, you'll still have to update the types for Jest. In a type declaration file of your choosing, apply the following: // Add custom jest matchers
declare global {
namespace jest {
interface Matchers<R> {
/**
* Used to test that a function throws when it is called. Allows
* assertions to be performed on the error that is generated.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
toThrowWithError(expected: (error: any) => void): R extends Promise<void> ? R : void;
}
}
} (Note that I use TS ESLint's recommended settings. So I have a comment for disabling a linter warning.) I would still love to hear from the Jest devs about updating their |
Being able to analyze the error in the usual jest chained functional style feels like it should be part of the standard api, especially with the discouragement of try{} catch{} |
I noticed that I was able to workaround this an make a Usage: it("checks an error", async () => {
let error = await getError(doAsyncStuff(arg1, arg2));
// wrap in `async` function if you want to use a sync function or `await` keyword
// let error = await getError(async () => {...})
// typeguards for typescript to be happy, since `error` will be `unknown` just like in a `catch` clause
// if (!expectedErrorGuard(error)) throw error;
// for example:
// if (!(error instanceof Error)) throw error;
expect(error.message).toContain("my error message")
expect(error.exitCode).toBe(1)
}) Implementation: class NoErrorThrownError extends Error {}
const getError = async (
erroring: Promise<unknown> | (() => Promise<unknown>)
) => {
try {
let promise = typeof erroring === "function" ? erroring() : erroring;
await promise;
throw new NoErrorThrownError();
} catch (error: unknown) {
if (error instanceof NoErrorThrownError) throw error;
return error;
}
}; Just realized this is exactly what the lint rule recommends 馃槄 so not my idea |
I wanted to add that implementing this (or the proposal to make The only ways to do that I'm currently aware of are by using a I'll probably dig into writing a custom matcher, but FWIW I think the usability of Jest would be significantly improved if it was easier to make assertions about errors other than the message exactly matching a particular string. |
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. |
This still seems like a legit issue to me (correct me if I'm wrong), so just commenting to keep the bot from killing it as stale (I couldn't remove the stale label) |
馃殌 Feature Proposal
.toThrow
and.rejects.toThrow
should return the error thrown.Motivation
When working with a project that uses ava I noticed their
.throws
and.throwsAsync
return the original error. It is very convenient.This would make it possible to never need the
expect.hasAssertions()
+try / catch
syntax.Example
Pitch
Because using
.toThrow
and.rejects.toThrow
overtry/catch
it prevents tests that don't fail because they no longer reject. #3917The text was updated successfully, but these errors were encountered: