From b72930b405ab8e2e8df8f05a1c4bb1d70fb46bd6 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Wed, 7 Aug 2024 23:57:53 +0100 Subject: [PATCH] CLI: Apply trace cleaning and internal greying to assertion stack Follows-up https://github.com/qunitjs/qunit/pull/1789, which applied this to traces under uncaught errors. We now apply this clean up to traces under assertion failures as well. --- src/core/reporters/TapReporter.js | 10 ++++++++-- src/core/stacktrace.js | 15 +++++++++------ test/cli/cli-main.js | 2 -- test/cli/fixtures/config-noglobals-add.tap.txt | 2 -- test/cli/fixtures/config-noglobals-remove.tap.txt | 2 -- .../config-notrycatch-hook-rejection.tap.txt | 2 -- .../config-notrycatch-test-rejection.tap.txt | 2 -- test/cli/fixtures/config-testTimeout.tap.txt | 2 -- test/cli/fixtures/done-after-timeout.tap.txt | 2 -- test/cli/fixtures/hanging-test.tap.txt | 2 -- .../fixtures/pending-async-after-timeout.tap.txt | 2 -- test/cli/fixtures/timeout.tap.txt | 2 -- 12 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/core/reporters/TapReporter.js b/src/core/reporters/TapReporter.js index b102af584..579a6a5e3 100644 --- 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 index 67ce75218..4dfac41dd 100644 --- 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 index 8b5e538b7..07a7dc844 100644 --- 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 index 51f74e85e..df11ccc66 100644 --- 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 index 57d853388..9bbd52354 100644 --- 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 index 289194d47..3ccfefcd5 100644 --- 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 index 30d661445..2a372f911 100644 --- 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 index 5a1f25372..6ff7de815 100644 --- 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 index 54e72b97d..9bfdcccbf 100644 --- 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 index 347016660..a6d30784d 100644 --- 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 index a8641fa57..7fbc31ebb 100644 --- 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 index b29da673f..3a4e8233e 100644 --- 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