Fix bug with files=/full/path, and config=../relative/path

* ControlServer was using path.join() instead of path.resolve().
* run() hardcoded process.cwd() instead of using options.cwd.

While at it:
* refactor options passing to be more streamlined.
* add tests for both of these bugs, which for config means adding
  the first integration test of the config system.
* add tests for browser definitions, by definining a 'fake' browser
  that simply talks to the server and submits fake results without
  actually spawning or doing anything. This is a fast and reliable
  way to test everything except the browser integration, which is
  useful to cover on its own in a test case before the real browsers,
  for faster feedback, and to validate that if a test fails there,
  it is likely specific to one browsers and/or a real browser
  environment, after we've covered 99% of everything else.

bin/qtap.js

@@ -71,10 +71,10 @@ if (opts.version) {
   try {
     const result = await qtap.runWaitFor(opts.browser, program.args, {
       config: opts.config,
-      timeout: opts.timeout,
+      idleTimeout: opts.timeout,
       connectTimeout: opts.connectTimeout,
       reporter: opts.reporter,
-      debug: opts.debug || (process.env.QTAP_DEBUG === '1'),
+      debugMode: opts.debug || (process.env.QTAP_DEBUG === '1'),
       verbose: opts.debug || opts.verbose,
     });
     process.exit(result.exitCode);

eslint.config.js

@@ -30,6 +30,9 @@ export default [
         QUnit: 'readonly'
       }
     },
-    ...qunitRecommended
+    ...qunitRecommended,
+    rules: {
+      'object-property-newline': 'off',
+    }
   }
 ];

package.json

