Improve 'consoleerror' handler to include all arguments, Handle remot…

…e HTTP error Based on decycledShallowClone() from QUnit's TapReporter, which was originally implemented by Zachary Mulgrew in qunitjs/js-reporters@c38f467af9. Co-authored-by: Zachary Mulgrew <[email protected]>
qunitjs · Jan 24, 2025 · 1b8801a · 1b8801a
1 parent 88877a5
commit 1b8801a
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 15 deletions.
diff --git a/src/client.js b/src/client.js
@@ -32,6 +32,54 @@ export function qtapClientHead () {
   var log = console.log && console.log.apply ? console.log : function () {};
   var warn = console.warn && console.warn.apply ? console.warn : function () {};
   var error = console.error && console.error.apply ? console.error : function () {};
+  var jsonStringify = JSON.stringify.bind(JSON);
+  var toString = Object.prototype.toString;
+  var hasOwn = Object.prototype.hasOwnProperty;
+
+  /**
+   * Create a shallow clone of an object, with cycles replaced by "[Circular]".
+   */
+  function decycledShallowClone (object, ancestors) {
+    ancestors = ancestors || [];
+    if (ancestors.indexOf(object) !== -1) {
+      return '[Circular]';
+    }
+    if (ancestors.length > 100) {
+      return '...';
+    }
+    var type = toString.call(object).replace(/^\[.+\s(.+?)]$/, '$1').toLowerCase();
+    var clone;
+    switch (type) {
+      case 'array':
+        ancestors.push(object);
+        clone = [];
+        for (var i = 0; i < object.length; i++) {
+          clone[i] = decycledShallowClone(object[i], ancestors);
+        }
+        ancestors.pop();
+        break;
+      case 'object':
+        ancestors.push(object);
+        clone = {};
+        for (var key in object) {
+          if (hasOwn.call(object, key)) {
+            clone[key] = decycledShallowClone(object[key], ancestors);
+          }
+        }
+        ancestors.pop();
+        break;
+      default:
+        clone = object;
+    }
+    return clone;
+  }
+
+  function stringify (data) {
+    if (typeof data !== 'object') {
+      return '' + data;
+    }
+    return jsonStringify(decycledShallowClone(data));
+  }
 
   function createBufferedWrite (url) {
     var buffer = '';
@@ -71,13 +119,21 @@ export function qtapClientHead () {
     return log.apply(console, arguments);
   };
 
-  console.warn = function qtapConsoleWarn (str) {
-    writeConsoleError('' + str);
+  console.warn = function qtapConsoleWarn () {
+    var str = [];
+    for (var i = 0; i < arguments.length; i++) {
+      str[i] = stringify(arguments[i]);
+    }
+    writeConsoleError(str.join(' '));
     return warn.apply(console, arguments);
   };
 
-  console.error = function qtapConsoleError (str) {
-    writeConsoleError('' + str);
+  console.error = function qtapConsoleError () {
+    var str = [];
+    for (var i = 0; i < arguments.length; i++) {
+      str[i] = stringify(arguments[i]);
+    }
+    writeConsoleError(str.join(' '));
     return error.apply(console, arguments);
   };
 

diff --git a/src/reporters.js b/src/reporters.js
@@ -89,9 +89,8 @@ function plain (eventbus) {
     console.log(util.styleText('grey', `[${event.clientId}]`) + ` Error! ${event.reason}`);
   });
   eventbus.on('result', (event) => {
-    console.log(util.styleText('grey', `[${event.clientId}]`) + ' Finished!');
-    // TODO: Tests completed, X passed [, X failed ] [, X skipped ].
     // TODO: Report wall-clock runtime
+    console.log(util.styleText('grey', `[${event.clientId}]`) + ` Finished! Ran ${event.total} tests, ${event.failed} failed.`);
 
     if (event.skips.length) {
       const minimalResults = event.skips.map((result) => {

diff --git a/src/server.js b/src/server.js
@@ -137,7 +137,11 @@ class ControlServer {
     // fetch() does not yet support file URLs (as of Node.js 21).
     if (this.isURL(file)) {
       this.logger.debug('testfile_fetch', `Requesting a copy of ${file}`);
-      return await (await fetch(file)).text();
+      const resp = await fetch(file);
+      if (!resp.ok) {
+        throw new Error('Remote URL responded with HTTP ' + resp.status);
+      }
+      return await resp.text();
     } else {
       this.logger.debug('testfile_read', `Reading file contents from ${file}`);
       return (await fsPromises.readFile(file)).toString();
@@ -367,16 +371,20 @@ class ControlServer {
     if (clientId !== null) {
       // Serve the testfile from any URL path, as chosen by launchBrowser()
       const browser = this.browsers.get(clientId);
-      if (browser) {
-        browser.clientIdleActive = performance.now();
-        browser.logger.debug('browser_connected', `${browser.getDisplayName()} connected! Serving test file.`);
-        this.eventbus.emit('online', { clientId });
-      } else {
-        this.logger.debug('respond_static_testfile', clientId);
+      if (!browser) {
+        this.logger.debug('browser_connected_unknown', clientId);
+        return this.serveError(resp, 403, 'Forbidden');
       }
+
+      browser.logger.debug('browser_connected', `${browser.getDisplayName()} connected! Serving test file.`);
+      this.eventbus.emit('online', { clientId });
+
       resp.writeHead(200, { 'Content-Type': util.MIME_TYPES[ext] || util.MIME_TYPES.html });
       resp.write(await this.getTestFile(clientId));
       resp.end();
+
+      // Count proxying the test file toward connectTimeout, not idleTimeout.
+      browser.clientIdleActive = performance.now();
       return;
     }
 
@@ -414,8 +422,7 @@ class ControlServer {
           bodyExcerpt
         );
         browser.tapParser.write(body);
-
-        browser.clientIdleActive = performance.now();
+        browser.clientIdleActive = now;
       } else {
         this.logger.debug('browser_tap_unhandled', clientId, bodyExcerpt);
       }

diff --git a/test/fixtures/console.html b/test/fixtures/console.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<title>console</title>
+<script>
+console.log('TAP version 13');
+
+console.warn('My warning', 1, { arr: [ true, 3 ] });
+console.error('My error', 1, { arr: [ true, 3 ] });
+
+function createCyclical () {
+  var cyclical = { a: 'example' };
+  cyclical.cycle = cyclical;
+  return cyclical;
+}
+console.warn('Cyclical object', createCyclical());
+
+setTimeout(function () {
+  console.log('ok 1 Foo bar');
+  console.log('1..1');
+}, 50);
+</script>
diff --git a/test/fixtures/qunit-notests.html b/test/fixtures/qunit-notests.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<meta charset="utf-8">
+<title>QUnit</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>
+</body>
+</html>
diff --git a/test/qtap.test.js b/test/qtap.test.js
@@ -84,6 +84,19 @@ QUnit.module('qtap', function () {
       ],
       exitCode: 1
     },
+    console: {
+      files: 'test/fixtures/console.html',
+      options,
+      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]"}',
+        'result: { ok: true, total: 1, passed: 1, failed: 0 }',
+      ],
+      exitCode: 0
+    },
     mocking: {
       files: 'test/fixtures/mocking.html',
       options,
@@ -107,6 +120,19 @@ QUnit.module('qtap', function () {
       ],
       exitCode: 0
     },
+    qunitNotests: {
+      files: 'test/fixtures/qunit-notests.html',
+      options: {
+        ...options,
+        timeout: 30,
+      },
+      expected: [
+        'client: running test/fixtures/qunit-notests.html',
+        'online',
+        'result: { ok: false, total: 1, passed: 0, failed: 1 }',
+      ],
+      exitCode: 1
+    },
     qunitFail: {
       files: 'test/fixtures/qunit-fail.html',
       options: {