Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: add error message if custom reporter throws error #48974

Closed
wants to merge 0 commits into from

Conversation

ocodista
Copy link
Contributor

@ocodista ocodista commented Jul 30, 2023

This goal of this PR is to solve the issue 48937

Where the test silently fails if there's an exception inside the Custom test Reporter.

I created this repo with a minimum reproduction of the bug.

I also created a new test (that's currently failing) to check if the Custom Reporter error message will be shown.

I'm still working on getting the exception from the custom reporter (start step) and displaying it on the test result.

Any help/guidance would be appreciated!

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jul 30, 2023
@atlowChemi
Copy link
Member

Any help/guidance would be appreciated!

@ocodista Is there anything specific you would like help with? 🙂

@ocodista
Copy link
Contributor Author

Any help/guidance would be appreciated!

@ocodista Is there anything specific you would like help with? 🙂

I'm still not sure where the call is being made so I can wrap it with a try-catch.

Also, it takes an eternity to build node on my PC, is there a way to make it faster? 😅

This way would be easier to add console logs and find it 😁

@atlowChemi
Copy link
Member

atlowChemi commented Jul 31, 2023

I'm still not sure where the call is being made so I can wrap it with a try-catch.

Reporter details

We create the reporter here: https://github.com/nodejs/node/blob/4b3d964a8c01c4b541fc85d3a419b58524806f04/lib/internal/test_runner/utils.js#L165-L172

Basically we call compose on the TestStream (rootTest.reporter) with the relevant reporters

this.reporter = new TestsStream();

This compose function is a Stream function you can find here:

module.exports = function compose(...streams) {

Also, it takes an eternity to build node on my PC, is there a way to make it faster? 😅

This depends on what OS you are using, but I suggest having a look at BUILDING.MD:

## Building Node.js on supported platforms

This way would be easier to add console logs and find it 😁

If you are only changing JS modules (and no src c++ code etc) you can avoid additional builds by setting the --node-builtin-modules-path option:

node/BUILDING.md

Lines 548 to 558 in 4b3d964

When modifying only the JS layer in `lib`, it is possible to externally load it
without modifying the executable:
```bash
./configure --node-builtin-modules-path "$(pwd)"
```
The resulting binary won't include any JS files and will try to load them from
the specified directory. The JS debugger of Visual Studio Code supports this
configuration since the November 2020 version and allows for setting
breakpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throwing an exception on test reporters silently error the test runner
3 participants