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

[v18.x backport] test_runner features #46839

Closed
wants to merge 20 commits into from
Closed

[v18.x backport] test_runner features #46839

wants to merge 20 commits into from

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Feb 25, 2023 •
edited
Loading

since these commits depend on each other and many of them don't land clearly - backporting these in bulk makes more sense

fossamagna and others added 5 commits February 25, 2023 23:21
@fossamagna @MoLow
test_runner: avoid swallowing of asynchronously thrown errors
9ec0a4e 
Fixes: nodejs#44612
PR-URL: nodejs#45264
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MoLow
test_runner: add reporters
7201845 
PR-URL: nodejs#45712
Fixes: nodejs#45648
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@GeoffreyBooth @MoLow
module: move test reporter loading
a34809a 
Move the logic for handling --test-reporter out of the
general module loader and into the test_runner subsystem.

PR-URL: nodejs#45923
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@MoLow
test_runner: report file in test runner events
b628161 
PR-URL: nodejs#46030
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig @MoLow
test_runner: make built in reporters internal
6d9ec6c 
This commit updates the test runner to make the built in test
reporters internal modules.

PR-URL: nodejs#46092
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Feb 25, 2023
@MoLow MoLow mentioned this pull request Feb 25, 2023
Merged
@MoLow MoLow changed the title [v18.x backport] test_runner reporters [v18.x backport] test_runner features Feb 25, 2023
@cjihrig @MoLow
test_runner: add initial code coverage support
6cb8f02 
This commit adds code coverage functionality to the node:test
module. When node:test is used in conjunction with the new
--test-coverage CLI flag, a coverage report is created when
the test runner finishes. The coverage summary is forwarded to
any test runner reporters so that the display can be customized
as desired. This new functionality is compatible with the
existing NODE_V8_COVERAGE environment variable as well.

There are still several limitations, which will be addressed in
subsequent pull requests:

- Coverage is only reported for a single process. It is possible
  to merge coverage reports together. Once this is done, the
  --test flag will be supported as well.
- Source maps are not currently supported.
- Excluding specific files or directories from the coverage
  report is not currently supported. Node core modules and
  node_modules/ are excluded though.

PR-URL: nodejs#46017
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@MoLow MoLow force-pushed the backport-v18-test-runner-reporters branch from 985157f to dd5e659 Compare February 25, 2023 21:47
cjihrig and others added 14 commits February 25, 2023 23:55
@cjihrig @MoLow
doc,lib,src,test: rename --test-coverage
8ad7668 
Add experimental to the name as requested during review.

PR-URL: nodejs#46017
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@pulkit-30 @MoLow
test: fix tap escaping with and without --test
95250e1 
PR-URL: nodejs#46311
Fixes: nodejs#45836
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@pulkit-30 @MoLow
test_runner: top-level diagnostics not ommited when running with --test
e1f5a3a 
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@MoLow
test_runner: fix missing test diagnostics
f4264f0 
PR-URL: nodejs#46450
Fixes: nodejs#45911
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@pulkit-30 @MoLow
test: fix tap parser fails if a test logs a number
9c49554 
PR-URL: nodejs#46056
Fixes: nodejs#46048
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@MoLow
test_runner: allow nesting test within describe
1502aca 
PR-URL: nodejs#46544
Fixes: nodejs#46478
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MoLow
test_runner: flatten TAP output when running using --test
f7d8bed 
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@mertcanaltin @MoLow
test_runner: parse non-ascii character correctly
730f11f 
PR-URL: nodejs#45736
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@richiemccoll @MoLow
test_runner: display skipped tests in spec reporter output
4a63d24 
PR-URL: nodejs#46651
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@cjihrig @MoLow
test_runner: centralize CLI option handling
32a0802 
The test runner relies on a few CLI options. That code was spread
across a few locations. This commit centralizes that logic.

PR-URL: nodejs#46707
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richiemccoll @MoLow
test_runner: emit test-only diagnostic warning
b50cfb3 
PR-URL: nodejs#46540
Fixes: nodejs#46448
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MoLow
test_runner: bootstrap reporters before running tests
69100c9 
PR-URL: nodejs#46737
Fixes: nodejs#46747
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@richiemccoll @MoLow
test_runner: add describe.only and it.only shorthands
6ef4886 
PR-URL: nodejs#46604
Fixes: nodejs#46562
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@cjihrig @MoLow
test_runner: better handle async bootstrap errors
b7aefcb 
The test runner is bootstrapped synchronously, with the exception
of importing custom reporters. To better handle asynchronously
throw errors, this commit introduces an internal error type that
can be checked for from the test runner's uncaughtException
handler.

After nodejs#46707 and this change
land, the other throw statement in the uncaughtException handler
can be removed. This will allow the test runner to handle errors
thrown from outside of tests (which currently prevents the test
runner from reporting results).

PR-URL: nodejs#46720
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MoLow MoLow force-pushed the backport-v18-test-runner-reporters branch from dd5e659 to b7aefcb Compare February 25, 2023 21:55
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 25, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 25, 2023
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@mertcanaltin @juanarbol
test_runner: parse non-ascii character correctly
8bd3c0f 
PR-URL: #45736
Backport-PR-URL: #46839
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@richiemccoll @juanarbol
test_runner: display skipped tests in spec reporter output
638bbe9 
PR-URL: #46651
Backport-PR-URL: #46839
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@cjihrig @juanarbol
test_runner: centralize CLI option handling
d44f83d 
The test runner relies on a few CLI options. That code was spread
across a few locations. This commit centralizes that logic.

