ava icon indicating copy to clipboard operation
ava copied to clipboard

Report all assertion failures

Open jamestalmage opened this issue 8 years ago • 18 comments

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.

test.verbose('title', t => {
  t.fail('a');
  t.fail('b');
});

Perhaps only mode should also just imply verbose mode (you should have way less noise with a single test).

jamestalmage avatar Nov 23 '15 07:11 jamestalmage

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.

sindresorhus avatar Nov 23 '15 07:11 sindresorhus

ava -v would be wonderful. Not sure about individual tests, though I see the point being made.

Qix- avatar Nov 23 '15 09:11 Qix-

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?

vadimdemedes avatar Nov 23 '15 22:11 vadimdemedes

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.

jamestalmage avatar Nov 23 '15 22:11 jamestalmage

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.

jamestalmage avatar Nov 23 '15 22:11 jamestalmage

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().

vadimdemedes avatar Nov 24 '15 10:11 vadimdemedes

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.

sindresorhus avatar Nov 24 '15 10:11 sindresorhus

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?

novemberborn avatar Feb 29 '16 14:02 novemberborn

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.

vadimdemedes avatar Feb 29 '16 14:02 vadimdemedes

I will let @jamestalmage comment first, but I would close this too.

sindresorhus avatar Feb 29 '16 15:02 sindresorhus

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.

jamestalmage avatar Mar 21 '16 20:03 jamestalmage

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.

novemberborn avatar Mar 22 '16 21:03 novemberborn

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.

jamestalmage avatar Mar 22 '16 22:03 jamestalmage

OK I'm +1 on this.

novemberborn avatar Mar 23 '16 11:03 novemberborn

Sure, why not, although I've never needed this, we should make debugging failing tests as easy as possible.

sindresorhus avatar Mar 26 '16 08:03 sindresorhus

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 avatar Feb 02 '18 17:02 MaffooBristol

@MaffooBristol https://twitter.com/slicknet/status/782274190451671040 :)

sindresorhus avatar Feb 03 '18 07:02 sindresorhus

@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!

MaffooBristol avatar Feb 03 '18 14:02 MaffooBristol