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.

Sign up for GitHub

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

Jump to bottom

Some way of not displaying skipped or filtered tests in the spec test reporter #51383

Closed
domenic opened this issue Jan 5, 2024 · 5 comments · Fixed by #52221
Closed

Some way of not displaying skipped or filtered tests in the spec test reporter #51383

domenic opened this issue Jan 5, 2024 · 5 comments · Fixed by #52221
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@domenic
Copy link
Contributor

domenic commented Jan 5, 2024

What is the problem this feature will solve?

In jsdom, we have 4187 tests. Sometimes I would like to run only a few of them, so I experimented with --test-name-pattern="dom/events" to only run tests under my dom/events suite.

The result was that I saw 4100+ lines of skipped tests, within which there was somewhere buried ~50 lines of passing tests. This was not a great experience.

Compare this with Mocha, where using --fgrep="dom/events" only reports tests that match the pattern.

What is the feature you are proposing to solve the problem?

I see basically two ways to solve this:

  • Treat filtered-out tests differently from skipped tests. For example, the totals displayed at the bottom of the test report could exclude filtered-out tests entirely. (This is how Mocha behaves.)

  • Provide some way of not displaying skipped tests when using the spec reporter. E.g., --test-show-skipped=false or something.

The former would be better for me, because sometimes I actually have tests marked as long-term skipped, and I'd like to see those even when I'm filtering. E.g. if I have 50 dom/events tests, of which 3 are skipped, running with --test-name-pattern="dom/events" would ideally show 47 passes and 3 skipped. If we filtered away all skipped tests, then it would show only the 47 passes.

The latter would probably be easier to implement, and fit better with how the test runner currently works.

What alternatives have you considered?

I could write my own reporter, I guess.
@domenic domenic added the feature request Issues that request new features to be added to Node.js. label Jan 5, 2024
@domenic domenic mentioned this issue Jan 5, 2024
Open
@benjamingr benjamingr added the test_runner Issues and PRs related to the test runner subsystem. label Jan 5, 2024
@benjamingr
Copy link
Member

@nodejs/test_runner not displaying filtered out tests seems reasonable to me wdyt?

@MoLow
Copy link
Member

MoLow commented Jan 6, 2024

I think filtering tests and running only test is currently broken in node:test. we have a few issues discussing that #48932

@rluvaton
Copy link
Member

rluvaton commented Feb 5, 2024

Should it not be displayed only in the spec reporter?

@cjihrig cjihrig mentioned this issue Mar 14, 2024
Merged
cjihrig added a commit to cjihrig/node that referenced this issue Mar 26, 2024
@cjihrig
test_runner: omit filtered test from output
844c85d 
This commit updates the test runner to suppress any output for
filtered tests. Filtered tests no longer generate reporter events,
and the unfiltered tests are renumbered in the output as though
the filtered tests were not present. Skipped tests that are not
filtered are still included in the output.

This change is particularly useful when filtering a large number
of tests, as the previously displayed skip output could be
distracting.

Fixes: nodejs#51383
cjihrig added a commit to cjihrig/node that referenced this issue Mar 26, 2024
@cjihrig
test_runner: omit filtered test from output
d2de388 
This commit updates the test runner to suppress any output for
filtered tests. Filtered tests no longer generate reporter events,
and the unfiltered tests are renumbered in the output as though
the filtered tests were not present. Skipped tests that are not
filtered are still included in the output.

This change is particularly useful when filtering a large number
of tests, as the previously displayed skip output could be
distracting.

Fixes: nodejs#51383
@cjihrig cjihrig mentioned this issue Mar 26, 2024
Merged
nodejs-github-bot pushed a commit that referenced this issue Mar 28, 2024
@cjihrig
test_runner: omit filtered test from output
29de7f8 
This commit updates the test runner to suppress any output for
filtered tests. Filtered tests no longer generate reporter events,
and the unfiltered tests are renumbered in the output as though
the filtered tests were not present. Skipped tests that are not
filtered are still included in the output.

This change is particularly useful when filtering a large number
of tests, as the previously displayed skip output could be
distracting.

Fixes: #51383
PR-URL: #52221
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@RedYetiDev

This comment was marked as outdated.

Sign in to view
@RedYetiDev RedYetiDev mentioned this issue Apr 16, 2024
Merged
@RedYetiDev
Copy link
Member

I apologize for my oversight, the information in that comment is incorrect. I'm sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test_runner: omit filtered test from output cjihrig/node
5 participants
@domenic @benjamingr @MoLow @rluvaton @RedYetiDev