Skip to content

test_runner: pass options directly to TestCoverage - #55578

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
avivkeller:options-testcoverage
Nov 1, 2024
Merged

test_runner: pass options directly to TestCoverage#55578
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
avivkeller:options-testcoverage

Conversation

@avivkeller

Copy link
Copy Markdown
Member

This should make future modifications to the TestCoverage class simpler, as additional options don't need to be added to the constructor.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Oct 28, 2024
@codecov

codecovBot commented Oct 29, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.32%. Comparing base (5633c62) to head (c982694).
Report is 200 commits behind head on main.

Files with missing linesPatch %Lines
lib/internal/test_runner/coverage.js95.83%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #55578 +/- ##
==========================================
+ Coverage 87.93% 88.32% +0.39% 
==========================================
Files 654 654 Lines 187686 187658 -28 Branches 35819 36049 +230 ==========================================
+ Hits 165038 165746 +708 + Misses 15849 15125 -724 + Partials 6799 6787 -12 
Files with missing linesCoverage Δ
lib/internal/test_runner/coverage.js64.34% <95.83%> (-0.47%)⬇️

... and 98 files with indirect coverage changes

Comment threadlib/internal/test_runner/coverage.js

@pmarchinipmarchini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@avivkelleravivkeller 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 Oct 29, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@avivkeller

Copy link
Copy Markdown
MemberAuthor

CI is 💚 , can someone add the commit-queue label so once 48hrs pass this can land quickly and smoothly?

@JakobJingleheimer

Copy link
Copy Markdown
Member

IIR, you can add it whenever, and once the wait time has elapsed, it will kick in automatically

@avivkeller

avivkeller commented Oct 29, 2024

Copy link
Copy Markdown
MemberAuthor

IIR, you can add it whenever, and once the wait time has elapsed, it will kick in automatically

I can't, I'm not a collaborator. If I could, I would.

@pmarchinipmarchini added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 29, 2024
@JakobJingleheimer

Copy link
Copy Markdown
Member

It looks like CI stalled after everything passed, and just didn't mark itself completed.

@JakobJingleheimerJakobJingleheimer removed the needs-ci PRs that need a full CI run. label Oct 30, 2024
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 30, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/55578
✔ Done loading data for nodejs/node/pull/55578
----------------------------------- PR info ------------------------------------
Title test_runner: pass `options` directly to `TestCoverage` (#55578)
Author Aviv Keller <redyetidev@gmail.com> (@RedYetiDev)
Branch RedYetiDev:options-testcoverage -> nodejs:main
Labels author ready, test_runner
Commits 1
- test_runner: pass `options` directly to `TestCoverage`
Committers 1
- RedYetiDev <38299977+RedYetiDev@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/55578
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55578
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
--------------------------------------------------------------------------------
ℹ This PR was created on Mon, 28 Oct 2024 23:00:22 GMT
✔ Approvals: 1
✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/55578#pullrequestreview-2401251223
✘ This PR needs to wait 119 more hours to land (or 0 hours if there is one more approval)
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-10-29T14:40:07Z: https://ci.nodejs.org/job/node-test-pull-request/63337/
- Querying data for job/node-test-pull-request/63337/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/11603069194

@avivkeller

avivkeller commented Oct 30, 2024

Copy link
Copy Markdown
MemberAuthor

The GitHub bot didn't count at @pmarchini's review to do a typo in the README. See #55625

@avivkelleravivkeller removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 31, 2024
@avivkeller

Copy link
Copy Markdown
MemberAuthor

Thanks @rluvaton! Now that this has two countable reviews, can someone add the commit-queue label?

@rluvatonrluvaton added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 1, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 1, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 9b6cea6 into nodejs:mainNov 1, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 9b6cea6

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2024
PR-URL: #55578
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
@ruyadorno

ruyadorno commented Nov 27, 2024

Copy link
Copy Markdown
Member

This commit does not land cleanly on v22.x-staging and will need manual backport in case we want it in v22.x.

Backport blocked by #54705 (comment)

@ruyadornoruyadorno added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Nov 27, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.backport-requested-v22.xPRs awaiting manual backport to the v22.x-staging branch.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@avivkeller@nodejs-github-bot@JakobJingleheimer@ruyadorno@MoLow@rluvaton@pmarchini