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: print failed tests with dot reporter #51769

Closed
mcollina opened this issue Feb 15, 2024 · 7 comments · Fixed by #52655
Closed

test_runner: print failed tests with dot reporter #51769

mcollina opened this issue Feb 15, 2024 · 7 comments · Fixed by #52655
Labels
feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. test_runner

Comments

@mcollina
Copy link
Member

I don't think that the dot reporter is really useful, because in case of failures it does not print the failed tests. Could we make it print the failed tests?

@mcollina mcollina added test_runner feature request Issues that request new features to be added to Node.js. labels Feb 15, 2024
@cjihrig cjihrig added the good first issue Issues that are suitable for first-time contributors. label Mar 27, 2024
@mihir254
Copy link
Contributor

mihir254 commented Apr 19, 2024

Hi! I am interested in working on this issue. I could reproduce the scenario and can see what you mean. I would like to understand the requirements. Is the dot reporter required to print the names of the tests that failed?
example:
...X..X
name_of_the_test_that_failed_1
name_of_the_test_that_failed_2

@RedYetiDev
Copy link
Member

Yes, it seems like that is what @mcollina was going for, but I could be wrong.

@mcollina
Copy link
Member Author

It should report the name of failed tests identically to the spec reporter.

@mihir254
Copy link
Contributor

mihir254 commented Apr 23, 2024

image
Is this how you're expecting it? ("test that is meant to failed" is the name of the test)
I can remove the duration is that is not necessary

@RedYetiDev
Copy link
Member

Looks good to start, if you submit a PR we can review it (and address any errors with it)

mihir254 added a commit to mihir254/node that referenced this issue Apr 23, 2024
Add failed test names and execution times to dot reporter output, improving issue identification without verbose reporting.

Fixes: nodejs#51769
@mihir254
Copy link
Contributor

I've noticed suggestions to include stack traces in the dot reporter's output. Would adding detailed stack traces make it too similar to the spec reporter?
According to the node.js documentation: The dot reporter outputs the test results in a compact format
Looking forward to your thoughts on maintaining the compact nature of the dot reporter while enhancing its functionality.

mihir254 added a commit to mihir254/node that referenced this issue Apr 25, 2024
@MoLow
Copy link
Member

MoLow commented Apr 26, 2024

I've noticed suggestions to include stack traces in the dot reporter's output. Would adding detailed stack traces make it too similar to the spec reporter?

the proposal is to do this only for failed tests

mihir254 added a commit to mihir254/node that referenced this issue May 5, 2024
mihir254 added a commit to mihir254/node that referenced this issue May 6, 2024
test_runner: dot reporter displays failed tests

Fixes: nodejs#51769

test_runner: fixes for dot reporter displaying failed test names

test_runner: fixed conventions for dot reporter

test_runner: added symbols to utils for reusability

test_runner: squashed utility function changes for failed test

test_runner: dot reporter displays failed test name and trace
test_runner: added a utility function to print failed test

test_runner: handled global variable, updated snapshots

test_runner: added space after Failed tests message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. test_runner
Projects
Status: Pending Triage
Development

Successfully merging a pull request may close this issue.

5 participants