PR-URL: #46707
Backport-PR-URL: #46839
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@richiemccoll @juanarbol
test_runner: emit test-only diagnostic warning
94c870b 
PR-URL: #46540
Backport-PR-URL: #46839
Fixes: #46448
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@MoLow @juanarbol
test_runner: bootstrap reporters before running tests
faedcde 
PR-URL: #46737
Backport-PR-URL: #46839
Fixes: #46747
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@richiemccoll @juanarbol
test_runner: add describe.only and it.only shorthands
bbe1f7d 
PR-URL: #46604
Backport-PR-URL: #46839
Fixes: #46562
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@cjihrig @juanarbol
test_runner: better handle async bootstrap errors
e17d857 
The test runner is bootstrapped synchronously, with the exception
of importing custom reporters. To better handle asynchronously
throw errors, this commit introduces an internal error type that
can be checked for from the test runner's uncaughtException
handler.

After #46707 and this change
land, the other throw statement in the uncaughtException handler
can be removed. This will allow the test runner to handle errors
thrown from outside of tests (which currently prevents the test
runner from reporting results).

PR-URL: #46720
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@juanarbol
Copy link
Member

Landed in 9434396...e17d857 🎉

@juanarbol juanarbol closed this Mar 3, 2023
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@fossamagna @juanarbol
test_runner: avoid swallowing of asynchronously thrown errors
5fdf374 
Fixes: #44612
PR-URL: #45264
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: add reporters
4d50db1 
PR-URL: #45712
Backport-PR-URL: #46839
Fixes: #45648
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@GeoffreyBooth @juanarbol
module: move test reporter loading
403df21 
Move the logic for handling --test-reporter out of the
general module loader and into the test_runner subsystem.

PR-URL: #45923
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: report file in test runner events
79f4b42 
PR-URL: #46030
Backport-PR-URL: #46839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@cjihrig @juanarbol
test_runner: make built in reporters internal
0d999e3 
This commit updates the test runner to make the built in test
reporters internal modules.

PR-URL: #46092
Backport-PR-URL: #46839
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@cjihrig @juanarbol
test_runner: add initial code coverage support
28a775b 
This commit adds code coverage functionality to the node:test
module. When node:test is used in conjunction with the new
--test-coverage CLI flag, a coverage report is created when
the test runner finishes. The coverage summary is forwarded to
any test runner reporters so that the display can be customized
as desired. This new functionality is compatible with the
existing NODE_V8_COVERAGE environment variable as well.

There are still several limitations, which will be addressed in
subsequent pull requests:

- Coverage is only reported for a single process. It is possible
  to merge coverage reports together. Once this is done, the
  --test flag will be supported as well.
- Source maps are not currently supported.
- Excluding specific files or directories from the coverage
  report is not currently supported. Node core modules and
  node_modules/ are excluded though.

PR-URL: #46017
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@cjihrig @juanarbol
doc,lib,src,test: rename --test-coverage
63563f8 
Add experimental to the name as requested during review.

PR-URL: #46017
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@pulkit-30 @juanarbol
test: fix tap escaping with and without --test
32b020c 
PR-URL: #46311
Backport-PR-URL: #46839
Fixes: #45836
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@pulkit-30 @juanarbol
test_runner: top-level diagnostics not ommited when running with --test
c5f16fb 
PR-URL: #46441
Backport-PR-URL: #46839
Fixes: #45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: fix missing test diagnostics
ba784e8 
PR-URL: #46450
Backport-PR-URL: #46839
Fixes: #45911
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@pulkit-30 @juanarbol
test: fix tap parser fails if a test logs a number
4f5aff2 
PR-URL: #46056
Backport-PR-URL: #46839
Fixes: #46048
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: allow nesting test within describe
391ff0d 
PR-URL: #46544
Backport-PR-URL: #46839
Fixes: #46478
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: flatten TAP output when running using --test
5b3c606 
PR-URL: #46440
Backport-PR-URL: #46839
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@mertcanaltin @juanarbol
test_runner: parse non-ascii character correctly
894d711 
PR-URL: #45736
Backport-PR-URL: #46839
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@richiemccoll @juanarbol
test_runner: display skipped tests in spec reporter output
0898145 
PR-URL: #46651
Backport-PR-URL: #46839
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@cjihrig @juanarbol
test_runner: centralize CLI option handling
c19fa45 
The test runner relies on a few CLI options. That code was spread
across a few locations. This commit centralizes that logic.

PR-URL: #46707
Backport-PR-URL: #46839
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@richiemccoll @juanarbol
test_runner: emit test-only diagnostic warning
cd3aaa8 
PR-URL: #46540
Backport-PR-URL: #46839
Fixes: #46448
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@MoLow @juanarbol
test_runner: bootstrap reporters before running tests
28a1317 
PR-URL: #46737
Backport-PR-URL: #46839
Fixes: #46747
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@richiemccoll @juanarbol
test_runner: add describe.only and it.only shorthands
0a690ef 
PR-URL: #46604
Backport-PR-URL: #46839
Fixes: #46562
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@cjihrig @juanarbol
test_runner: better handle async bootstrap errors
1ef3c53 
The test runner is bootstrapped synchronously, with the exception
of importing custom reporters. To better handle asynchronously
throw errors, this commit introduces an internal error type that
can be checked for from the test runner's uncaughtException
handler.

After #46707 and this change
land, the other throw statement in the uncaughtException handler
can be removed. This will allow the test runner to handle errors
thrown from outside of tests (which currently prevents the test
runner from reporting results).

PR-URL: #46720
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MoLow MoLow deleted the backport-v18-test-runner-reporters branch April 2, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanarbol juanarbol juanarbol approved these changes

@benjamingr benjamingr benjamingr approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@MoLow @nodejs-github-bot @juanarbol @benjamingr @fossamagna @GeoffreyBooth @cjihrig @pulkit-30 @mertcanaltin @richiemccoll