-
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
Report all assertion failures #261
Comments
Not sure we should have a
I think I like the idea of |
|
As for me, I can't find the use-case for a verbose mode in my mind. If you could help me with that, that'd be great :) If I have 10 assertions with 1st assertion causing the rest to fail, why do we need to see the other 9? Are there any examples in some projects or other test runners, where this functionality is needed/implemented? |
Usually, yes - you only need to see the first assertion. Though sometimes additional information can be helpful: var a = sum(2, 2); // => 4 (correct)
var b = sum(4, 4); // => 7 (wrong - for some weird reason)
var c = sum(a, b); // => 11 (correct for the wrong input).
t.is(c, 12); // this will fail (telling us there is something wrong - but not what the root of the problem is)
t.is(b, 8); // this will also fail and in this case is the more helpful information Granted, the solution above is pretty obvious - reorder the assertions. But that is not so easy to see in every problem. |
Here is an example where I wish I could have seen every failure: I could not use |
This shouldn't really bother us, since AVA is not trying to replicate tap/tape, but trying to stand out.
In that case, verbose mode should definitely be optional and dead-clear to enable. I'm not a fan of enabling verbose mode automatically for |
Been thinking hard about this one today. I've changed my mind about implicit verbose when using |
Just changed the title on this issue to "Report all assertion failures" given that we now have a If we were to run all assertions and print subsequent failures how would that work with features like #493? In that case if How does tap/tape handle these scenarios? |
As previously, no matter how hard I try I don't see a point in continuing test execution, when test had already failed. And it will for sure confuse users, when tests continue to run after failed assertions. I think we should close this. |
I will let @jamestalmage comment first, but I would close this too. |
We currently do continue test execution, and users aren't confused. We are already collecting every assertion result, so not doing anything with them is just wasteful. If we decide not to do this, then our assertions should throw Errors.
This: var tap = require('tap');
tap.test('foo', function (t) {
t.is(1, 2, 'failing');
t.is(1, 1, 'passing');
t.end();
}); Produces:
As @novemberborn brought up. There are times when a failed assertion certainly means the test will blow up: t.ok(foo); // if this assertion fails
t.ok(foo.bar); // then this will blow up If a test throws an error after a failed assertion, I say we assume this exact scenario has occurred. Suppress the thrown error and just report all the assertions leading up to it.
It would probably be helpful for tests like this. Having a list of what passed and what failed might be helpful in hunting down your mistake. You could argue that each of those assertions would be better written as separate tests, but the current way is far more convenient. |
Yea, having recently looked into the
To clarify, right now we also report the test blowing up? I'm +1 on best-effort reporting of all assertions, but not muddling the waters with exceptions after an assertion has failed. |
No. Right now we report just the first failed assertions. Anything that happens after that is ignored. It behaves almost like we threw an error to stop execution... but we didn't. |
OK I'm +1 on this. |
Sure, why not, although I've never needed this, we should make debugging failing tests as easy as possible. |
Is there any movement on this? I'd like to be able to see all failures in a single test without having to split them into multiple ones- I assume there's still no way of doing this yet? |
@sindresorhus Ha, I know. But sometimes things get fixed or change unrelated to an issue on Github, or there are many asking the same thing and people don't communicate between, so no harm in asking... imo! |
With #259, we have moved from only printing the last assertion failure, to only printing the first.
Once #243 (chainable methods) happens, I think we should allow printing of every failed assertion to give the most information possible.
Perhaps
only
mode should also just implyverbose
mode (you should have way less noise with a single test).The text was updated successfully, but these errors were encountered: