Fix bin/qtap.js --version requiring a <file> arg

ARCHITECTURE.md

@@ -295,6 +295,12 @@ The clientId is only unique to a single qtap process and thus should not be used
 
 * When creating a temporary directory for the Firefox browser profile, we call our `LocalBrowser.mkTempDir` utility which uses [Node.js `fs.mkdtemp`](https://nodejs.org/docs/latest-v22.x/api/fs.html#fsmkdtempprefix-options-callback), which creates a new directory with a random name that didn't already exist. This is favoured over `os.tmpdir()` with a prefix like `"qtap" + clientId`, as that would conflict with with concurrent invocations, and any remnants of past invokations.
 
+## QTap API: Config file
+
+Inspired by ESLint FlatConfig.
+
+User controls how and what modules to import there. Avoid hard-to-debug YAML or JSON.
+
 ## QTap Internal: Client send
 
 _See also: `function qtapClientHead` in [/src/server.js](./src/server.js)_.

bin/qtap.js

@@ -12,7 +12,7 @@ program
   .description('Run unit tests in real browsers', {
     file: 'One or more local HTML files or URLs'
   })
-  .argument('<file...>')
+  .argument('[file...]')
   .option('-b, --browser <name>',
     'One or more browser names.\n'
        + 'Available: firefox, chrome, safari.',
@@ -27,16 +27,15 @@ program
   )
   .option('-c, --config <file>', 'Optional config file to define additional browsers.')
   .option('--timeout <number>',
-    'Fail if a browser is quiet for this long '
-      + 'before or between results,\nin millseconds.',
+    'Fail if a browser is quiet for more than this many seconds.',
     function (val) {
       val = Number(val);
       if (val < 0 || !Number.isFinite(val)) {
         throw new InvalidArgumentError('Not a number.');
       }
       return val;
     },
-    3000
+    3
   )
   .option('-w, --watch', 'Watch files for changes and re-run the test suite.')
   .option('-v, --verbose', 'Enable verbose debug logging.')

src/browsers.js

@@ -105,17 +105,17 @@ function * getEdgePaths () {
   }
 }
 
+// http://kb.mozillazine.org/About:config_entries
+// https://github.com/sitespeedio/browsertime/blob/v23.5.0/lib/firefox/settings/firefoxPreferences.js
+// https://github.com/airtap/the-last-browser-launcher/blob/v0.1.1/lib/launch/index.js
+// https://github.com/karma-runner/karma-firefox-launcher/blob/v2.1.3/index.js
 async function firefox (url, signal, logger) {
   const profileDir = LocalBrowser.makeTempDir();
   const args = [url, '-profile', profileDir, '-no-remote', '-wait-for-browser'];
   if (!QTAP_DEBUG) {
     args.push('-headless');
   }
 
-  // http://kb.mozillazine.org/About:config_entries
-  // https://github.com/sitespeedio/browsertime/blob/v23.5.0/lib/firefox/settings/firefoxPreferences.js
-  // https://github.com/airtap/the-last-browser-launcher/blob/v0.1.1/lib/launch/index.js
-  // https://github.com/karma-runner/karma-firefox-launcher/blob/v2.1.3/index.js
   logger.debug('firefox_prefs_create', 'Creating temporary prefs.js file');
   fs.writeFileSync(path.join(profileDir, 'prefs.js'), createFirefoxPrefsJs({
     'app.update.disabledForTesting': true, // Disable auto-updates
@@ -145,9 +145,9 @@ async function firefox (url, signal, logger) {
   await LocalBrowser.spawn(getFirefoxPaths(), args, signal, logger);
 }
 
+// https://github.com/GoogleChrome/chrome-launcher/blob/main/docs/chrome-flags-for-tools.md
 async function chromium (paths, url, signal, logger) {
   const dataDir = LocalBrowser.makeTempDir();
-  // https://github.com/GoogleChrome/chrome-launcher/blob/main/docs/chrome-flags-for-tools.md
   const args = [
     '--user-data-dir=' + dataDir,
     '--no-default-browser-check',

src/qtap.js

@@ -30,12 +30,17 @@ function makeLogger (defaultChannel, printError, printDebug = null) {
 }
 
 /**
- * @param {string[]} browsers One or more local browser names,
- *  or path starting with "./" to a JSON file.
- * @param {string} files Files and/or URLs.
+ * @param {string[]} browserNames One or more browser names, referring either
+ *  to a built-in browser launcher from QTap, or to a key in the optional
+ *  `config.browsers` object.
+ * @param {string[]} files Files and/or URLs.
  * @param {Object} [options]
- * @param {string} [options.config] Path to JS file that exports additional browsers.
- *  User controls how and what modules to import there. Inspired by ESLint FlatConfig.
+ * @param {string} [options.config] Path to JS file that defines additional browsers.
+ * @param {Object<string,Function>} [options.config.browsers] Refer to API.md for
+ *  how to define a browser launch function.
+ * @param {number} [options.timeout=3] Fail if a browser is idle for this many seconds.
+ * @param {number} [options.connectTimeout=60] How long a browser may initially take
+   to launch and open the URL, in seconds.
  * @param {boolean} [options.verbose=false]
  * @param {Function} [options.printInfo=console.log]
  * @param {Function} [options.printError=console.error]
@@ -52,7 +57,10 @@ async function run (browserNames, files, options) {
 
   const servers = [];
   for (const file of files) {
-    servers.push(new ControlServer(options.root, file, logger));
+    servers.push(new ControlServer(options.root, file, logger, {
+      idleTimeout: options.timeout,
+      connectTimeout: options.connectTimeout
+    }));
   }
 
   let config;

src/server.js

@@ -16,7 +16,15 @@ class ControlServer {
   static nextServerId = 1;
   static nextClientId = 1;
 
-  constructor (root, testFile, logger) {
+  /**
+   * @param {Mixed} root
+   * @param {Mixed} testFile
+   * @param {Mixed} logger
+   * @param {Object} options
+   * @param {number|undefined} options.idleTimeout
+   * @param {number|undefined} options.connectTimeout
+   */
+  constructor (root, testFile, logger, options) {
     if (!root) {
       // For `qtap test/index.html`, default root to cwd.
       root = process.cwd();
@@ -30,6 +38,8 @@ class ControlServer {
 
     this.root = root;
     this.testFile = testFile;
+    this.idleTimeout = options.idleTimeout || 3;
+    this.connectTimeout = options.connectTimeout || 60;
     this.browsers = new Map();
     this.logger = logger.channel('qtap_server_' + this.constructor.nextServerId++);
     // Optimization: Prefetch test file in parallel with server starting and browser launching.
@@ -99,10 +109,11 @@ class ControlServer {
     const controller = new AbortController();
     const summary = { ok: true };
 
-    // TODO: Separate initial connection timeout from idle timeout
-    const CLIENT_IDLE_TIMEOUT = 60_000;
-    const CLIENT_IDLE_INTERVAL = 1000;
     let clientIdleTimer = null;
+    // TODO: Actually implement CONNECT_TIMEOUT
+    // const CONNECT_TIMEOUT = this.connectTimeout;
+    const IDLE_TIMEOUT = this.idleTimeout;
+    const TIMEOUT_CHECK_INTERVAL_MS = 1000;
 
     let readableController;
     const readable = new ReadableStream({
@@ -158,34 +169,21 @@ class ControlServer {
     // tapParser.on('assert', logger.debug.bind(logger, 'browser_tap_assert'));
     // tapParser.on('plan', logger.debug.bind(logger, 'browser_tap_plan'));
 
-    // TODO: Make timeout configurable, e.g. --timeout=3000s
-    //
-    // TODO: Consider handling timeout client-side by setting options.timeout to
-    // qunit_config_testtimeout in getTestFile() and send a "Bail out!" tap
-    // message which makes the server stop the browser.
-    // Pro - timeouts are in sync. Con - we still need a "real" timeout
-    // on this side.
-    //
-    // TODO: Dynamically increase this to whatever timeout the client's test framework
-    // has set (QUnit, Mocha, Jasmine), with one second tolerance added for delay
-    // (network/interprocess) between client and QTap.
-    // We could probably read out smth like QUnit.config.testTimeout
-    // and collect it via an endpoint like /.qtap/set_timeout?clientId=
-    //
-    // NOTE: We use performance.now() instead of natively clearTimeout+setTimeout
-    // on each event, because that would add significant overhead from Node.js/V8
-    // creating tons of timers when processing a large batch of test results back-to-back.
-    clientIdleTimer = setTimeout(function qtapClientTimeout () {
-      if ((performance.now() - browser.clientIdleActive) > CLIENT_IDLE_TIMEOUT) {
-        logger.warning('browser_idle_timeout', `Browser timed out after ${CLIENT_IDLE_TIMEOUT}ms, stopping browser`);
+    // Optimization: The naive approach would be to clearTimeout+setTimeout on every tap line, in
+    // readableController or `tapParser.on('line')`. That would add significant overhead from
+    // Node.js/V8 natively allocating many timers when processing large batches of test results.
+    // Instead, merely store performance.now() and check that periodically.
+    clientIdleTimer = setTimeout(function qtapCheckTimeout () {
+      if ((performance.now() - browser.clientIdleActive) > (IDLE_TIMEOUT * 1000)) {
+        logger.warning('browser_idle_timeout', `Browser timed out after ${IDLE_TIMEOUT}s, stopping browser`);
         // TODO:
         // Produce a tap line to report this test failure to CLI output/reporters.
         summary.ok = false;
         stopBrowser('QTap: browser_idle_timeout');
       } else {
-        clientIdleTimer = setTimeout(qtapClientTimeout, CLIENT_IDLE_INTERVAL);
+        clientIdleTimer = setTimeout(qtapCheckTimeout, TIMEOUT_CHECK_INTERVAL_MS);
       }
-    }, CLIENT_IDLE_INTERVAL);
+    }, TIMEOUT_CHECK_INTERVAL_MS);
 
     const [readeableForParser, readableForFinished] = readable.tee();
     readeableForParser.pipeTo(stream.Writable.toWeb(tapParser));