Repository navigation
Add a level parameter to test runner diagnostics #55922
Description
Activity
Hi @redyetidev @MoLow, I would love to contribute to this.
Feel free to open a PR
Reacted by Harshil Patel and Jurj Andrei GeorgeHi @redyetidev, Here is the Summary of Planned changes, please take a look and give feedback
Summary of Planned Changes
To address the problem of coverage threshold errors not being visually distinct, I propose the following enhancements:
-
Introduce a
levelParameter in Diagnostics:- Add a
levelparameter (e.g.,debug,info,warn,error) to diagnostic messages. - This parameter will categorize the severity of the message and allow reporters to handle formatting based on the message type.
- Add a
-
Update Diagnostic Emission:
- Modify the diagnostic function to include the
levelparameter along with the message and location. - Example:
reporter.diagnostic(nesting, loc, { message: `Error: ${actual}% ${name} coverage does not meet threshold of ${threshold}%.`, level: 'error' });
- Modify the diagnostic function to include the
-
Delegate Formatting to Reporters:
- Ensure reporters use the
levelparameter to apply appropriate formatting (e.g., red for errors, yellow for warnings). - This will keep presentation logic within reporters, maintaining the separation of concerns.
- Ensure reporters use the
Reacted by Pietro Marchini-
Hey @hpatel292-seneca, thanks for contributing! 😊
Regarding the planned changes: IMHO, it looks okay🚀 I think you could start working on the PR!Only one thing: atm, the message is a string parameter.
Changing it from a string to an object could require more work and attention, potentially causing breaking changes
I would suggest considering the idea of simply adding a new parameter to the function instead.Just a trivial reminder: please ensure tests are added to cover this feature.
Reacted by Harshil PatelUnderstood. Thanks @pmarchini
Reacted by Pietro MarchiniHi @pmarchini,
I just wanted to confirm one thing,
so I updated the reporter.diagnostic to accept level parameter like thisdiagnostic(nesting, loc, message, level = 'info') { this[kEmitMessage]('test:diagnostic', { __proto__: null, nesting, message, level, ...loc, }); }
Then I updated
#handleEventlike this#handleEvent({ type, data }) { switch (type) { case 'test:fail': if (data.details?.error?.failureType !== kSubtestsFailed) { ArrayPrototypePush(this.#failedTests, data); } return this.#handleTestReportEvent(type, data); case 'test:pass': return this.#handleTestReportEvent(type, data); case 'test:start': ArrayPrototypeUnshift(this.#stack, { __proto__: null, data, type }); break; case 'test:stderr': case 'test:stdout': return data.message; case 'test:diagnostic': // Here I added logic 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 ); } }
And I am Updated
reporterColorMaplike thisconst reporterColorMap = { __proto__: null, get 'test:fail'() { return colors.red; }, get 'test:pass'() { return colors.green; }, 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; }, };
and color already contain logic for this colors
so my question is I will set the reporter.diagnostic call from test.js like this (level="Error")
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}%.`, 'error' // Level is set to error for red color ); }
right?
Hey @hpatel292-seneca, I would suggest moving this discussion to a draft PR!
Also, please include at least some tests (even in the draft) to verify the behavior 🚀
Reacted by Harshil PatelHi @pmarchini, I am confused, I read Building.md but not sure what option is best for me to build Node. I have windows. Could you please let me know what is best option to build Node locally for Windows.
Thanks
Hi @pmarchini, I am confused, I read Building.md but not sure what option is best for me to build Node. I have windows. Could you please let me know what is best option to build Node locally for Windows.
Thanks
I would suggest going with option 3: Boxstarter 🚀
Reacted by Harshil Patel@pmarchini How much time do you think the build will take?
Because my building has been running for 1 hour and 10 minutes and still hasn't been completed.@hpatel292-seneca it greatly depends on your machine.
On my Windows laptop it takes circa 25 minutes19 remaining items
- added 7 commits that reference this issue
on Apr 18, 2025 - added a commit that references this issue
on May 19, 2025 - added a commit that references this issue
on May 31, 2025 - added a commit that references this issue
on Jun 10, 2025 There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale
Issues and PRs exempt from automated stale handling. label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
maybe we should add a
levelparameter in diagnostics (i.e debug/info/warn/error) so reporters can implement coloring or other thingsOriginally posted by @MoLow in #55911 (comment)