From 4fd224b5d9a26850c2c704625fcf64ab8a742b65 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 22 Nov 2024 21:58:23 -0500 Subject: [PATCH 01/12] test_runner: add level parameter to reporter.diagnostic Added a parameter to allow severity-based formatting for diagnostic messages. Defaults to 'info'. This update enables better control over message presentation (e.g., coloring) based on severity levels such as 'info', 'warn', and 'error'. Refs: https://github.com/nodejs/node/issues/55922 --- lib/internal/test_runner/tests_stream.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/tests_stream.js b/lib/internal/test_runner/tests_stream.js index 2fda1e68069c19..318d7f49998c0e 100644 --- a/lib/internal/test_runner/tests_stream.js +++ b/lib/internal/test_runner/tests_stream.js @@ -116,11 +116,12 @@ class TestsStream extends Readable { }); } - diagnostic(nesting, loc, message) { + diagnostic(nesting, loc, message, level = 'info') { this[kEmitMessage]('test:diagnostic', { __proto__: null, nesting, message, + level, ...loc, }); } From 3c9c4a834676ced04ce64c6ade42017e4633c540 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 22 Nov 2024 22:01:05 -0500 Subject: [PATCH 02/12] test_runner: enhance #handleEvent to handle levels Updated to process the parameter for events. Messages are now formatted with colors based on the (e.g., 'info', 'warn', 'error'). This change ensures diagnostic messages are visually distinct, improving clarity and reducing debugging effort during test runs. Refs: https://github.com/nodejs/node/issues/55922 --- lib/internal/test_runner/reporter/spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index e03c8df9e82489..9031025e57d930 100644 --- a/lib/internal/test_runner/reporter/spec.js +++ b/lib/internal/test_runner/reporter/spec.js @@ -94,8 +94,10 @@ class SpecReporter extends Transform { case 'test:stderr': case 'test:stdout': return data.message; - case 'test:diagnostic': - return `${reporterColorMap[type]}${indent(data.nesting)}${reporterUnicodeSymbolMap[type]}${data.message}${colors.white}\n`; + case 'test:diagnostic':{ + const diagnosticColor = reporterColorMap[data.level] || reporterColorMap['test:diagnostic']; + return `${diagnosticColor}${indent(data.nesting)}${reporterUnicodeSymbolMap[type]}${data.message}${colors.white}\n`; + } case 'test:coverage': return getCoverageReport(indent(data.nesting), data.summary, reporterUnicodeSymbolMap['test:coverage'], colors.blue, true); From 5ef5c9e487dcb7dd603400a151cb78eaf316b0dd Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 22 Nov 2024 22:02:28 -0500 Subject: [PATCH 03/12] test_runner: extend reporterColorMap for levels Enhanced to include colors for the following diagnostic levels: : blue - info : yellow - warn : red - error Refs: https://github.com/nodejs/node/issues/55922 --- lib/internal/test_runner/reporter/utils.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/internal/test_runner/reporter/utils.js b/lib/internal/test_runner/reporter/utils.js index 256619039e8e90..26e641be782316 100644 --- a/lib/internal/test_runner/reporter/utils.js +++ b/lib/internal/test_runner/reporter/utils.js @@ -37,6 +37,18 @@ const reporterColorMap = { get 'test:diagnostic'() { return colors.blue; }, + get 'info'() { + return colors.blue; + }, + get 'debug'() { + return colors.gray; + }, + get 'warn'() { + return colors.yellow; + }, + get 'error'() { + return colors.red; + }, }; function indent(nesting) { From 947130e79c0efff0022c395f11e927b5ff17fee9 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 22 Nov 2024 22:03:30 -0500 Subject: [PATCH 04/12] test_runner: set level for coverage threshold Updated coverage threshold checks in to use the parameter when calling. Errors now use the 'error' level for red-colored formatting. This ensures coverage errors are highlighted effectively in the output. Fixes: https://github.com/nodejs/node/issues/55922 --- lib/internal/test_runner/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index c8390586456db6..5b6a202761ea7a 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1235,7 +1235,7 @@ class Test extends AsyncResource { if (actual < threshold) { harness.success = false; process.exitCode = kGenericUserError; - reporter.diagnostic(nesting, loc, `Error: ${NumberPrototypeToFixed(actual, 2)}% ${name} coverage does not meet threshold of ${threshold}%.`); + reporter.diagnostic(nesting, loc, `Error: ${NumberPrototypeToFixed(actual, 2)}% ${name} coverage does not meet threshold of ${threshold}%.`, 'error'); } } From c515f56c30bedb164ca00d047e75259bc3635b92 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Sat, 23 Nov 2024 19:14:46 -0500 Subject: [PATCH 05/12] test_runner: remove debug from reporterColorMap implemented requested change by removing debug from reporterColorMap Refs: https://github.com/nodejs/node/pull/55964#pullrequestreview-2456483116 --- lib/internal/test_runner/reporter/utils.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/internal/test_runner/reporter/utils.js b/lib/internal/test_runner/reporter/utils.js index 26e641be782316..eb1a008aaf006a 100644 --- a/lib/internal/test_runner/reporter/utils.js +++ b/lib/internal/test_runner/reporter/utils.js @@ -40,9 +40,6 @@ const reporterColorMap = { get 'info'() { return colors.blue; }, - get 'debug'() { - return colors.gray; - }, get 'warn'() { return colors.yellow; }, From e0d74f7a0b5dffe52f699734dfb7ef37738e4327 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Mon, 25 Nov 2024 22:52:36 -0500 Subject: [PATCH 06/12] doc: update 'test:diagnostic' event to include level parameter updated the documentation for the 'test:diagnostic' event to include the new level parameter. clarified its purpose, default value, and possible severity levels ('info', 'warn', 'error'). Fixes: https://github.com/nodejs/node/issues/55922 --- doc/api/test.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/api/test.md b/doc/api/test.md index e1a1f31d16ec9e..e9aa559220a572 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -3004,6 +3004,11 @@ defined. The corresponding declaration ordered event is `'test:start'`. `undefined` if the test was run through the REPL. * `message` {string} The diagnostic message. * `nesting` {number} The nesting level of the test. + * `level` {string} The severity level of the diagnostic message, which determines its output color in the reporter. + Possible values are: + * `'info'`: Informational messages. + * `'warn'`: Warnings. + * `'error'`: Errors. Emitted when [`context.diagnostic`][] is called. This event is guaranteed to be emitted in the same order as the tests are From 0f145bfb30d4699982d7ba03a5a45d7e82a67f55 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Wed, 27 Nov 2024 12:08:23 -0500 Subject: [PATCH 07/12] test: add test for spec reporter red color output Add a test in to verify that the diagnostic error messages about unmet coverage thresholds are displayed in red when using the spec reporter. Fixes: https://github.com/nodejs/node/issues/55922 --- .../test-runner-coverage-thresholds.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js index 61066f80a39cc0..44c4fb830b0df2 100644 --- a/test/parallel/test-runner-coverage-thresholds.js +++ b/test/parallel/test-runner-coverage-thresholds.js @@ -90,6 +90,24 @@ for (const coverage of coverages) { assert(!findCoverageFileForPid(result.pid)); }); + test(`test failing ${coverage.flag} with red color`, () => { + const result = spawnSync(process.execPath, [ + '--test', + '--experimental-test-coverage', + `${coverage.flag}=99`, + '--test-reporter', 'spec', + fixture, + ], { + env: { ...process.env, FORCE_COLOR: '3' }, + }); + + const stdout = result.stdout.toString(); + const redColorRegex = /\u001b\[31mℹ Error: \d{2}\.\d{2}% \w+ coverage does not meet threshold of 99%/; + assert.match(stdout, redColorRegex, 'Expected red color code not found in diagnostic message'); + assert.strictEqual(result.status, 1); + assert(!findCoverageFileForPid(result.pid)); + }); + test(`test failing ${coverage.flag}`, () => { const result = spawnSync(process.execPath, [ '--test', From 263658804ccdca1e5301aea05ef0ebe19be901cf Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Wed, 27 Nov 2024 14:10:41 -0500 Subject: [PATCH 08/12] test: disable no-control-regex for color regex Added eslint-disable comment to bypass no-control-regex. This allows testing ANSI escape sequences for red color in error messages without triggering lint errors. Fixes: https://github.com/nodejs/node/issues/55922 --- test/parallel/test-runner-coverage-thresholds.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js index 44c4fb830b0df2..18d3117fe0970a 100644 --- a/test/parallel/test-runner-coverage-thresholds.js +++ b/test/parallel/test-runner-coverage-thresholds.js @@ -102,6 +102,7 @@ for (const coverage of coverages) { }); const stdout = result.stdout.toString(); + // eslint-disable-next-line no-control-regex const redColorRegex = /\u001b\[31mℹ Error: \d{2}\.\d{2}% \w+ coverage does not meet threshold of 99%/; assert.match(stdout, redColorRegex, 'Expected red color code not found in diagnostic message'); assert.strictEqual(result.status, 1); From a1590bbdf7a2947c6b9189c6f776bfa417513c97 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Thu, 28 Nov 2024 09:51:28 -0500 Subject: [PATCH 09/12] doc: clarify diagnostic behavior in API docs Updated the description of the parameter to note that color output is specific to the spec reporter. This helps users understand its behavior and create custom reporters with accurate expectations. Fixes: https://github.com/nodejs/node/issues/55922 --- doc/api/test.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/test.md b/doc/api/test.md index e9aa559220a572..c48fd5a4531b14 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -3004,7 +3004,7 @@ defined. The corresponding declaration ordered event is `'test:start'`. `undefined` if the test was run through the REPL. * `message` {string} The diagnostic message. * `nesting` {number} The nesting level of the test. - * `level` {string} The severity level of the diagnostic message, which determines its output color in the reporter. + * `level` {string} The severity level of the diagnostic message. Possible values are: * `'info'`: Informational messages. * `'warn'`: Warnings. From 8636dd2ac67eead1728c707a963ae81d80d58791 Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 13 Dec 2024 11:29:35 -0500 Subject: [PATCH 10/12] test: validate diagnostic events in test runner add a test to ensure that diagnostic events emitted by the test runner contain level parameter. Refs: https://github.com/nodejs/node/pull/55964 --- test/parallel/test-runner-run.mjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 2028aa11cc0e36..218826dd06804a 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -33,6 +33,24 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { for await (const _ of stream); }); + it('should emit diagnostic events with level parameter', async () => { + const diagnosticEvents = []; + + const stream = run({ + files: [join(testFixtures, 'coverage.js')], + reporter: 'spec', + }); + + stream.on('test:diagnostic', (event) => { + diagnosticEvents.push(event); + }); + + for await (const _ of stream); + assert(diagnosticEvents.length > 0, 'No diagnostic events were emitted'); + const infoEvent = diagnosticEvents.find((e) => e.level === 'info'); + assert(infoEvent, 'No diagnostic events with level "info" were emitted'); + }); + const argPrintingFile = join(testFixtures, 'print-arguments.js'); it('should allow custom arguments via execArgv', async () => { const result = await run({ files: [argPrintingFile], execArgv: ['-p', '"Printed"'] }).compose(spec).toArray(); From 88188f2375fe7e94ec2ecd3274e88fe5b04f2aae Mon Sep 17 00:00:00 2001 From: Harshil Patel <26harshilpatel11@gmail.com> Date: Fri, 13 Dec 2024 12:55:12 -0500 Subject: [PATCH 11/12] test: Fix Lint error Added eslint-disable-next-line to bypass no-unused-vars check ref: https://github.com/nodejs/node/pull/55964 --- test/parallel/test-runner-run.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 218826dd06804a..06ccf0643eba46 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -44,7 +44,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:diagnostic', (event) => { diagnosticEvents.push(event); }); - + // eslint-disable-next-line no-unused-vars for await (const _ of stream); assert(diagnosticEvents.length > 0, 'No diagnostic events were emitted'); const infoEvent = diagnosticEvents.find((e) => e.level === 'info'); From d009a9d1ad3287dfc56dcaec44cd6b8d7a218d4d Mon Sep 17 00:00:00 2001 From: JacopoPatroclo Date: Fri, 18 Apr 2025 15:31:12 +0200 Subject: [PATCH 12/12] test: fix failing coverage thresholds red line unit test Co-authored-by: Harshil Patel <26harshilpatel11@gmail.com> --- test/parallel/test-runner-coverage-thresholds.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js index 18d3117fe0970a..e45e1191299ca7 100644 --- a/test/parallel/test-runner-coverage-thresholds.js +++ b/test/parallel/test-runner-coverage-thresholds.js @@ -94,6 +94,7 @@ for (const coverage of coverages) { const result = spawnSync(process.execPath, [ '--test', '--experimental-test-coverage', + '--test-coverage-exclude=!test/**', `${coverage.flag}=99`, '--test-reporter', 'spec', fixture,