eslint-plugin-jest: [new rule] prefer-to-throw / consistent-expect-error
I would like to see a new rule named prefer-to-throw
, or consistent-expect-error
that would (preferably) default to the .toThrow
syntax.
If consistent-expect-error
is set to use the try/catch
syntax, an error should be thrown if expect.hasAssertions
(or a variation of) has not been set.
const throwSyncError = () => {
throw new Error('sync example');
};
const throwAsyncError = async () => {
throw new Error('async example');
};
describe('prefer-to-throw - sync', () => {
test('eslint pass', () => {
expect(() => throwSyncError()).toThrow('async example');
});
test('eslint error', () => {
expect.hasAssertions();
try {
throwSyncError();
} catch (error) {
expect(error.message).toEqual('sync example');
}
});
});
describe('prefer-to-throw - async', () => {
test('eslint pass', async () => {
await expect(throwAsyncError()).rejects.toThrow('async example');
});
test('eslint error', async () => {
expect.hasAssertions();
try {
await throwAsyncError();
} catch (error) {
expect(error.message).toEqual('async example');
}
});
});
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 17 (8 by maintainers)
re my response: I had just woken up and have not retained the context for this issue since it’s 6 months old, so I freeballed it to push you in the right direction 😂
I expected it to be “mostly right”, but should have mentioned “something like [code example]” to make that clearer.
@SimenB’s code example is actually also “almost” - it actually needs to be:
So to break down whats going on here, and why:
rejects
is required becausetoThrow
is sync, and promises don’t “throw” in thetry/catch
senserejects
requires a promise, which is why we callpromiseMeSomething
directly instead of in a functionMatcher error: received value must be a promise
.await
because otherwise the test will “pass” with a promise rejection error thatjest
won’t see.Overall I think at this point (thats again without having the context from 6 months ago), I would personally recommend using a utility function of some kind if you want to test things on the actual error:
The benefit to me is that the
getError
utility function is a clear easy to recognize pattern that solely “inverts” errors & returns by returning any errors that are thrown, and throwing if no errors are.That way you don’t duplicate that across your tests, where it’s possible make a typo that could still pass in your testbase. Granted we have tools to help reduce the likelihood of this, but it’s still possible.
(This is one of the reasons why
no-try-expect
was introduced).In addition, we’re balancing usefulness & time-saving-potential against complexity and maintainability:
Detecting if
hasAssertions
exists isn’t just a one-line function call - it requires parsing the whole function body and then some; that’s not including things other ways you can addexpect.hasAssertions
, such as as a hook for every test.However, improving the text is definitely something we could do easily; I’ve not looked into the message myself, but more than happy to discuss that 😃
almost - need to be
await expect(() => store.dispatch(actions.someFailingAction)).rejects.toThrow();
.We don’t have a really clean way if you wanna make multiple assertions on the error, though.
@jp7837 (it’s been another 6 months, so this is all iirc)
Yes, that’s the point. The idea is that you’re wanting to perform checks against the error, which will always fail against our
'no error thrown!'
error unless they were just testing that it’s an instance ofError
(or that the title was a string?).If you throw, then you’ll still get told the test failed, but not in a nice way because there’s a difference between an error being thrown and an assertion failing 😃
So, with the above helper these would all fail as expected:
The above will would all assert gracefully if no error was actually thrown 😃
@cartogram yeah, definitely!