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: move coverage collection to root.postRun() #47651

Merged
merged 1 commit into from
Apr 23, 2023

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Apr 20, 2023

This commit moves code coverage collection from the test harness exit handler to the postRun() function of the root test.

This is necessary preparatory work for supporting code coverage with --test. The reason is that --test is implemented on top of run(), and that function calls the root test's postRun() function, which outputs the test summary. This happens before the harness exit handler.

This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Apr 20, 2023
@MoLow
Copy link
Member

MoLow commented Apr 20, 2023

This is necessary preparatory work for supporting code coverage with --test

Am I missing something? coverage is not supported with --test?

@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 20, 2023

coverage is not supported with --test?

Correct -

node/doc/api/test.md

Lines 423 to 432 in 7b39e80

The test runner's code coverage functionality has the following limitations,
which will be addressed in a future Node.js release:
* Although coverage data is collected for child processes, this information is
not included in the coverage report. Because the command line test runner uses
child processes to execute test files, it cannot be used with
`--experimental-test-coverage`.
* Source maps are not supported.
* Excluding specific files or directories from the coverage report is not
supported.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@MoLow MoLow added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 23, 2023
@nodejs-github-bot nodejs-github-bot merged commit ecb023b into nodejs:main Apr 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ecb023b

@cjihrig cjihrig deleted the coverage-postrun branch April 23, 2023 15:12
yjl9903 pushed a commit to yjl9903/node that referenced this pull request Apr 28, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: nodejs#47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
yjl9903 pushed a commit to yjl9903/node that referenced this pull request Apr 28, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: nodejs#47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
yjl9903 pushed a commit to yjl9903/node that referenced this pull request Apr 29, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: nodejs#47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this pull request May 2, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: #47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@targos targos mentioned this pull request May 2, 2023
@muturgan
Copy link

muturgan commented Jun 3, 2023

will it be added in version 18?

@MoLow
Copy link
Member

MoLow commented Jun 4, 2023

@nodejs/releasers is there a v18 release scheduled?

danielleadams pushed a commit that referenced this pull request Jul 6, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: #47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
This commit moves code coverage collection from the test
harness exit handler to the postRun() function of the root
test.

This is necessary preparatory work for supporting
code coverage with --test. The reason is that --test is
implemented on top of run(), and that function calls the root
test's postRun() function, which outputs the test summary. This
happens before the harness exit handler.

PR-URL: nodejs#47651
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants