Update Test adapter to support QUnit 2.0

[trentmwillis]

QUnit 2.0 was released today, yay! But, unfortunately, Ember's test adapter for QUnit currently relies on QUnit.stop which has been removed. The recommended upgrade path is to use assert.async() which works for most cases, but since the asyncStart of the test adapter is invoked behind an abstraction, passing through the assert object might be a bit difficult.

Opening this up for some discussion on thoughts to get around this. I have a few thoughts but they all seem too hacky right now. I believe I have a solid solution, will open a PR soonish.

[trentmwillis]

I found a way to invoke assert.async within the test adapter, but this has issues when assertions come after done is called. Consider a test like so:

visit('/post/').then(() => assert.ok(true, 'visited post'));

This is essentially executed as:

var done = assert.async();
doActualVisit().then(done).then(() => assert.ok(true, 'visited post'));

Since the final assertion comes after the done call, QUnit throws an error. At this point I'm unsure if there is a good way to make this work within the existing test helper setup, but it may also point to gap that assert.async does not cover after switching from QUnit.start / Qunit.stop.

cc @leobalter for any additional insight on potential solutions

[pixelhandler]

@trentmwillis does your solution recommend a change to the test adapter API?

With Using assert.async is asyncStart even needed? asyncEnd seems to map to using var done = assert.async(); and calling done().

export default Adapter.extend({
  asyncStart() {
    QUnit.stop();
  },
  asyncEnd() {
    QUnit.start();
  },
  exception(error) {
    ok(false, inspect(error));
  }
});

This seems like the use case for asyncStart: https://github.com/emberjs/ember.js/blob/master/packages/ember-testing/lib/ext/application.js#L188-L192

function helper(app, name) {
  let fn = helpers[name].method;
  let meta = helpers[name].meta;
  if (!meta.wait) {
    return (...args) => fn.apply(app, [app, ...args]);
  }

  return (...args) => {
    let lastPromise = run(() => resolve(getLastPromise()));

    // wait for last helper's promise to resolve and then
    // execute. To be safe, we need to tell the adapter we're going
    // asynchronous here, because fn may not be invoked before we
    // return.
    asyncStart();
    return lastPromise.then(() => fn.apply(app, [app, ...args])).finally(asyncEnd);
  };
}

[trentmwillis]

@pixelhandler I don't currently have a solution. The one solution I did have, fails in the use case I describe above.

The only way that asyncStart would be unneeded is if we treated the entire test as async and had some reliable way to hooking into the end of the test. Since that doesn't exist (to my knowledge), then we need a way of denoting when test helpers have gone asynchronous (assert.async) and then when they have finished async behavior (done), but QUnit's current assumption is that when async behavior is done, no more assertions should occur.

[pixelhandler]

@trentmwillis yes seems like there needs some discussion as to a path ahead regarding QUnit 2.0

I'm curious if this should be an RFC issues as this is not technically an Ember.js bug.

[pixelhandler]

@trentmwillis Actually the discussion may be better if added to the thread on the "Grand Testing Unification RFC" - https://github.com/rwjblue/rfcs/blob/42/text/0000-grand-testing-unification.md#async--await

Seems that the proposal does not mention asyncStart but instead proposes async/await.

It may be the case that the testing suite may not use QUnit 2 until that above propose is settled.

Perhaps continue discussion on the topic of the API for test adapter on that RFC.

[pixelhandler]

@trentmwillis this issue is also related: #10484

[trentmwillis]

I have pulled together a potential fix in #13702. I don't think this is the best possible solution, but I don't think a "best" solution would be possible without some larger rethinking of how tests handle async currently.

I am definitely onboard with the Grand Testing Unification RFC and I think the whole async/await proposal will make this easier in the future, but I also don't want the community to need to wait indefinitely to be able to upgrade QUnit. Especially since we are actively trying to roll out features that have been requested from the Ember community (better module filtering and before/after hooks for example)

[rwjblue]

Closing this, ember-cli-qunit@3.0 uses ember-qunit@1.0.0-beta.1 which includes and updated QUnit adapter that is compatible with QUnit 2.0.