Ava: Too aggressive in declaring the test "finished"

[jamestalmage]

The following works in 0.18.2:

test(t => {
    t.notThrows(new Promise(resolve => {
        setTimeout(() => {
            t.pass();
            resolve();
        }, 100);
    }));
});

On master, it fails with:

Assertion passed, but test has already ended

We should wait until all pending assertions have resolved before declaring a test "finished" and throwing when new assertions show up.

Adding a call to t.plan() causes it to behave oddly going all the way back (we even added a test solidifying this odd behavior in #360):

test(t => {
   t.notThrows(new Promise(resolve => {
       t.plan(2);
       setTimeout(() => {
         t.pass();
         resolve();
       }, 100);
   }));
});

The above fails with:

Planned for 2 assertions, but got 1.

Similar results for as far back as I tested.

I think we should allow for added assertions, as long as we still have pending assertions. When t.plan is used, I don't think we should check for too few assertions until we've drained all pending assertions.

[novemberborn]

That's an interesting use case. I don't think it's a great use of t.notThrows() but it seems easier to make this work then to properly communicate why it doesn't.

[sindresorhus]

I'm tempted to say we require t.throws() to be awaited. I (and many users I've talked to) find it surprising that t.throws() works with a Promise in a "sync" test. Would be better to have a clear separation and only allow t.throws() with a promise in an async test (test(async t =>{})).

[sindresorhus]

I already always do:

test(async t => {
  await t.throws(…);
});

Instead of:

test(t => {
 t.throws(…);
});

The former is clearer by being explicitly async. It's easier to add stuff after the throws assertion and get expected order of execution. You can get the error without having to change things around. Simpler to document.

[novemberborn]

@sindresorhus I like that.

[jamestalmage]

How would we enforce that? Can you tell a function is async before calling it?

[novemberborn]

@jamestalmage see https://github.com/avajs/ava/pull/1335.