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: update kPatterns #54728

Merged
merged 4 commits into from
Sep 13, 2024
Merged

Conversation

pmarchini
Copy link
Member

@pmarchini pmarchini commented Sep 3, 2024

Fixes: #54726

@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 Sep 3, 2024
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

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@nodejs-github-bot

This comment was marked as outdated.

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 3, 2024
Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.90%. Comparing base (26b03c1) to head (318f0d2).
Report is 163 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54728      +/-   ##
==========================================
- Coverage   87.90%   87.90%   -0.01%     
==========================================
  Files         651      651              
  Lines      183364   183364              
  Branches    35719    35718       -1     
==========================================
- Hits       161182   161180       -2     
- Misses      15461    15465       +4     
+ Partials     6721     6719       -2     
Files with missing lines Coverage Δ
lib/internal/test_runner/utils.js 63.84% <100.00%> (ø)

... and 22 files with indirect coverage changes

@pmarchini
Copy link
Member Author

Hey @jakecastelli, could I please also ask for your review? 😬
Also, I think we have some issues with the CI

@RedYetiDev
Copy link
Member

Hey @jakecastelli, could I please also ask for your review? 😬

Requesting review per the comment above

Copy link
Member

@jakecastelli jakecastelli left a comment

Choose a reason for hiding this comment

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

Awesome work, you have also fixed the pattern for *-test 😄 looks like we don't have any coverage for *-test pattern, do you also want to add an addition test?

@pmarchini
Copy link
Member Author

@jakecastelli, yes, no problem at all, I'm going to add it ASAP 🐍

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM. Can you change the fixture path to not be "54726"? That naming will not mean a lot in the future.

@targos
Copy link
Member

targos commented Sep 3, 2024

Could be issue-54726

@RedYetiDev RedYetiDev 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 3, 2024
@pmarchini
Copy link
Member Author

@jakecastelli, I added the tests for *-test.
@cjihrig, following @targos suggestion, I renamed the fixtures directory.

@jakecastelli jakecastelli added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@nodejs-github-bot

This comment was marked as outdated.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2024
@github-actions 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 10, 2024

This comment was marked as outdated.

@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2024
@nodejs-github-bot

This comment was marked as outdated.

@pmarchini
Copy link
Member Author

image

I think we are having some issues with the CI

@nodejs-github-bot

This comment was marked as outdated.

@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

nodejs-github-bot commented Sep 12, 2024

@cjihrig cjihrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Sep 13, 2024
@nodejs-github-bot 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 nodejs-github-bot merged commit e21984b into nodejs:main Sep 13, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in e21984b

RafaelGSS pushed a commit that referenced this pull request Sep 16, 2024
PR-URL: #54728
Fixes: #54726
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Sep 16, 2024
RafaelGSS pushed a commit that referenced this pull request Sep 16, 2024
PR-URL: #54728
Fixes: #54726
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Sep 17, 2024
PR-URL: #54728
Fixes: #54726
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@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 22, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
PR-URL: nodejs#54728
Fixes: nodejs#54726
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. 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.

test_runner: latest.js file is included by default