FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: report coverage thresholds in `test:coverage` by avivkeller · Pull Request #54813 · nodejs/node · GitHub

/ node Public

test_runner: report coverage thresholds in test:coverage - #54813

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
avivkeller:report-coverage-thresholds
Sep 13, 2024
ConversationCommits (3)ChecksFiles changed
Merged

test_runner: report coverage thresholds in test:coverage#54813
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
avivkeller:report-coverage-thresholds

Conversation

avivkeller commented Sep 6, 2024

Copy link
Copy Markdown
Member

Fixes #54812

Adds a thresholds object to test:coverage's summary property.

nodejs-github-bot commented Sep 6, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

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 Sep 6, 2024
avivkeller added the coverage Issues and PRs related to native coverage support. label Sep 6, 2024
avivkeller mentioned this pull request Sep 6, 2024
Closed

codecov Bot commented Sep 6, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.90%. Comparing base (ce19715) to head (45db682).
Report is 224 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/test_runner/test.js 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54813      +/-   ##
==========================================
+ Coverage   87.60%   87.90%   +0.29%     
==========================================
  Files         650      651       +1     
  Lines      182943   183351     +408     
  Branches    35399    35709     +310     
==========================================
+ Hits       160270   161168     +898     
+ Misses      15936    15465     -471     
+ Partials     6737     6718      -19
Files with missing lines Coverage Δ
lib/internal/test_runner/coverage.js 20.69% <100.00%> (+0.97%) ⬆️
lib/internal/test_runner/test.js 96.93% <0.00%> (ø)

... and 87 files with indirect coverage changes

mcollina approved these changes Sep 7, 2024
View reviewed changes

mcollina 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

rozzilla reviewed Sep 7, 2024
View reviewed changes
Comment thread doc/api/test.md Outdated
avivkeller mentioned this pull request Sep 7, 2024
Merged

avivkeller commented Sep 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

This PR also fixes an issue that (I believe) is causing #53937 to have failing tests.

avivkeller added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 8, 2024
cjihrig requested changes Sep 8, 2024
View reviewed changes

cjihrig left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Requesting changes since this has the approvals to land.

Comment thread doc/api/test.md Outdated
Comment thread lib/internal/main/test_runner.js Outdated
Comment thread lib/internal/test_runner/harness.js Outdated
avivkeller requested a review from cjihrig September 8, 2024 20:52
cjihrig approved these changes Sep 8, 2024
View reviewed changes

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with minor comments.

Comment thread doc/api/test.md Outdated
Comment thread lib/internal/test_runner/test.js Outdated
Update doc/api/test.md
f86abda 
Co-authored-by: Colin Ihrig <cjihrig@gmail.com>
avivkeller added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Sep 8, 2024
github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Sep 9, 2024

github-actions Bot commented Sep 9, 2024

Copy link
Copy Markdown
Contributor
Failed to start CI
   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/10782754798

nodejs-github-bot commented Sep 10, 2024

Copy link
Copy Markdown
Collaborator

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

avivkeller removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Sep 10, 2024

avivkeller commented Sep 12, 2024

Copy link
Copy Markdown
Member Author

The CI appears to have been aborted, can a new one be started in its place?

nodejs-github-bot commented Sep 12, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 12, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 13, 2024

Copy link
Copy Markdown
Collaborator

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

avivkeller commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

CI is 🟢

cjihrig added author ready PRs that have at least one approval, no outstanding review comments, 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 Sep 13, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 13, 2024
nodejs-github-bot merged commit 99bbf80 into nodejs:main Sep 13, 2024

nodejs-github-bot commented Sep 13, 2024

Copy link
Copy Markdown
Collaborator

Landed in 99bbf80

RafaelGSS mentioned this pull request Sep 16, 2024
Merged

avivkeller commented Sep 16, 2024

Copy link
Copy Markdown
Member Author

Is this semver-minor and/or notable change? I know it's being released in v22.9.0, but it does add new information, so I wanted to be sure.

RafaelGSS pushed a commit that referenced this pull request Sep 16, 2024
test_runner: report coverage thresholds in test:coverage
d0ab1b1 
PR-URL: #54813
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>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 17, 2024
test_runner: report coverage thresholds in test:coverage
a52c199 
PR-URL: #54813
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>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
cjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 17, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
MoLow MoLow approved these changes
atlowChemi atlowChemi approved these changes
+1 more reviewer
rozzilla rozzilla left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. coverage Issues and PRs related to native coverage support. semver-minor PRs that contain new features and should be released in the next minor version. 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.

Running tests with --test-coverage-branches (same for lines|functions) doesn't emit test:fail when threshold isn't met

9 participants

Back | FazBrowse Home | New Git URL