@@ -16,11 +16,11 @@
   "main": "src/qtap.js",
   "scripts": {
     "unit": "qunit test/*.js",
-    "integration-basic": "node bin/qtap.js -r none -v test/fixtures/pass.html",
     "lint": "eslint --cache .",
     "lint-fix": "eslint --cache --fix .",
     "types": "tsc",
-    "test": "npm run -s integration-basic && npm run -s unit && npm run lint && npm run types"
+    "test": "npm run -s unit && npm run lint && npm run types && npm run -s integration-demo",
+    "integration-demo": "node bin/qtap.js -v test/fixtures/pass.html"
   },
   "dependencies": {
     "commander": "12.1.0",

src/qtap.js

@@ -82,9 +82,9 @@ function makeLogger (defaultChannel, printDebug, verbose = false) {
  * @typedef {Object} qtap.RunOptions
  * @property {qtap.Config|string} [config] Config object, or path to a qtap.config.js file.
  * Refer to API.md for how to define additional browsers.
- * @property {number} [timeout=5] How long a browser may be quiet between results.
+ * @property {number} [idleTimeout=5] How long a browser may be quiet between results.
  * @property {number} [connectTimeout=60] How many seconds a browser may take to start up.
- * @property {boolean} [debug=false]
+ * @property {boolean} [debugMode=false]
  * @property {boolean} [verbose=false]
  * @property {string} [reporter="none"]
  * @property {string} [cwd=process.cwd()] Base directory to interpret test file paths
@@ -96,12 +96,19 @@ function makeLogger (defaultChannel, printDebug, verbose = false) {
  * @param {string|string[]} browserNames One or more browser names, referring either
  *  to a built-in browser from QTap, or to a key in the optional `config.browsers` object.
  * @param {string|string[]} files Files and/or URLs.
- * @param {qtap.RunOptions} [options]
+ * @param {qtap.RunOptions} [runOptions]
  * @return {EventEmitter}
  */
-function run (browserNames, files, options = {}) {
+function run (browserNames, files, runOptions = {}) {
   if (typeof browserNames === 'string') browserNames = [browserNames];
   if (typeof files === 'string') files = [files];
+  const options = {
+    cwd: process.cwd(),
+    idleTimeout: 5,
+    connectTimeout: 60,
+    debugMode: false,
+    ...runOptions
+  };
 
   const logger = makeLogger(
     'qtap_main',
@@ -121,17 +128,7 @@ function run (browserNames, files, options = {}) {
 
   const servers = [];
   for (const file of files) {
-    servers.push(new ControlServer(
-      options.cwd || process.cwd(),
-      file,
-      eventbus,
-      logger,
-      {
-        idleTimeout: options.timeout || 5,
-        connectTimeout: options.connectTimeout || 60,
-        debugMode: options.debug ?? false,
-      }
-    ));
+    servers.push(new ControlServer(file, eventbus, logger, options));
   }
 
   const runPromise = (async () => {
@@ -141,7 +138,7 @@ function run (browserNames, files, options = {}) {
     let config;
     if (typeof options.config === 'string') {
       logger.debug('load_config', options.config);
-      config = (await import(path.resolve(process.cwd(), options.config))).default;
+      config = (await import(path.resolve(options.cwd, options.config))).default;
     }
     const globalController = new AbortController();
     const globalSignal = globalController.signal;
@@ -160,19 +157,23 @@ function run (browserNames, files, options = {}) {
       }
     }
 
-    const result = {
+    const finish = {
       ok: true,
-      exitCode: 0
+      exitCode: 0,
+      bails: {},
+      results: {}
     };
-    eventbus.on('bail', () => {
-      result.ok = false;
-      result.exitCode = 1;
+    eventbus.on('bail', (event) => {
+      finish.ok = false;
+      finish.exitCode = 1;
+      finish.bails[event.clientId] = event;
     });
     eventbus.on('result', (event) => {
       if (!event.ok) {
-        result.ok = false;
-        result.exitCode = 1;
+        finish.ok = false;
+        finish.exitCode = 1;
       }
+      finish.results[event.clientId] = event;
     });
 
     try {
@@ -195,7 +196,7 @@ function run (browserNames, files, options = {}) {
       }
     }
 
-    eventbus.emit('finish', result);
+    eventbus.emit('finish', finish);
   })();
   runPromise.catch((error) => {
     // Node.js automatically ensures users cannot forget to listen for the 'error' event.

src/server.js

@@ -17,21 +17,21 @@ class ControlServer {
   static nextClientId = 1;
 
   /**
-   * @param {string} cwd
    * @param {string} testFile File path or URL
    * @param {events.EventEmitter} eventbus
    * @param {Logger} logger
    * @param {Object} options
+   * @param {string} options.cwd
    * @param {number} options.idleTimeout
    * @param {number} options.connectTimeout
    * @param {boolean} options.debugMode
    */
-  constructor (cwd, testFile, eventbus, logger, options) {
+  constructor (testFile, eventbus, logger, options) {
     this.logger = logger.channel('qtap_server_' + ControlServer.nextServerId++);
 
     // For `qtap <url>`, default root to cwd (unused).
     // For `qtap test/index.html`, default root to cwd.
-    let root = cwd;
+    let root = options.cwd;
     let testFileAbsolute;
     if (this.isURL(testFile)) {
       testFileAbsolute = testFile;
@@ -41,7 +41,7 @@ class ControlServer {
       // For `qtap /tmp/foobar/test/index.html`, default root to nearest
       // common parent dir (i.e. longest common path between file and cwd).
       //
-      testFileAbsolute = path.join(root, testFile);
+      testFileAbsolute = path.resolve(root, testFile);
       const relPath = path.relative(root, testFileAbsolute);
       const parent = relPath.match(/^[./\\]+/)?.[0];
       if (parent) {

test/fixtures/fake_pass_4.txt

@@ -0,0 +1,6 @@
+TAP version 13
+ok 1 Foo bar
+ok 2 Baz > this thing
+ok 3 Baz > another thing
+ok 4 Quux
+1..4

test/fixtures/qtap.config.js

@@ -2,13 +2,39 @@ export default {
   browsers: {
     noop_true () {
     },
+
+    /**
+     * @param {string} url
+     */
+    fake: async function fake (url) {
+      fake.displayName = 'FakeBrowser';
+
+      // Fetch page to signal that we're online, and to fetch fake results.
+      // We use the response for real to validate that `files` and `cwd` are
+      // resolved and served correctly.
+      const body = await (await fetch(url)).text();
+      const tapDocument = body.replace(/<script>.*<\/script>/, '');
+
+      // Determine submission endpoint
+      // Leave `qtap_clientId` in urlObj.search unchanged
+      const qtapTapUrl = String(Object.assign(new URL(url), {
+        pathname: '/.qtap/tap/'
+      }));
+
+      // Submit fake results
+      await fetch(qtapTapUrl, {
+        method: 'POST',
+        body: tapDocument
+      });
+    },
+
     async noop_false (url, signals) {
       await new Promise((resolve, reject) => {
         setTimeout(() => {
           reject(new Error('Burp'));
         }, 3000);
         signals.browser.addEventListener('abort', () => {
-          reject(new Error('Boo'));
+          reject(new Error('Bye'));
         });
       });
     }

test/qtap.test.js

@@ -3,36 +3,102 @@ import { fileURLToPath } from 'url';
 import util from 'node:util';
 
 import qtap from '../src/qtap.js';
+import { ControlServer } from '../src/server.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
-const root = path.join(__dirname, '..');
+const cwd = path.join(__dirname, '..');
 const options = {
-  root,
-  timeout: 30,
+  cwd,
+  idleTimeout: 30,
   verbose: !!process.env.CI,
   // verbose: true, // debugging
   printDebug: (str) => { console.error('# ' + str); }
 };
 
 function debugReporter (eventbus) {
-  const steps = [];
-  eventbus.on('client', (event) => steps.push(`client: running ${event.testFile}`));
-  eventbus.on('online', () => steps.push('online'));
-  eventbus.on('bail', (event) => steps.push(`bail: ${event.reason}`));
-  eventbus.on('consoleerror', (event) => steps.push(`consoleerror: ${event.message}`));
+  const events = [];
+  eventbus.on('client', (event) => events.push(`client: running ${event.testFile}`));
+  eventbus.on('online', () => events.push('online'));
+  eventbus.on('bail', (event) => events.push(`bail: ${event.reason}`));
+  eventbus.on('consoleerror', (event) => events.push(`consoleerror: ${event.message}`));
   eventbus.on('result', (event) => {
     delete event.clientId;
     delete event.skips;
     delete event.todos;
     delete event.failures;
-    steps.push(`result: ${util.inspect(event, { colors: false })}`);
+    events.push(`result: ${util.inspect(event, { colors: false })}`);
   });
-  return steps;
+  return events;
 }
 
-QUnit.module('qtap', function () {
-  QUnit.test.each('run', {
+const EXPECTED_FAKE_PASS_4 = {
+  ok: true,
+  exitCode: 0,
+  results: {
+    client_1: {
+      clientId: 'client_1',
+      ok: true, total: 4, passed: 4, failed: 0,
+      skips: [], todos: [], failures: [],
+    }
+  },
+  bails: {},
+};
+
+QUnit.module('qtap', function (hooks) {
+  hooks.beforeEach(() => {
+    ControlServer.nextClientId = 1;
+  });
+
+  QUnit.test.each('runWaitFor()', {
+    basic: {
+      files: 'test/fixtures/fake_pass_4.txt',
+      options: {
+        ...options,
+        config: 'test/fixtures/qtap.config.js'
+      },
+      expected: EXPECTED_FAKE_PASS_4
+    },
+    'options.cwd': {
+      files: 'fixtures/fake_pass_4.txt',
+      options: {
+        ...options,
+        cwd: __dirname,
+        config: 'fixtures/qtap.config.js'
+      },
+      expected: EXPECTED_FAKE_PASS_4
+    },
+    'options.cwd files=../parent/file': {
+      files: '../fake_pass_4.txt',
+      options: {
+        ...options,
+        cwd: path.join(__dirname, 'fixtures/subdir/'),
+        config: '../qtap.config.js'
+      },
+      expected: EXPECTED_FAKE_PASS_4
+    },
+    'options.cwd files=/full/path/file': {
+      files: path.join(__dirname, 'fixtures/fake_pass_4.txt'),
+      options: {
+        ...options,
+        cwd: path.join(__dirname, 'fixtures/subdir/'),
+        config: path.join(__dirname, 'fixtures/qtap.config.js')
+      },
+      expected: EXPECTED_FAKE_PASS_4
+    }
+  }, async function (assert, params) {
+    assert.timeout(40_000);
+
+    const finish = await qtap.runWaitFor(
+      'fake',
+      params.files,
+      params.options
+    );
+
+    assert.deepEqual(finish, params.expected);
+  });
+
+  QUnit.test.each('run() events', {
     pass: {
       files: 'test/fixtures/pass.html',
       options,
@@ -57,7 +123,7 @@ QUnit.module('qtap', function () {
       files: 'test/fixtures/fail-and-timeout.html',
       options: {
         ...options,
-        timeout: 5
+        idleTimeout: 5
       },
       expected: [
         'client: running test/fixtures/fail-and-timeout.html',
@@ -164,7 +230,7 @@ QUnit.module('qtap', function () {
       files: 'test/fixtures/timeout.html',
       options: {
         ...options,
-        timeout: 5
+        idleTimeout: 5
       },
       expected: [
         'client: running test/fixtures/timeout.html',
@@ -241,19 +307,14 @@ QUnit.module('qtap', function () {
   }, async function (assert, params) {
     assert.timeout(40_000);
 
-    const run = qtap.run(
-      'firefox',
-      params.files,
-      params.options
-    );
-
-    const steps = debugReporter(run);
+    const run = qtap.run('firefox', params.files, params.options);
+    const events = debugReporter(run);
     const result = await new Promise((resolve, reject) => {
       run.on('finish', resolve);
       run.on('error', reject);
     });
 
-    assert.deepEqual(steps, params.expected, 'Output');
+    assert.deepEqual(events, params.expected, 'Output');
     assert.deepEqual(result.exitCode, params.exitCode, 'Exit code');
   });
 });