Remove 'stderr' endpoint in favor of sending stderr as TAP comment

Follows-up f75d8a1. This has the benefit of making it easy to guruantee a consistent order between test results and stderr messages. This is going to be useful in an upcoming change, which is to report any stderr collected "during a test" along with test diagnostic. This is difficult to do when there are likely race conditions, such as when the two streams are separate.
qunitjs · Feb 18, 2025 · 9322536 · 9322536
1 parent fe6f4c2
commit 9322536
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 53 deletions.
diff --git a/src/browsers.js b/src/browsers.js
@@ -224,8 +224,10 @@ export default {
   edge,
   safari,
 
+  // TODO: Create a 'manual' browser that just prints the URL
+
   // TODO: browserstack
   // TODO: saucelabs
   // TODO: puppeteer_coverage { outputDir: instanbul }
-  // TODO: integration test with puppeteer_coverage and nyc console+html output
+  //       Create a test with puppeteer_coverage and nyc console+html output
 };
diff --git a/src/client.js b/src/client.js
@@ -2,18 +2,14 @@
 /* global QUnit */
 // @ts-nocheck
 
-export function fnToStr (fn, qtapTapUrl, qtapStderrUrl) {
+export function fnToStr (fn, qtapTapUrl) {
   return fn
     .toString()
     .replace(/\/\/.+$/gm, '')
     .replace(/\n|^\s+/gm, ' ')
     .replace(
       /'{{QTAP_TAP_URL}}'/g,
       JSON.stringify(qtapTapUrl)
-    )
-    .replace(
-      /'{{QTAP_STDERR_URL}}'/g,
-      JSON.stringify(qtapStderrUrl)
     );
 }
 
@@ -110,7 +106,11 @@ export function qtapClientHead () {
   }
 
   var writeTap = createBufferedWrite('{{QTAP_TAP_URL}}');
-  var writeConsoleError = createBufferedWrite('{{QTAP_STDERR_URL}}');
+
+  function writeConsoleError (str) {
+    var prefix = '# console: ';
+    writeTap(prefix + str.replace(/\n/g, '\n' + prefix));
+  }
 
   console.log = function qtapConsoleLog (str) {
     if (typeof str === 'string') {

diff --git a/src/server.js b/src/server.js
@@ -104,9 +104,6 @@ class ControlServer {
           case '/.qtap/tap/':
             this.handleTap(req, url, resp);
             break;
-          case '/.qtap/stderr/':
-            this.handleStderr(req, url, resp);
-            break;
           default:
             await this.handleRequest(req, url, resp);
         }
@@ -215,6 +212,31 @@ class ControlServer {
       logger.warning('browser_tap_bailout', `Test run bailed, stopping browser. Reason: ${reason}`);
       stopBrowser('browser_tap_bailout', reason);
     });
+
+    tapParser.on('comment', (comment) => {
+      if (!comment.startsWith('# console: ')) {
+        return;
+      }
+
+      // Serve information as transparently as possible
+      // - Strip the prefix we added in /src/client.js
+      // - Strip the proxyBase and qtap_clientId param we added
+      const message = comment
+        .replace('# console: ', '')
+        .replace(/\n$/, '')
+        .replace(/^( {2}at )(http:\S+):(\S+)(?=\n|$)/gm, (m, at, frameUrlStr, lineno) => {
+          const frameUrl = new URL(frameUrlStr);
+          if (frameUrl.origin === this.proxyBase) {
+            return at + frameUrl.pathname + ':' + lineno;
+          }
+          return m;
+        });
+      this.eventbus.emit('consoleerror', {
+        clientId,
+        message
+      });
+    });
+
     // Debugging
     // tapParser.on('line', logger.debug.bind(logger, 'browser_tap_line'));
     // tapParser.on('assert', logger.debug.bind(logger, 'browser_tap_assert'));
@@ -318,9 +340,8 @@ class ControlServer {
   async getTestFile (clientId) {
     const proxyBase = await this.getProxyBase();
     const qtapTapUrl = proxyBase + '/.qtap/tap/?qtap_clientId=' + clientId;
-    const qtapStderrUrl = proxyBase + '/.qtap/stderr/?qtap_clientId=' + clientId;
 
-    let headInjectHtml = `<script>(${fnToStr(qtapClientHead, qtapTapUrl, qtapStderrUrl)})();</script>`;
+    let headInjectHtml = `<script>(${fnToStr(qtapClientHead, qtapTapUrl)})();</script>`;
 
     // and URL-based files can fetch their resources directly from the original server.
     // * Prepend as early as possible. If the file has its own <base>, theirs will
@@ -352,7 +373,7 @@ class ControlServer {
       /<\/html>(?![\s\S]*<\/html>)/i,
       /$/
     ],
-    (m) => '<script>(' + fnToStr(qtapClientBody, qtapTapUrl, qtapStderrUrl) + ')();</script>' + m
+    (m) => '<script>(' + fnToStr(qtapClientBody, qtapTapUrl) + ')();</script>' + m
     );
 
     return html;
@@ -431,37 +452,6 @@ class ControlServer {
     resp.end();
   }
 
-  handleStderr (req, url, resp) {
-    let body = '';
-    req.on('data', (data) => {
-      body += data;
-    });
-    req.on('end', () => {
-      const clientId = url.searchParams.get('qtap_clientId');
-      const browser = this.browsers.get(clientId);
-      if (browser) {
-        browser.logger.warning('browser_stderr_received', clientId, body);
-        // Serve information as transparently as possible
-        // Strip the proxyBase and query params (e.g. qtap_clientId)
-        const message = body.trimEnd().replace(/^( {2}at )(http:\S+):(\S+)(?=\n|$)/gm, (m, at, frameUrlStr, lineno) => {
-          const frameUrl = new URL(frameUrlStr);
-          if (frameUrl.origin === this.proxyBase) {
-            return at + frameUrl.pathname + ':' + lineno;
-          }
-          return m;
-        });
-        this.eventbus.emit('consoleerror', {
-          clientId,
-          message
-        });
-      } else {
-        browser.logger.debug('browser_stderr_unhandled', clientId, body);
-      }
-    });
-    resp.writeHead(204);
-    resp.end();
-  }
-
   /**
    * @param {http.ServerResponse} resp
    * @param {number} statusCode

diff --git a/test/qtap.test.js b/test/qtap.test.js
@@ -138,7 +138,8 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/fail-and-uncaught.html',
         'online',
-        'consoleerror: ReferenceError: bar is not defined\n  at /test/fixtures/fail-and-uncaught.html:14',
+        'consoleerror: ReferenceError: bar is not defined',
+        'consoleerror:   at /test/fixtures/fail-and-uncaught.html:14',
         'bail: End of fixture',
       ],
       exitCode: 1
@@ -159,9 +160,9 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/console.html',
         'online',
-        'consoleerror: My warning 1 {"arr":[true,3]}'
-          + '\nMy error 1 {"arr":[true,3]}'
-          + '\nCyclical object {"a":"example","cycle":"[Circular]"}',
+        'consoleerror: My warning 1 {"arr":[true,3]}',
+        'consoleerror: My error 1 {"arr":[true,3]}',
+        'consoleerror: Cyclical object {"a":"example","cycle":"[Circular]"}',
         'result: { ok: true, total: 1, passed: 1, failed: 0 }',
       ],
       exitCode: 0
@@ -255,7 +256,8 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/uncaught-early.html',
         'online',
-        'consoleerror: ReferenceError: bar is not defined\n  at /test/fixtures/uncaught-early.html:3',
+        'consoleerror: ReferenceError: bar is not defined',
+        'consoleerror:   at /test/fixtures/uncaught-early.html:3',
         'bail: End of fixture',
       ],
       exitCode: 1
@@ -266,7 +268,8 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/uncaught-mid.html',
         'online',
-        'consoleerror: ReferenceError: bar is not defined\n  at /test/fixtures/uncaught-mid.html:5',
+        'consoleerror: ReferenceError: bar is not defined',
+        'consoleerror:   at /test/fixtures/uncaught-mid.html:5',
         'bail: End of fixture',
       ],
       exitCode: 1
@@ -287,7 +290,8 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/uncaught-custom.html',
         'online',
-        'consoleerror: Error: Boo\n  at /test/fixtures/uncaught-custom.html:3',
+        'consoleerror: Error: Boo',
+        'consoleerror:   at /test/fixtures/uncaught-custom.html:3',
         'bail: End of fixture',
       ],
       exitCode: 1
@@ -298,8 +302,10 @@ QUnit.module('qtap', function (hooks) {
       expected: [
         'client: running test/fixtures/uncaught-multiple.html',
         'online',
-        'consoleerror: ReferenceError: bar is not defined\n  at /test/fixtures/uncaught-multiple.html:3'
-          + '\nReferenceError: quux is not defined\n  at /test/fixtures/uncaught-multiple.html:6',
+        'consoleerror: ReferenceError: bar is not defined',
+        'consoleerror:   at /test/fixtures/uncaught-multiple.html:3',
+        'consoleerror: ReferenceError: quux is not defined',
+        'consoleerror:   at /test/fixtures/uncaught-multiple.html:6',
         'bail: End of fixture',
       ],
       exitCode: 1