-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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: update test-eventsource.js to use new test runner #55747
Conversation
tlhunter
commented
Nov 6, 2024
•
edited
Loading
edited
- updating a test to use the new test runner
- NodeConfEU says hi
IMHO this isn't needed because there's only a single test |
The descriptive text adds extra context in case this test fails, so it seems useful even for single tests like this. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55747 +/- ##
==========================================
- Coverage 88.40% 88.40% -0.01%
==========================================
Files 654 654
Lines 187747 187747
Branches 36127 36126 -1
==========================================
- Hits 165972 165970 -2
- Misses 15009 15013 +4
+ Partials 6766 6764 -2
|
I agree. It's totally useless, slows down test execution, etc. I'm not sure why this "refactor to use node:test" was chosen as a code and learn exercise when there is no official TSC stance against it and there are contributors (me in primis) who are against it. I don't block these PRs out of respect for the authors who probably don't know about it. |