From 6fc27b8ee820730f7c81fc03efcdbe1886562ac9 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinkle@fastmail.com>
Date: Fri, 19 Jul 2024 03:13:56 +0100
Subject: [PATCH] Include output from `QUnit.on('error')`

The two are not likely to overlap, but this could in theory report
the same error twice. In practice, given the bridge kicks in fairly
late, uncaught errors will happen before our QUnit listener is
active, and late errors are likely exclusive to `QUnit.on('error')`.

The "No tests" message is changing in QUnit 3.0 from a "fake" test,
to an error event, hence this is in preparation for that.
---
 Gruntfile.js                 | 16 ++++++++++++++++
 chrome/bridge.js             |  4 ++++
 tasks/qunit.js               | 21 +++++++++++++++++++--
 test/qunit_fail_notests.html | 13 +++++++++++++
 4 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 test/qunit_fail_notests.html

diff --git a/Gruntfile.js b/Gruntfile.js
index 0918c21..c972735 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -99,6 +99,12 @@ module.exports = function(grunt) {
 >> Expected: true`)) {
             cb(err !== null);
 
+          // qunit:failNoTests
+          } else if (/test\/qunit_fail_notests\.html/.test(stdout) &&
+            stdout.includes(`>> global failure
+>> Message: No tests were run.`)) {
+            cb(err !== null);
+
           // qunit:failPageTimeout
           } else if (/test\/qunit_page_timeout\.html/.test(stdout) &&
               /Chrome timed out/.test(stdout)) {
@@ -134,6 +140,9 @@ module.exports = function(grunt) {
       failAssert: {
         command: 'grunt qunit:failAssert --with-failing'
       },
+      failNoTests: {
+        command: 'grunt qunit:failNoTests --with-failing'
+      },
       failCircularObject: {
         command: 'grunt qunit:failCircularObject --with-failing'
       },
@@ -156,6 +165,13 @@ module.exports = function(grunt) {
         ]
       }
     });
+    grunt.config.set('qunit.failNoTests', {
+      options: {
+        urls: [
+          'http://localhost:9000/test/qunit_fail_notests.html'
+        ]
+      }
+    });
     grunt.config.set('qunit.failCircularObject', {
       options: {
         urls: [
diff --git a/chrome/bridge.js b/chrome/bridge.js
index 7621d40..9f78336 100644
--- a/chrome/bridge.js
+++ b/chrome/bridge.js
@@ -40,6 +40,10 @@
   // QUnit reporter events
   // https://qunitjs.com/api/callbacks/QUnit.on/
 
+  QUnit.on('error', function(error) {
+    sendMessage('qunit.on.error', error);
+  });
+
   QUnit.on('testStart', function(obj) {
     sendMessage('qunit.on.testStart', obj);
   });
diff --git a/tasks/qunit.js b/tasks/qunit.js
index 9fe6491..ddc5dbf 100644
--- a/tasks/qunit.js
+++ b/tasks/qunit.js
@@ -231,13 +231,30 @@ module.exports = function(grunt) {
     combinedRunEnd.status = 'failed';
   });
 
-  eventBus.on('error.onError', function (msg) {
+  eventBus.on('qunit.on.error', function (err) {
     // It is the responsibility of QUnit to ensure a run is marked as failure
     // if there are (unexpected) messages received from window.onerror.
     //
     // Prior to QUnit 2.17, details of global failures were printed by
     // creating a fake test with "testEnd" event. Now, it is our responsiblity
-    // to print these, via browser-level pageerror or `QUnit.on('error')`.
+    // to print via `QUnit.on('error')`.
+    //
+    // NOTE: Avoid relying solely on Puppeteer's "pageerror" event, as that only
+    // catches the subset of execution errors that make their way to window.onerror.
+    // It misses out on unhandled rejections from the browser, the "No tests were run"
+    // internal QUnit error, and anything else reported to QUnit.onUncaughtException
+    // by QUnit plugins.
+    // https://qunitjs.com/api/extension/QUnit.onUncaughtException/
+    grunt.log.writeln();
+    grunt.log.error(err.stack || err);
+    grunt.event.emit('qunit.error.onError', err);
+  });
+
+  eventBus.on('error.onError', function (msg) {
+    // This is important in addition to `QUnit.on('error')` to catch uncaught
+    // errors that happen before the bridge is in effect (which in practice
+    // will happen at DOMContentLoaded, after qunit.js and test files have done
+    // their initial execution, but before QUnit.begin event and any actual tests).
     grunt.log.writeln();
     grunt.log.error(msg.stack || msg);
     grunt.event.emit('qunit.error.onError', msg);
diff --git a/test/qunit_fail_notests.html b/test/qunit_fail_notests.html
new file mode 100644
index 0000000..87e80e1
--- /dev/null
+++ b/test/qunit_fail_notests.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test Suite</title>
+  <link rel="stylesheet" href="../node_modules/qunit/qunit/qunit.css" media="screen">
+  <script src="../node_modules/qunit/qunit/qunit.js"></script>
+</head>
+<body>
+  <div id="qunit"></div>
+  <!-- No tests -->
+</body>
+</html>