Add option to allow assertions after using assert.async

[trentmwillis]

Currently, if you use assert.async() and then have another assertion after calling the returned done() function, you get an error like so:

Assertion after the final `assert.async` was resolved

This is fine for most use cases, but within the Ember community we have some helpers which currently use assert.async() within some test helpers that execute asynchronously so that they can prevent tests from completing prematurely. However, this also means that assertions are likely to happen after done() is called.

So, I'd like to propose adding an option to assert.async() that enables you to have assertions after done() is called:

// assert.async( acceptCallCount, allowAssertionsAfter );
var done = assert.async( 1, true );
done();
assert.ok( true ); // no error!

Thoughts?

[platinumazure]

I'm curious to know what the rationale was for making it an error in the first place, actually. Depending on that rationale, I may be 👍 or 👎 to this idea.

[trentmwillis]

I'm unsure, and wasn't able to find a clear reason after digging through some old issues/PRs. I imagine the thinking is that done() should be the last thing called in a test so that you potentially avoid extra assertions happening after you intend for the test to be completed.

I think, however, that case is solved just by using assert.expect() and the fact that we error if an assertion occurs after a test has finished.

[platinumazure]

If we error for assertions after the test is done slash in a test that isn't the current test, then I'm 👍 to this enhancement.

[trentmwillis]

I've opened two PRs to get this working as expected.

Also, for reference, here is the issue from Ember that spurred this issue: ember-cli/ember-cli-qunit#162 (comment)

[Krinkle]

The concept of a "test is done" already exists in QUnit and is reached when all pending async handlers have been invoked (if any), and last remaining function stack yields.

I'm curious how we determine the latter at the moment, but assuming that it is in stable shape, I would support adopting this behaviour always instead of it being optional (should not be a breaking change).

For the API, I would prefer we avoid extra parameters to async(), especially boolean parameters.

[trentmwillis]

I'm curious how we determine the latter at the moment...

I believe this is done using setTimeout so we ensure the current function stack has emptied before trying to advance again.

...I would support adopting this behaviour always instead of it being optional (should not be a breaking change).

I am also on board with this (just went with a more conservative approach). It is almost the opposite actually (an un-breaking change?) since we'll no longer err in the given case.

[leobalter]

Fixed at #1148

[sukima]

Apologies for reviving an old issue but this topic missed an edge case that I think would be very useful say you have an event handler that runs an async task and then sets side effects on the element. The best way to test that would be to await on the assert.async().

Application code

export function setFooBar(element) {
  element.addEventListener('validate', async () => {
    element.foo = await getBarValueAsynchronously();
    element.dispatchEvent(new CustomEvent('validated'));
  });
}

Test code

test('sets foo on element after async task', async function(assert) {
  let { subject } = this;
  let validatedCalled = assert.async();

  setFooBar(subject);

  subject.addEventListener('validated', validatedCalled);
  subject.dispatchEvent(new CustomEvent('validate'));

  await validatedCalled; // How to do this?

  assert.equal(subject.foo, 'bar');
});

[sukima]

I found an alternative

test('sets foo on element after async task', async function(assert) {
  let { subject } = this;

  setFooBar(subject);

  await new Promise(resolve => {
    subject.addEventListener('validated', resolve);
    subject.dispatchEvent(new CustomEvent('validate'));
  });

  assert.equal(subject.foo, 'bar');
});

[Krinkle]

@sukima Yep, that's a good way to do it. Another way could be to place your assertion in the callback directly:

test('sets foo on element after async task', assert => {
  const done = assert.async();
  setFooBar(subject);

  subject.addEventListener('validated', () => {
    assert.equal(subject.foo, 'bar');
    done();
  });

  subject.dispatchEvent(new CustomEvent('validate'));
});

Although I generally recommend against this, because it means if the test fails you have to wait until a timeout. And if you have more than one event, it can get rather complicated to do this right. You can use assert.step() as a more resilient and strict way of asserting values that are expected to be found in a specific order and no more than once. With assert.step() you would also want to have a way still to await until the expected work is completed before callling assert.verifySteps(). We support async functions natively so that you can await within the test function between the "prep/act" and "assert" parts of your test case.