Core: Remove deprecated QUnit.load()

* Move scheduleBegin() code into QUnit.start().
* Remove internal config.pageLoaded, unused.
* Remove redundant `config.blocking` assignment, already set by begin().

== Reasoning for DOM-ready delay in QUnit.start ==

After the above changes, /test/startError.html failed when run
via grunt-contrib-qunit (e.g. CI) due to a Chrome timeout with no
test results. It passed in a browser. I realised that this is because
the bridge is injected after QUnit.start() and our tiny test were
already finishesd.

I believe this would affect even simple cases where someone sets
autostart=false and correctly calls QUnit.start(). This worked before
because legacy QUnit.load/QUnit.autostart was implicitly delaying the
begin() call even if you don't use autostart or call load+start both.
The reason is the `config.pageLoaded`. If the user disables autostart,
it is common for them to use async code loading (e.g. AMD) and thus
start manually after DOM-ready. But, it is entirely valid for them
to be ready before DOM-ready in some cases. In that case, our legacy
logic was still kicking in by re-enabling autostart and then waiting
for the original html.js event handler for window.onload to call
QUnit.start() before we "really" begin.

I don't remember if that was a lazy way to resolve conflicts between
the two, or a deliberate feature. We deprecated the conflict handling
part of this in QUnit 2 and this patch removes that. But, it seems
this was also serving as a feature to always ensure we wait for
DOM-ready no matter what, which is actually very useful to making sure
that integrations and reporter plugins have a chance to listen for
"runStart".

Solution: If you call QUnit.start(), and there is a document, and
we've not reached `document.readyStart=complete` (i.e. you called it
manually, most likely with autostart=false), then we will now still
explicitly wait for window.onload before calling begin().

For all intents and purposes, this delay is now invisible and internal
to QUnit. We do consider QUnit to have "started" during this delay.
We were already using setTimeout previously between `start` and `begin`
and this is effectively and extension of that.

Ref #1084

docs/api/QUnit/load.md

@@ -4,10 +4,12 @@ title: QUnit.load()
 excerpt: Inform the test runner that code has finished loading.
 groups:
   - deprecated
+  - removed
 redirect_from:
   - "/QUnit/load/"
 version_added: "1.0.0"
 version_deprecated: "2.21.0"
+version_removed: "unreleased"
 ---
 
 `QUnit.load()`
@@ -22,6 +24,7 @@ As of [QUnit 2.1.1](https://github.com/qunitjs/qunit/releases/tag/2.1.1), calls
 
 ## Changelog
 
+| UNRELEASED | Removed.
 | [QUnit 2.21.0](https://github.com/qunitjs/qunit/releases/tag/2.21.0) | Deprecated. Use [`QUnit.start()`](./start.md) instead.
 | [QUnit 2.1.1](https://github.com/qunitjs/qunit/releases/tag/2.1.1) | `QUnit.start()` no longer requires calling `QUnit.load()` first.
 

src/core.js

@@ -33,9 +33,6 @@ config.currentModule.suiteReport = runSuite;
 
 config.pq = new ProcessingQueue(test);
 
-let globalStartCalled = false;
-let runStarted = false;
-
 // Figure out if we're running the tests from a server or not
 QUnit.isLocal = (window && window.location && window.location.protocol === 'file:');
 
@@ -66,46 +63,35 @@ extend(QUnit, {
   skip: test.skip,
   only: test.only,
 
-  start: function (count) {
+  start: function () {
     if (config.current) {
-      throw new Error('QUnit.start cannot be called inside a test context.');
-    }
-
-    const globalStartAlreadyCalled = globalStartCalled;
-    globalStartCalled = true;
-
-    if (runStarted) {
-      throw new Error('Called start() while test already started running');
-    }
-    if (globalStartAlreadyCalled || count > 1) {
-      throw new Error('Called start() outside of a test context too many times');
+      throw new Error('QUnit.start cannot be called inside a test.');
     }
-    if (config.autostart) {
-      throw new Error('Called start() outside of a test context when ' +
-        'QUnit.config.autostart was true');
+    if (config._runStarted) {
+      if (document && config.autostart) {
+        throw new Error('QUnit.start() called too many times. Did you call QUnit.start() in browser context when autostart is also enabled? https://qunitjs.com/api/QUnit/start/');
+      }
+      throw new Error('QUnit.start() called too many times.');
     }
 
-    // Until we remove QUnit.load() in QUnit 3, we keep `pageLoaded`.
-    // It no longer serves any purpose other than to support old test runners
-    // that still call only QUnit.load(), or that call both it and QUnit.start().
-    if (!config.pageLoaded) {
-      // If the test runner used `autostart = false` and is calling QUnit.start()
-      // to tell is their resources are ready, but the browser isn't ready yet,
-      // then enable autostart now, and we'll let the tests really start after
-      // the browser's "load" event handler calls autostart().
-      config.autostart = true;
-
-      // If we're in Node or another non-browser environment, we start now as there
-      // won't be any "load" event. We return early either way since autostart
-      // is responsible for calling scheduleBegin (avoid "beginning" twice).
-      if (!document) {
-        QUnit.autostart();
-      }
+    config._runStarted = true;
 
-      return;
+    // Add a slight delay to allow definition of more modules and tests.
+    if (document && document.readyState !== 'complete' && setTimeout) {
+      // In browser environments, if QUnit.start() is called very early,
+      // still wait for DOM ready to ensure reliable integration of reporters.
+      window.addEventListener('load', function () {
+        setTimeout(function () {
+          begin();
+        });
+      });
+    } else if (setTimeout) {
+      setTimeout(function () {
+        begin();
+      });
+    } else {
+      begin();
     }
-
-    scheduleBegin();
   },
 
   onUnhandledRejection: function (reason) {
@@ -114,37 +100,6 @@ extend(QUnit, {
     onUncaughtException(reason);
   },
 
-  load: function () {
-    Logger.warn('QUnit.load is deprecated and will be removed in QUnit 3.0.' +
-      ' https://qunitjs.com/api/QUnit/load/');
-
-    QUnit.autostart();
-  },
-
-  /**
-   * @internal
-   */
-  autostart: function () {
-    config.pageLoaded = true;
-
-    // Initialize the configuration options
-    // TODO: Move this to config.js in QUnit 3.
-    extend(config, {
-      started: 0,
-      updateRate: 1000,
-      autostart: true,
-      filter: ''
-    }, true);
-
-    if (!runStarted) {
-      config.blocking = false;
-
-      if (config.autostart) {
-        scheduleBegin();
-      }
-    }
-  },
-
   stack: function (offset) {
     offset = (offset || 0) + 2;
     return sourceFromStacktrace(offset);
@@ -153,25 +108,12 @@ extend(QUnit, {
 
 registerLoggingCallbacks(QUnit);
 
-function scheduleBegin () {
-  runStarted = true;
-
-  // Add a slight delay to allow definition of more modules and tests.
-  if (setTimeout) {
-    setTimeout(function () {
-      begin();
-    });
-  } else {
-    begin();
-  }
-}
-
 function unblockAndAdvanceQueue () {
   config.blocking = false;
   config.pq.advance();
 }
 
-export function begin () {
+function begin () {
   if (config.started) {
     unblockAndAdvanceQueue();
     return;

src/core/config.js

@@ -10,8 +10,7 @@ const config = {
   // HTML Reporter: Modify document.title when suite is done
   altertitle: true,
 
-  // TODO: Move here from /src/core.js in QUnit 3.
-  // autostart: true,
+  autostart: true,
 
   // HTML Reporter: collapse every test except the first failing test
   // If false, all failing tests will be expanded
@@ -25,7 +24,7 @@ const config = {
   failOnZeroTests: true,
 
   // Select by pattern or case-insensitive substring match against "moduleName: testName"
-  filter: undefined,
+  filter: '',
 
   // TODO: Make explicit in QUnit 3.
   // fixture: undefined,
@@ -62,8 +61,7 @@ const config = {
   // so that the browser can visually paint DOM changes with test results.
   // This also helps avoid causing browsers to prompt a warning about
   // long-running scripts.
-  // TODO: Move here from /src/core.js in QUnit 3.
-  // updateRate: 1000,
+  updateRate: 1000,
 
   // HTML Reporter: List of URL parameters that are given visual controls
   urlConfig: [],
@@ -122,11 +120,11 @@ const config = {
   // Internal: ProcessingQueue singleton, created in /src/core.js
   pq: null,
 
-  // Internal: Created in /src/core.js
-  // TODO: Move definitions here in QUnit 3.0.
-  // started: 0,
+  // Internal: Used for runtime measurement to runEnd event and QUnit.done.
+  started: 0,
 
   // Internal state
+  _runStarted: false,
   _deprecated_timeout_shown: false,
   blocking: true,
   callbacks: {},

src/html-reporter/html.js

@@ -1070,10 +1070,18 @@ const stats = {
     testItem.className = 'fail';
   });
 
+  function autostart () {
+    // Check as late as possible because if projecst set autostart=false,
+    // they generally do so in their own scripts, after qunit.js.
+    if (config.autostart) {
+      QUnit.start();
+    }
+  }
+
   if (document.readyState === 'complete') {
-    QUnit.autostart();
+    autostart();
   } else {
-    addEvent(window, 'load', QUnit.autostart);
+    addEvent(window, 'load', autostart);
   }
 
   // Wrap window.onerror. We will call the original window.onerror to see if

test/startError.js

@@ -1,34 +1,42 @@
 /* eslint-env browser */
 
+QUnit.config.autostart = false;
+
+// Real
+QUnit.start();
+
+// Bad 1
+QUnit.config.autostart = true;
 try {
-  QUnit.config.autostart = true;
   QUnit.start();
 } catch (thrownError) {
   window.autostartStartError = thrownError;
 }
 
+// Bad 2
+QUnit.config.autostart = false;
 try {
   QUnit.start();
 } catch (thrownError) {
   window.tooManyStartsError = thrownError;
 }
 
-QUnit.module('global start unrecoverable errors');
+QUnit.module('startError');
 
-QUnit.test('start() throws when QUnit.config.autostart === true', function (assert) {
-  assert.equal(window.autostartStartError.message,
-    'Called start() outside of a test context when QUnit.config.autostart was true');
+QUnit.test('start() with autostart enabled [Bad 1]', function (assert) {
+  assert.propContains(window.autostartStartError,
+    { message: 'QUnit.start() called too many times. Did you call QUnit.start() in browser context when autostart is also enabled? https://qunitjs.com/api/QUnit/start/' });
 });
 
-QUnit.test('Throws after calling start() too many times outside of a test context', function (assert) {
-  assert.equal(window.tooManyStartsError.message,
-    'Called start() outside of a test context too many times');
+QUnit.test('start() again [Bad 2]', function (assert) {
+  assert.propContains(window.tooManyStartsError,
+    { message: 'QUnit.start() called too many times.' });
 });
 
-QUnit.test('QUnit.start cannot be called inside a test context.', function (assert) {
+QUnit.test('start() inside a test', function (assert) {
   assert.throws(function () {
     // eslint-disable-next-line qunit/no-qunit-start-in-tests
     QUnit.start();
   },
-  /QUnit\.start cannot be called inside a test context\./);
+  new Error('QUnit.start cannot be called inside a test.'));
 });