Require explicit calls to loadTests and setupEmberOnerrorValidation

[ef4]

This makes start from ember-qunit really only responsible for starting tests, and splits test loading into loadTests() and the classical onError validation into setupEmberOnerrorValidation. It's necessary because test loading is a classic AMD concept that's not wanted in new-style apps building with Embroider. Also we want to drop the dependency on ember-cli-test-loader because that is a v1 addon.

To keep the classic behavior, the blueprint will change like:

+import { loadTests } from 'ember-qunit/test-loader';
-import { start } from 'ember-qunit';
+import { start, setupEmberOnerrorValidation } from 'ember-qunit';
+setupEmberOnerrorValidation();
+loadTests();
start();

Note that setupEmberOnerrorValidation was already an exported function, it was just missing a type declaration. We decided to drop this from happening automatically because it protects people during this upgrade: if they don't change their blueprint at all, they will get a failing test suite with no tests in it. Had we kept the automatic OnError validation, the presence of that test would cause a spurious passing test suite when you fail to call loadTests().

This is still draft because we need to confirm the TS support in some real apps both with and without moduleResolution: "nodenext". I tested this under both moduleResolution modes.

[ef4]

An important point: if you fail to do this blueprint update, you get a green but entirely empty test suite!
This is addressed.

[NullVoxPopuli]

could we do something like:

// ... ope.js
export const state = {
  requiresLoadTestsProbably : Boolean(globalThis.require),
  didNotLoadAMDTests: true,
}
export function setNeedToError(x) { state.didNotLoadAMDTests = x; }

// ... load-tests.js
import { setNeedToError } from './ope.js';

function loadTests() {
  setNeedToError(false);
}

// ... index.js
import { state } from './ope.js';

export function start() {
  if (state.requiresLoadTestsProbably && state.didNotLoadAMDTests) {
    throw new Error("loadTests() is needed!")
  } 
}

[Turbo87]

is this still compatible with ember-exam? or is it planned to update ember-exam to account for these changes?

[ef4]

It looks like ember-exam was already using loadTests: false:

https://github.com/ember-cli/ember-exam/blob/801200616b81024855c40037ac661e7957436337/addon-test-support/start.js#L29

which is now the default behavior, so I don't think this breaks anything that was working before.

[Turbo87]

which is now the default behavior, so I don't think this breaks anything that was working before.

then I'm doing something wrong in rust-lang/crates.io#10241 😅

[ef4]

I think that PR is accidentally replacing start from ember-exam/test-support/start with start from ember-qunit.

[Turbo87]

that's essentially what's happening in https://github.com/ember-cli/ember-exam/blob/main/addon-test-support/start.js, but it doesnt seem to work. I have a feeling that in https://github.com/ember-cli/ember-exam/blob/main/addon-test-support/-private/ember-exam-test-loader.js#L16 it fails to load the parent class 🤔

[ef4]

Ah, so yes, there's no exported TestLoader class now. I guess it would be easy enough to restore the export.

But also: the PR definitely does include a mistake about which start it's calling. I will review over there.

[ef4]

#1183

[Turbo87]

seems to have worked. thanks for the fast reply! :)