CLI: Apply trace cleaning and internal greying to assertion stack

Follows-up #1789, which applied this to traces under uncaught errors. We now apply this clean up to traces under assertion failures as well. Closes #1795.
qunitjs · Aug 9, 2024 · 276e09d · 276e09d
1 parent a952967
commit 276e09d
Show file tree

Hide file tree

Showing 12 changed files with 17 additions and 28 deletions.
diff --git a/src/core/reporters/TapReporter.js b/src/core/reporters/TapReporter.js
@@ -269,7 +269,10 @@ export default class TapReporter {
     if (error.stack) {
       // Since stacks aren't user generated, take a bit of liberty by
       // adding a trailing new line to allow a straight-forward YAML Blocks.
-      out += `\n  stack: ${prettyYamlValue(error.stack + '\n')}`;
+      const fmtStack = annotateStacktrace(error.stack, kleur.grey);
+      if (fmtStack.length) {
+        out += `\n  stack: ${prettyYamlValue(fmtStack + '\n')}`;
+      }
     }
 
     out += '\n  ...';
@@ -281,7 +284,10 @@ export default class TapReporter {
     out += `\n  message: ${prettyYamlValue(errorString(error))}`;
     out += `\n  severity: ${prettyYamlValue('failed')}`;
     if (error && error.stack) {
-      out += `\n  stack: ${prettyYamlValue(annotateStacktrace(error, kleur.grey) + '\n')}`;
+      const fmtStack = annotateStacktrace(error.stack, kleur.grey, error.toString());
+      if (fmtStack.length) {
+        out += `\n  stack: ${prettyYamlValue(fmtStack + '\n')}`;
+      }
     }
     out += '\n  ...';
     this.log(out);

diff --git a/src/core/stacktrace.js b/src/core/stacktrace.js
@@ -60,17 +60,20 @@ function qunitFileName () {
 const fileName = qunitFileName();
 
 /**
+ * Responsibilities:
  * - For internal errors from QUnit itself, remove the first qunit.js frames.
  * - For errors in Node.js, format any remaining qunit.js and node:internal
  *   frames as internal (i.e. grey out).
+ *
+ * @param {string} Error#stack
+ * @param {Function} formatInternal Format a string in an "internal" color
+ * @param {string|null} [eToString] Error#toString() to help remove
+ *  noise from Node.js/V8 stack traces.
  */
-export function annotateStacktrace (e, formatInternal) {
-  if (!e || !e.stack) {
-    return String(e);
-  }
-  const frames = e.stack.split('\n');
+export function annotateStacktrace (stack, formatInternal, eToString = null) {
+  const frames = stack.split('\n');
   const annotated = [];
-  if (e.toString().indexOf(frames[0]) !== -1) {
+  if (eToString && eToString.indexOf(frames[0]) !== -1) {
     // In Firefox and Safari e.stack starts with frame 0, but in V8 (Chrome/Node.js),
     // e.stack starts first stringified message. Preserve this separately,
     // so that, below, we can distinguish between internal frames on top

diff --git a/test/cli/cli-main.js b/test/cli/cli-main.js
@@ -39,8 +39,6 @@ not ok 2 slow
   ---
   message: Test took longer than 7ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 ok 3 config
 1..3

diff --git a/test/cli/fixtures/config-noglobals-add.tap.txt b/test/cli/fixtures/config-noglobals-add.tap.txt
@@ -6,8 +6,6 @@ not ok 1 adds global var
   ---
   message: Introduced global variable(s): dummyGlobal
   severity: failed
-  stack: |
-        at qunit.js
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/config-noglobals-remove.tap.txt b/test/cli/fixtures/config-noglobals-remove.tap.txt
@@ -6,8 +6,6 @@ not ok 1 deletes global var
   ---
   message: Deleted global variable(s): dummyGlobal
   severity: failed
-  stack: |
-        at qunit.js
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/config-notrycatch-hook-rejection.tap.txt b/test/cli/fixtures/config-notrycatch-hook-rejection.tap.txt
@@ -12,8 +12,6 @@ not ok 1 example > passing test
   ---
   message: Test took longer than 1000ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/config-notrycatch-test-rejection.tap.txt b/test/cli/fixtures/config-notrycatch-test-rejection.tap.txt
@@ -12,8 +12,6 @@ not ok 1 example > returns a rejected promise
   ---
   message: Test took longer than 1000ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/config-testTimeout.tap.txt b/test/cli/fixtures/config-testTimeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 slow
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/done-after-timeout.tap.txt b/test/cli/fixtures/done-after-timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 times out before scheduled done is called
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/hanging-test.tap.txt b/test/cli/fixtures/hanging-test.tap.txt
@@ -6,8 +6,6 @@ not ok 1 hanging
   ---
   message: Test took longer than 3000ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/pending-async-after-timeout.tap.txt b/test/cli/fixtures/pending-async-after-timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 example
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0

diff --git a/test/cli/fixtures/timeout.tap.txt b/test/cli/fixtures/timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 timeout > first
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 ok 2 timeout > second
 1..2