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

test_runner: refactor coverage to pass in config options #53931

Merged
merged 1 commit into from
Jul 22, 2024
Merged

test_runner: refactor coverage to pass in config options #53931

merged 1 commit into from
Jul 22, 2024

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jul 18, 2024

This commit updates the test runner's code coverage so that coverage options are explicitly passed in instead of pulled from command line options.

Refs: #53924
Refs: #53867
Refs: #53866

@cjihrig
test_runner: refactor coverage to pass in config options
d36dcdc 
This commit updates the test runner's code coverage so that
coverage options are explicitly passed in instead of pulled
from command line options.
@cjihrig cjihrig requested a review from MoLow July 18, 2024 19:54
@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_runner Issues and PRs related to the test runner subsystem. labels Jul 18, 2024
@cjihrig cjihrig mentioned this pull request Jul 18, 2024
Merged
mcollina
mcollina approved these changes Jul 18, 2024
View reviewed changes
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

@atlowChemi atlowChemi mentioned this pull request Jul 19, 2024
Merged
@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 20, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 20, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60474/

GeoffreyBooth
GeoffreyBooth reviewed Jul 20, 2024
View reviewed changes
Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

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

Wouldn't the documentation change as a result of this?

@cjihrig
Copy link
Contributor Author

cjihrig commented Jul 20, 2024

No, this is only an internal refactor. This change is required so that in the future (#53937) the run() API can be used to configure code coverage.

The back story is that the --test CLI is built on top of run(), but currently run() accesses global state such as command line arguments, process.argv, etc. As much as possible, run() should only work on its inputs. The CLI should parse all of the global state and pass it as config options to run(). This work is currently ongoing.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60476/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60477/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60480/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60521/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 22, 2024 •
edited by cjihrig
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/60524/ 🟢

@cjihrig cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Jul 22, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 22, 2024
@nodejs-github-bot nodejs-github-bot merged commit ea837a0 into nodejs:main Jul 22, 2024
66 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ea837a0

@cjihrig cjihrig deleted the coverage-opts branch July 22, 2024 04:26
targos pushed a commit that referenced this pull request Jul 28, 2024
@cjihrig @targos
test_runner: refactor coverage to pass in config options
280ca56 
This commit updates the test runner's code coverage so that
coverage options are explicitly passed in instead of pulled
from command line options.

PR-URL: #53931
Refs: #53924
Refs: #53867
Refs: #53866
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jul 30, 2024
Merged
RafaelGSS pushed a commit that referenced this pull request Aug 5, 2024
@cjihrig @RafaelGSS
test_runner: refactor coverage to pass in config options
876e7b3 
This commit updates the test runner's code coverage so that
coverage options are explicitly passed in instead of pulled
from command line options.

PR-URL: #53931
Refs: #53924
Refs: #53867
Refs: #53866
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GeoffreyBooth GeoffreyBooth GeoffreyBooth left review comments

@atlowChemi atlowChemi atlowChemi approved these changes

@MoLow MoLow MoLow approved these changes

@jasnell jasnell jasnell approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cjihrig @nodejs-github-bot @mcollina @jasnell @GeoffreyBooth @MoLow @atlowChemi @targos