Output every failed assertion or just the first one (currently outputs just the last one)

[jamestalmage]

test('foo', t => {
    t.plan(2);
    var a = 2;
    var b = 3;
    var c = 4;
    t.ok(a === b);
    t.ok(b === c);
});

output:

 ✖ foo   
  t.ok(b === c)
       |     | 
       3     4 


  1 test failed

It would be better if either:

1. Both assertion failures printed.
2. If there can only be one, print the first.

[Qix-]

This is strange. BDD/TDD style assertions should throw if the assertion fails. Can you comment out t.ok(b === c); and see if it still throws?

Also, you're missing t.end().

[jamestalmage]

Is t.end() required when using t.plan()? I thought it was one or the other.

[jamestalmage]

test('foo', t => {
    t.plan(2);
    var a = 2;
    var b = 3;
    var c = 4;

    try {
        t.ok(a === b);
        console.log('a === b did NOT throw');
    } catch (e) {
        console.log('a === b DID throw');
    }

    try {
        t.ok(b === c);
        console.log('b === c did NOT throw');
    } catch (e) {
        console.log('a === b DID throw');
    }

    t.end();
});

output:

a === b did NOT throw
b === c did NOT throw
  ✖ foo   
  t.ok(b === c)
       |     | 
       3     4 

[Qix-]

This is a bug within t.ok(), then...

[jamestalmage]

This is a bug within t.ok(), then...

No, I don't think so. The API seems modeled after node-tap. Search those docs for reference to bailout (implying that it does not bailout on a test failure by default).

It is a bit confusing coming from mocha, etc. But can be fairly handy (especially when your test framework actually shows you every failed assertion 😉). It might just be that the second/third/... failed assertion is the one that has the truly helpful hint that helps you identify the problem.

This is why I prefer Option 1 from above

It would be better if either:

1. BothAll assertion failures printed. <-- Do this one!
2. If there can only be one, print the first.

[Qix-]

I agree that showing all failed assertions would be nice, instead of just dying on the first one.

[sindresorhus]

Yes, it was modeled after node-tap. You can use the --fail-fast flag if you want it to bailout after the first failure.

I'm open to supporting showing all assertion failures. I just didn't see the point when I first made AVA. There has been some times that I could have used this since then, though. Note to self: No matter what we go for we should at least more clearly document the behavior.

@vdemedes What do you think?

[vadimdemedes]

Honestly, I'm not sure about this one. I agree with @Qix-, it is a bit weird, that failed assertion does not throw. I realize it would be handy in some cases, but it also has its downsides.

Imagine I have a test with Promise A, 5 assertions, then another Promise B inside and 5 more assertions. If it does not fail early, the whole code will be executed and I'll get a ton of errors just for one test. And where I'll start looking to fixing it? Right, from the first failed assertion.

[jamestalmage]

@sindresorhus even with --fail-fast, the above code still shows the last assertion. Probably because the code is synchronous and ava assertions don't throw (I am guessing --fail-fast does not make assertions start throwing, but just short circuits the wait for t.end()/t.plan() in async situations).

I think we all agree that the first test failure is the most important, but I don't think it is necessary to change to a throwing behavior if you don't like the verbosity of printing out all those failures - just don't print them (or print them in an ultra compact form).

Here is the current output:

  ✔ foo
  ✖ bar   
  t.is(b, c)
       |  | 
       3  4 

  1 test failed

  1. bar
  AssertionError:   
  t.is(b, c)
       |  | 
       3  4 
    at Test.fn (/Users/jamestalmage/WebstormProjects/promisify-firebase/test.js:86:5)

It might be cool to print all the assertions with dots and x's:

  ✔ foo .........
  ✖ bar ....✖.....✖  
  t.is(b, c)
       |  | 
       3  4 

  1 test failed

  1. bar
  AssertionError:   
  t.is(b, c)
       |  | 
       3  4 
    at Test.fn (/Users/jamestalmage/WebstormProjects/promisify-firebase/test.js:86:5)

I could also get behind slightly more output:

  ✔ foo .........
  ✖ bar ....✖.....✖  
  t.is(a, b)
       |  | 
       2  3 
  t.is(b, c)                      // compact output from every assertion failure up top
       |  | 
       3  4 

  1 test failed

  1. bar                          // full output of only the first assertion failure down below. 
  AssertionError:   
  t.is(a, b)
       |  | 
       2  3 
    at Test.fn (/Users/jamestalmage/WebstormProjects/promisify-firebase/test.js:86:5)

Also, the node-tap, bailout feature is not without merits. It helps with the situation @vdemedes describes: "If this first assertion fails, there is no point continuing (the rest will fail), throw and bail so my output is not cluttered with a bunch of errors". (I am talking about the per-assertion bailout option. --fail-fast already provides a test-suite-wide solution).

[vadimdemedes]

@jamestalmage I liked your idea on this:

It might be cool to print all the assertions with dots and x's:

But, there's one reason it is just not possible with AVA - Parallelism. AVA executes all tests in parallel, so there's no way we can modify previous output, because other tests might've displayed in the meantime.

[jamestalmage]

By the time the promise test bar has resolved and we are printing ✖ bar, we should know the state of every assertion that happened inside that test.

[schnittstabil]

there's no way we can modify previous output

We haven't to, ✖ bar ....✖.....✖ is outputted afterwards, but I would agree that the dots and x's would indicate some order, which is misleading.

[jamestalmage]

There is order within an individual test.

[vadimdemedes]

@jamestalmage that is correct, my oversight ;)

[vadimdemedes]

I was just thinking about displaying the assert progress live.

[schnittstabil]

@jamestalmage The syntactical order need not match the run-time order:

setTimeout(() => t.ok(true), 2000);
setTimeout(() => t.fail(),  500);

[jamestalmage]

@schnittstabil

Two options there:

1. Just use runtime order. Easy for us. Users just need to be made to understand how it works.
2. power-assert already embeds the line number of the assertion when it instruments code. It would be possible to write a custom reporter that exposed line numbers to ava, and we could give syntactical ordering.

[twada]

@jamestalmage @schnittstabil

FYI: extracting line number from power-assert would be easy enough.

If we change saveContextOnRethrow option to true in lib/enhance-assert.js,

    empower(assert,
        powerAssertFormatter({
            renderers: [
                powerAssertRenderers.AssertionRenderer,
                powerAssertRenderers.SuccinctRenderer
            ]
        }),
        {
            destructive: true,
            modifyMessageOnRethrow: true,
            saveContextOnRethrow: true,
            patterns: module.exports.PATTERNS
        }
    );

then powerAssertContext property appears in thrown AssertionError.

powerAssertContext has structure like this

{
    source: {
        content: "t.is(foo, bar)",
        filepath: "/path/to/some_test.js",
        line: 1
    },
    args: [
        {
            value: "foo",
            events: [
                {
                    value: "foo",
                    espath: "arguments/0"
                }
            ]
        },
        {
            value: "bar",
            events: [
                {
                    value: "bar",
                    espath: "arguments/1"
                }
            ]
        }
    ]
}

source.line is the line number of original assertion.

[sindresorhus]

I'm ok with switching to showing the first assertion instead of the last, but I'm still undecided about showing multiple asserts.

[jamestalmage]

What if we created an alternate one-line format for all the subsequent failures:

power-assert-js/power-assert-formatter#21

[Qix-]

Showing the last assertion makes absolutely zero sense.

const ctx = new Context('test', 'hello\n new\n  again\n back\n sustain\nroot');
t.ok(ctx.scope.spaces === 0);
t.ok(ctx.inIndent === true);
t.ok(ctx.skip(5));
t.ok(ctx.inIndent === false);
t.ok(ctx.peek() === '\n');
t.ok(ctx.read() === '\n');
t.ok(ctx.inIndent === true);
t.ok(ctx.peek() === ' ');
t.ok(ctx.scope.spaces === 0);
t.ok(ctx.read() === ' ');
t.ok(ctx.inIndent === true);
t.ok(ctx.scope.spaces === 1);
t.end();

In the above, the last assertion is the only one that shows up, leading me to initially believe everything leading up to it was ok. Low and behold, I commented it out and the one before it (ctx.inIndent === true) also failed the assertion. Now I have to comment out every single assertion until I find the one that is actually the problem.

Incredibly counter-intuitive.

[sindresorhus]

@Qix- Totally agree. Not sure why I did this initially.