Open jamestalmage opened 8 years ago
Not sure we should have a verbose
modifier. I don't see why would want to augment just some tests. I'd rather have a --verbose
flag for all tests.
Perhaps only mode should also just imply verbose mode (you should have way less noise with a single test).
I think I like the idea of only
triggering verbose mode for those tests.
ava -v
would be wonderful. Not sure about individual tests, though I see the point being made.
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?
tap
/tape
shows you every assertion failure.
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: https://github.com/jamestalmage/babel-plugin-detective/blob/master/test/babel-6-test.js#L165
I could not use deepEquals
, and using assert
which throws at the first error was annoying because you can not know ahead of time which failure will highlight your mistake.
tap/tape shows you every assertion failure.
This shouldn't really bother us, since AVA is not trying to replicate tap/tape, but trying to stand out.
Though sometimes additional information can be helpful
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 test.only()
.
Been thinking hard about this one today. I've changed my mind about implicit verbose when using test.only()
. That's just too magic. As for a --verbose
flag, I guess that could be useful, but I've honestly never needed it. I think we should punt it for now, but leave this open for additional comments. We can make a decisions later.
Just changed the title on this issue to "Report all assertion failures" given that we now have a --verbose
reporter.
If we were to run all assertions and print subsequent failures how would that work with features like #493? In that case if const err = t.throws(fn)
fails because fn
does not throw an error then any subsequent assertions or even code is guaranteed to blow up. It's the same in @jamestalmage's example where if t.same(expressions, [])
then code like expressions[0].type
will blow up.
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.
And it will for sure confuse users, when tests continue to run after failed assertions
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.
How does tap/tape handle these scenarios?
This:
var tap = require('tap');
tap.test('foo', function (t) {
t.is(1, 2, 'failing');
t.is(1, 1, 'passing');
t.end();
});
Produces:
test.js
foo
1) failing
✓ passing
1 passing (296.424ms)
1 failing
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.
I don't see a point in continuing test execution
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.
We currently do continue test execution, and users aren't confused.
Yea, having recently looked into the t.throws()
stuff I suppose we do! And certain assertions are asynchronous, too.
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.
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.
To clarify, right now we also report the test blowing up?
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?
@MaffooBristol https://twitter.com/slicknet/status/782274190451671040 :)
@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).