-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Have assertions return booleans #2455
Comments
Hello, can i work on this ? |
@aviralch yea that'd be great, thanks! |
Hi, @novemberborn I am also attempting an update to this issue and wondering if I could be assigned to it too? Not sure if that is allowed, but I would like to work on this also. Thanks |
@pmascheroni let's give @aviralch a chance first. |
@novemberborn, @pmascheroni this is my first issue, I will try to do this and keep you updated by the end of of week. |
Hi, if no one is currently working on this and it is still open, i would like to contribute. |
@aviralch what do you think? |
Yup, sure go ahead. I am currently in middle of exams but if you need to get in touch lmk. |
Thank you @aviralch, and good luck! |
Hi, is it done ?, if not I can make a try ! Please let me know 👍 I would like to contribute |
Hi @RahulKannan12, I am currently working on it. |
Hi @BARICISSE , Okay great ! thanks for the information 👍 |
Hello is this ticket still open? If not, I would like to grab this! |
@mzzlillieee yes go for it. |
@mzzlillieee @novemberborn Hey yo! :) I have a draft PR for this issue that I believe is almost there. @novemberborn need a little guidance on how to update the test file -- this is my first issue. Would you mind if I pushed up what I had if @mzzlillieee is busy? |
@jmarkham828 No I don't mind. Something came up, so I won't be able to work on this issue until next week. |
@mzzlillieee awesome! @novemberborn would you mind adding me to this issue so I can push? |
@jmarkham828 sure, but you don't need to be assigned an issue to create a PR 😉 |
Does this issue still a contributer? |
@rShar01 this was picked up by @jmarkham828 quite recently, so maybe check in with them. |
Will work on this PR #2564 more this weekend -- took me a while to tie up some projects at work, but am free to work Saturday and Sunday! Like I said, @novemberborn, I wasn't quite sure how to update the test file, what do you think? |
@novemberborn , I'd like to take a crack at this issue. If @jmarkham828 is no longer active. |
@adiSuper94 both you and @Davidmh93 have expressed an interest. And we haven't yet heard back from @jmarkham828 but that's OK. I think if either of you could take their commits and add on top of them, based on my feedback in #2564 that'd be great. And then you could help each other as well if need be. Does that sound all right? |
I'm unable to pick this one up now sorry |
Can I work on this? |
@shailenpatel1 maybe you could pick up where #2586 left off? |
Merged in #2696. |
Spurred by conversation in #2449 and #1485 we'd like for our assertions to return booleans.
true
when they pass,false
when they fail. This does not apply to thethrows()
andthrowsAsync()
assertions.Failing assertions will fail the test, but do not throw exceptions. This means your test does not stop executing. Usually that's fine, but there are conditions where you're about to do something that uses a lot of resources, and it's already unnecessary. Returning booleans will help with this.
The assertions are defined here:
ava/lib/assert.js
Line 238 in 18aeac6
The type definition should be updated to reflect the new return type:
ava/index.d.ts
Line 41 in 18aeac6
Some tests should be added to (or existing test updated in) https://github.com/avajs/ava/blob/master/test-tap/assert.js.
The text was updated successfully, but these errors were encountered: