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

Allow throws / throwsAsync to work with any value, not just errors #2517

Closed
novemberborn opened this issue Jun 14, 2020 · 12 comments · Fixed by #3245
Closed

Allow throws / throwsAsync to work with any value, not just errors #2517

novemberborn opened this issue Jun 14, 2020 · 12 comments · Fixed by #3245

Comments

@novemberborn
Copy link
Member

t.throws() and t.throwsAsync() require the resulting exception to be a proper error:

ava/lib/assert.js

Lines 147 to 154 in 952a017

if (!isError(actual)) {
throw new AssertionError({
assertion,
message,
savedError,
values: [formatWithLabel(`${prefix} exception that is not an error:`, actual)]
});
}

I propose we add an any: boolean option to the expectations object used for these assertions. If true it won't cause a failed assertion when the exception is not an error.

We need to update the validation logic here to allow this property:

function validateExpectations(assertion, expectations, numberArgs) { // eslint-disable-line complexity

Don't forget to update the type definition:

ava/index.d.ts

Line 11 in 952a017

export type ThrowsExpectation = {

If any is true, we should try and change the typing of the return value to be unknown:

ava/index.d.ts

Line 252 in 952a017

<ThrownError extends Error>(fn: () => any, expectations?: ThrowsExpectation | null, message?: string): ThrownError;

ava/index.d.ts

Line 263 in 952a017

<ThrownError extends Error>(fn: () => PromiseLike<any>, expectations?: null, message?: string): Promise<ThrownError>;

And update our own tests:

test('.throws()', gather(t => {

test('.throwsAsync()', gather(t => {

See also the discussion in #1841.

@sindresorhus
Copy link
Member

I think we should add a short note to the docs that if the user is in control of the error, they should prefer to make it a proper Error.

@randomicon00
Copy link

randomicon00 commented Aug 18, 2020

Hello @novemberborn & @sindresorhus, I would like to tackle this issue if possible.

Thank you,

@novemberborn
Copy link
Member Author

@randomicon00 that's great go for it 😄

@randomicon00
Copy link

@novemberborn Will do thanks :)

@dmaevsky

This comment has been minimized.

@adiSuper94
Copy link
Contributor

@novemberborn I'd like to take a crack at this.

@randomicon00
Copy link

It assigned to me, I still didn't complete it but you can try I guess. I am still working on it.

@adiSuper94
Copy link
Contributor

@randomicon00 , it's cool if you are working on it. I though that it was inactive, so asked.

@travis

This comment was marked as off-topic.

@adiSuper94
Copy link
Contributor

adiSuper94 commented Aug 28, 2023

@novemberborn I'd still like to take a crack at this.

@novemberborn
Copy link
Member Author

By all means @adiSuper94! I'm low on time but I do want this in for AVA 6. Hopefully you get to it before I do 😀

@adiSuper94
Copy link
Contributor

I have added a PR that tries to fix this.
@novemberborn your comments outlining the places for code change was very helpful. Thank you.

If any is true, we should try and change the typing of the return value to be unknown

I am a bit confused about this. The signature of the throws/throwsAsync function itself doesn't seem change.
The return type of of the function passed to throws/throwsAsync is any.
And even if it throws a non error value, wouldn't the return type still remain the same?

If I am missing anything could you explain how this can be done ?

novemberborn added a commit that referenced this issue Sep 11, 2023
Fixes #2517.

Co-authored-by: Mark Wubben <mark@novemberborn.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants