Browser: Finish extraction of browser runner from HTML Reporter

This refactors the initFixture and initUrlconfig code to be an exported
callable so that it is safe to define without side-effects and can then
be called conditionally.

The previous code was already conditionally with an inline check
for window/document. This is now centralised in prep for making it
further conditional on whether or not QUnit was already exported,
thus making it safe to load QUnit twice (e.g. ESM and CJS without
split-brain conflict). Tracked at
#1551.

Follows-up e1e03e6.

Closes #1118.

src/browser/browser-runner.js

@@ -0,0 +1,93 @@
+import initFixture from './fixture';
+import initUrlConfig from './urlparams';
+
+export function initBrowser (QUnit, window, document) {
+  // Report uncaught exceptions to QUnit.
+  //
+  // Wrap and preserve any pre-existing window.onerror.
+  // An existing handller can "accepts" the erorr by returning true.
+  //
+  // Returning true from window.onerror suppresses the browser's default error
+  // reporting. Likewise, we will also not report it in that case.
+  const originalWindowOnError = window.onerror;
+  window.onerror = function (message, fileName, lineNumber, columnNumber, errorObj, ...args) {
+    let ret = false;
+    if (originalWindowOnError) {
+      ret = originalWindowOnError.call(
+        this,
+        message,
+        fileName,
+        lineNumber,
+        columnNumber,
+        errorObj,
+        ...args
+      );
+    }
+
+    // Treat return value as window.onerror itself does,
+    // Only do our handling if not suppressed.
+    if (ret !== true) {
+      // If there is a current test that sets the internal `ignoreGlobalErrors` field
+      // (such as during `assert.throws()`), then the error is ignored and native
+      // error reporting is suppressed as well. This is because in browsers, an error
+      // can sometimes end up in `window.onerror` instead of in the local try/catch.
+      // This ignoring of errors does not apply to our general onUncaughtException
+      // method, nor to our `unhandledRejection` handlers, as those are not meant
+      // to receive an "expected" error during `assert.throws()`.
+      if (QUnit.config.current && QUnit.config.current.ignoreGlobalErrors) {
+        return true;
+      }
+
+      // According to
+      // https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror,
+      // most modern browsers support an errorObj argument; use that to
+      // get a full stack trace if it's available.
+      const error = errorObj || new Error(message);
+      if (!error.stack && fileName && lineNumber) {
+        error.stack = `${fileName}:${lineNumber}`;
+      }
+      QUnit.onUncaughtException(error);
+    }
+
+    return ret;
+  };
+
+  window.addEventListener('unhandledrejection', function (event) {
+    QUnit.onUncaughtException(event.reason);
+  });
+
+  QUnit.on('runEnd', function (runEnd) {
+    if (QUnit.config.altertitle && document.title) {
+      // Show ✖ for good, ✔ for bad suite result in title
+      // use escape sequences in case file gets loaded with non-utf-8
+      // charset
+      document.title = [
+        (runEnd.status === 'failed' ? '\u2716' : '\u2714'),
+        document.title.replace(/^[\u2714\u2716] /i, '')
+      ].join(' ');
+    }
+
+    // Scroll back to top to show results
+    if (QUnit.config.scrolltop && window.scrollTo) {
+      window.scrollTo(0, 0);
+    }
+  });
+
+  initFixture(QUnit, document);
+  initUrlConfig(QUnit);
+  QUnit.reporters.perf.init(QUnit);
+  QUnit.reporters.html.init(QUnit);
+
+  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 (QUnit.config.autostart) {
+      QUnit.start();
+    }
+  }
+  if (document.readyState === 'complete') {
+    autostart();
+  } else {
+    window.addEventListener('load', autostart, false);
+  }
+}

src/html-runner/fixture.js → src/browser/fixture.js

@@ -1,11 +1,4 @@
-import QUnit from '../core';
-import { window, document } from '../globals';
-
-(function () {
-  if (!window || !document) {
-    return;
-  }
-
+export default function initFixture (QUnit, document) {
   const config = QUnit.config;
 
   // Stores fixture HTML for resetting later
@@ -50,4 +43,4 @@ import { window, document } from '../globals';
   }
 
   QUnit.testStart(resetFixture);
-})();
+}

src/browser/urlparams.js

@@ -0,0 +1,30 @@
+const hasOwn = Object.prototype.hasOwnProperty;
+
+export default function initUrlConfig (QUnit) {
+  // Wait until QUnit.begin() so that users can add their keys to urlConfig
+  // any time during test loading, including during `QUnit.on('runStart')`.
+  QUnit.begin(function () {
+    const urlConfig = QUnit.config.urlConfig;
+
+    for (let i = 0; i < urlConfig.length; i++) {
+      // Options can be either strings or objects with nonempty "id" properties
+      let option = QUnit.config.urlConfig[i];
+      if (typeof option !== 'string') {
+        option = option.id;
+      }
+
+      // only create new property for user-defined QUnit.config.urlConfig keys
+      // that don't conflict with a built-in QUnit.config option or are otherwise
+      // already set. This prevents internal TypeError from bad urls where keys
+      // could otherwise unexpectedly be set to type string or array.
+      //
+      // Given that HTML Reporter renders checkboxes based on QUnit.config
+      // instead of QUnit.urlParams, this also helps make sure that checkboxes
+      // for built-in keys are correctly shown as off if a urlParams value exists
+      // but was invalid and discarded by config.js.
+      if (!hasOwn.call(QUnit.config, option)) {
+        QUnit.config[option] = QUnit.urlParams[option];
+      }
+    }
+  });
+}

src/qunit.js

@@ -1,5 +1,7 @@
 import QUnit from './core';
-import './html-runner/fixture';
-import './html-runner/urlparams';
+import { initBrowser } from './browser/browser-runner';
+import { window, document } from './globals';
 
-QUnit.reporters.html.init(QUnit);
+if (window && document) {
+  initBrowser(QUnit, window, document);
+}

src/reporters/HtmlReporter.js

@@ -142,107 +142,14 @@ export default class HtmlReporter {
    * @param {Object} [options.config] For internal usage
    */
   static init (QUnit, options) {
+    return new HtmlReporter(QUnit, options);
+  }
+
+  constructor (QUnit, options = {}) {
     // Don't init the HTML Reporter in non-browser environments
     if (!window || !document) {
       return;
     }
-
-    // TODO: Move to caller (browser runner)
-    // Wrap window.onerror. We will call the original window.onerror to see if
-    // the existing handler fully handles the error; if not, we will call the
-    // QUnit.onError function.
-    const originalWindowOnError = window.onerror;
-    // Cover uncaught exceptions
-    // Returning true will suppress the default browser handler,
-    // returning false will let it run.
-    window.onerror = function (message, fileName, lineNumber, columnNumber, errorObj, ...args) {
-      let ret = false;
-      if (originalWindowOnError) {
-        ret = originalWindowOnError.call(
-          this,
-          message,
-          fileName,
-          lineNumber,
-          columnNumber,
-          errorObj,
-          ...args
-        );
-      }
-
-      // Treat return value as window.onerror itself does,
-      // Only do our handling if not suppressed.
-      if (ret !== true) {
-        // If there is a current test that sets the internal `ignoreGlobalErrors` field
-        // (such as during `assert.throws()`), then the error is ignored and native
-        // error reporting is suppressed as well. This is because in browsers, an error
-        // can sometimes end up in `window.onerror` instead of in the local try/catch.
-        // This ignoring of errors does not apply to our general onUncaughtException
-        // method, nor to our `unhandledRejection` handlers, as those are not meant
-        // to receive an "expected" error during `assert.throws()`.
-        if (QUnit.config.current && QUnit.config.current.ignoreGlobalErrors) {
-          return true;
-        }
-
-        // According to
-        // https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror,
-        // most modern browsers support an errorObj argument; use that to
-        // get a full stack trace if it's available.
-        const error = errorObj || new Error(message);
-        if (!error.stack && fileName && lineNumber) {
-          error.stack = `${fileName}:${lineNumber}`;
-        }
-        QUnit.onUncaughtException(error);
-      }
-
-      return ret;
-    };
-
-    // TODO: Move to caller (browser runner)
-    window.addEventListener('unhandledrejection', function (event) {
-      QUnit.onUncaughtException(event.reason);
-    });
-
-    // TODO: Move to caller (browser runner)
-    QUnit.reporters.perf.init(QUnit);
-
-    QUnit.on('runEnd', function (runEnd) {
-      if (QUnit.config.altertitle && document.title) {
-        // Show ✖ for good, ✔ for bad suite result in title
-        // use escape sequences in case file gets loaded with non-utf-8
-        // charset
-        document.title = [
-          (runEnd.status === 'failed' ? '\u2716' : '\u2714'),
-          document.title.replace(/^[\u2714\u2716] /i, '')
-        ].join(' ');
-      }
-
-      // Scroll back to top to show results
-      if (QUnit.config.scrolltop && window.scrollTo) {
-        window.scrollTo(0, 0);
-      }
-    });
-
-    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 (QUnit.config.autostart) {
-        QUnit.start();
-      }
-    }
-
-    const reporter = new HtmlReporter(QUnit, options);
-
-    // TODO: Move to caller (browser runner)
-    if (document.readyState === 'complete') {
-      autostart();
-    } else {
-      DOM.on(window, 'load', autostart);
-    }
-
-    return reporter;
-  }
-
-  constructor (QUnit, options = {}) {
     this.stats = {
       failedTests: [],
       defined: 0,