server: Fix truncation of proxied landing page due to Content-Length

* Strip any Content-Length or Transfer-Encoding response header
  from the origin server for the landing page as it won't be
  correct for the modified version that has the injected script tag.

* Simplify `finish` event to be a summary that is ideal for single
  clients.

Author: Krinkle

API.md

@@ -163,14 +163,21 @@ async function mybrowser (url, signals) {
 
 ### Event: `'finish'`
 
+Summary event that is ideal for when you run one test suite in one browser, or if you otherwise don't need a break down of results by client.
+
 * `event.ok <boolean>` Aggregate status of each client's results. If any failed, this is false.
 * `event.exitCode <number>` Suggested exit code, 0 for success, 1 for failed.
-* `event.bails <Object<string,string>>` For clients that bailed, this contains the bail reason keyed by client ID.
-* `event.results <Object<string,Object>>` For clients completed their test, this contains the detailed `result` event object, keyed by client ID.
+* `event.total <number>` Aggregated from `result` events.
+* `event.passed <number>` Aggregated from `result` events.
+* `event.failed <number>` Aggregated from `result` events.
+* `event.skips <array>` Carried from the first client that failed, or empty.
+* `event.todos <array>` Carried from the first client that failed, or empty.
+* `event.failures <array>` Carried from the first client that failed, or empty.
+* `event.bailout <false|string>` Carried from the first client that failed, or false.
 
 ## QTap reporter events
 
-A client will never emit these events more than once, except for `consoleerror`.
+A client will emit each of these events only once, except `consoleerror` which may be emitted any number of times.
 
 ### Event: `'client'`
 
@@ -209,9 +216,9 @@ The `bail` event is emitted when a browser was unable to start or complete a tes
 
 ### Event: `'consoleerror'`
 
-The `consoleerror` is event for any warnings or errors that may be observed from the browser console. These are for debug purposes only, and do not indicate that any test has failed. A complete and successful test run, may nonetheless print warnings or errors to the console.
+The `consoleerror` event relays any warning or error messages from the browser console. These are for debug purposes only, and do not indicate that a test has failed. A complete and successful test run, may nonetheless print warnings or errors to the console.
 
-It is recommended that reporters only display these if a browser bailed, or if the result includes failed tests.
+It is recommended that reporters only display console errors if a test run failed (i.e. there was a failed test result, or the cilent bailed).
 
 * `event.clientId <string>`
 * `event.message <string>`

src/qtap.js

@@ -180,20 +180,33 @@ function run (files, browserNames = 'detect', runOptions = {}) {
     const finish = {
       ok: true,
       exitCode: 0,
-      bails: {},
-      results: {}
+      total: 0,
+      passed: 0,
+      failed: 0,
+      skips: [],
+      todos: [],
+      failures: [],
+      bailout: false
     };
     eventbus.on('bail', (event) => {
-      finish.ok = false;
-      finish.exitCode = 1;
-      finish.bails[event.clientId] = event;
+      if (finish.ok) {
+        finish.ok = false;
+        finish.exitCode = 1;
+        finish.bailout = event.reason;
+      }
     });
     eventbus.on('result', (event) => {
-      if (!event.ok) {
+      finish.total += event.total;
+      finish.passed += event.passed;
+      finish.failed += event.failed;
+
+      if (finish.ok && !event.ok) {
         finish.ok = false;
         finish.exitCode = 1;
+        finish.skips = event.skips;
+        finish.todos = event.todos;
+        finish.failures = event.failures;
       }
-      finish.results[event.clientId] = event;
     });
 
     // Wait for all tests and browsers to finish/stop, regardless of errors thrown,

src/server.js

@@ -162,7 +162,6 @@ class ControlServer {
       if (!resp.ok) {
         throw new Error('Remote URL responded with HTTP ' + resp.status);
       }
-      // TODO: Write a test to confirm that we preserve response headers
       headers = resp.headers;
       body = await resp.text();
     } else {
@@ -257,7 +256,11 @@ class ControlServer {
 
       const testFileResp = await this.getTestFile(clientId);
       for (const [name, value] of testFileResp.headers) {
-        resp.setHeader(name, value);
+        // Ignore these incompatible headers from the original response,
+        // as otherwise the browser may truncate the amended test file.
+        if (!['content-length', 'transfer-encoding'].includes(name.toLowerCase())) {
+          resp.setHeader(name, value);
+        }
       }
       if (!testFileResp.headers.get('Content-Type')) {
         resp.setHeader('Content-Type', util.MIME_TYPES.html);

test/fixtures/proxied.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<meta charset="utf-8">
+<title>proxied</title>
+<link rel="stylesheet" href="../../node_modules/qunit/qunit/qunit.css">
+<body>
+  <div id="qunit"></div>
+  <div id="qunit-fixture"></div>
+  <script src="../../node_modules/qunit/qunit/qunit.js"></script>
+  <script src="proxied.js"></script>
+</body>
+</html>

test/fixtures/proxied.js

@@ -0,0 +1,10 @@
+/* global QUnit */
+
+QUnit.module('proxied', function () {
+  QUnit.test('example', function (assert) {
+    assert.equal(4, 4);
+  });
+  QUnit.test('Last-Modified header', function (assert) {
+    assert.equal(new Date(document.lastModified).toISOString(), '2011-08-12T06:15:00.000Z');
+  });
+});

test/qtap.test.js

@@ -1,16 +1,18 @@
 import http from 'node:http';
+import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'url';
 import util from 'node:util';
 
 import qtap from '../src/qtap.js';
 import { ControlServer } from '../src/server.js';
+import { MIME_TYPES } from '../src/util.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
-const cwd = path.join(__dirname, '..');
+const root = path.join(__dirname, '..');
 const options = {
-  cwd,
+  cwd: root,
   idleTimeout: 30,
   verbose: !!process.env.CI,
   // verbose: true, // debugging
@@ -36,14 +38,9 @@ function debugReporter (eventbus) {
 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: {},
+  total: 4, passed: 4, failed: 0,
+  skips: [], todos: [], failures: [],
+  bailout: false
 };
 
 QUnit.module('qtap', function (hooks) {
@@ -519,7 +516,6 @@ QUnit.module('qtap', function (hooks) {
     });
     server.listen();
     const port = await new Promise((resolve) => {
-      // @ts-ignore
       server.on('listening', () => resolve(server.address().port));
     });
 
@@ -548,4 +544,67 @@ QUnit.module('qtap', function (hooks) {
 
     server.close();
   });
+
+  // - The test server must remove any Content-Length response header
+  //   if the origin server sends one, as otherwise the browser will
+  //   truncate the response mid-way and time out.
+  // - The test server must inject a <base> tag or otherwise ensure that
+  //   any referenced JS and CSS files are requested from the origin server.
+  QUnit.test('runWaitFor() [proxy resources from custom server]', async function (assert) {
+    assert.timeout(40_000);
+
+    const requestLog = [];
+
+    // Static file server
+    const server = http.createServer((req, resp) => {
+      const ext = path.extname(req.url).slice(1);
+      const filePath = path.join(root, req.url);
+      console.error('# Request ' + req.url + ' for ' + filePath);
+      let fileBuf;
+      try {
+        fileBuf = fs.readFileSync(filePath);
+      } catch (e) {
+        resp.writeHead(404);
+        resp.end();
+        return;
+      }
+      requestLog.push(req.url);
+      resp.writeHead(200, {
+        'Content-Type': MIME_TYPES[ext] || MIME_TYPES.bin,
+        'Content-Length': fileBuf.length,
+        'Last-Modified': 'Fri, 12 Aug 2011 09:15:00 +0300',
+      });
+      resp.write(fileBuf);
+      resp.end();
+    });
+    server.listen();
+    const port = await new Promise((resolve) => {
+      server.on('listening', () => resolve(server.address().port));
+    });
+
+    const finish = await qtap.runWaitFor(
+      `http://localhost:${port}/test/fixtures/proxied.html`,
+      'firefox',
+      options
+    );
+    server.close();
+
+    assert.deepEqual(finish, {
+      ok: true,
+      exitCode: 0,
+      total: 2,
+      passed: 2,
+      failed: [],
+      skips: [],
+      todos: [],
+      failures: [],
+      bailout: false
+    });
+    assert.deepEqual(requestLog, [
+      '/test/fixtures/proxied.html',
+      '/node_modules/qunit/qunit/qunit.css',
+      '/node_modules/qunit/qunit/qunit.js',
+      '/test/fixtures/proxied.js',
+    ], 'requestLog');
+  });
 });

test/structure.test.js

@@ -13,7 +13,9 @@ QUnit.module('structure', function () {
 
     let expectedHtmlFiles = fs.readFileSync(path.join(__dirname, 'qtap.test.js'), 'utf8')
       .split('\n')
-      .map((line) => line.replace(/^\s*files: 'test\/fixtures\/(.+\.html)',$|^.*$|/, '$1'))
+      // match `files: 'test/fixtures/foo.html'`
+      // match `'/test/fixtures/foo.html'`
+      .map((line) => line.replace(/^\s*(?:files: )?'\/?test\/fixtures\/(.+\.html)',$|^.*$|/, '$1'))
       .filter(Boolean)
       .sort();
     // Remove duplicates