-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Conversation
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
|
Hey @jakecastelli, could I please also ask for your review? 😬 |
Requesting review per the comment above |
There was a problem hiding this 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?
@jakecastelli, yes, no problem at all, I'm going to add it ASAP 🐍 |
There was a problem hiding this 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.
Could be |
@jakecastelli, I added the tests for |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
241b6bf
to
318f0d2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Landed in e21984b |
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]>
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]>
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]>
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]>
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]>
Fixes: #54726