-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
test: refactor repl tab complete tests #58636
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: refactor repl tab complete tests #58636
Conversation
// TODO: the following async IIFE and the completePromise function are necessary because | ||
// the reply tests are all run against the same repl instance (testMe) and thus coordination | ||
// needs to be in place for the tests not to interfere with each other, this is really | ||
// not ideal, the tests in this file should be refactored so that each use its own isolated | ||
// repl instance, making sure that no special coordination needs to be in place for them | ||
// and also allowing the tests to all be run in parallel |
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.
PS: this is the driving reason for this PR
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58636 +/- ##
==========================================
- Coverage 90.16% 90.15% -0.02%
==========================================
Files 637 637
Lines 188001 188001
Branches 36881 36877 -4
==========================================
- Hits 169509 169484 -25
- Misses 11238 11286 +48
+ Partials 7254 7231 -23 🚀 New features to boost your workflow:
|
ae1c33f
to
ce45b2a
Compare
cc. @nodejs/repl would anyone like to have a look? 🙂 |
refactor the test/parallel/test-repl-tab-complete.js file by: - making the tests in the file self-contained (instead of all of them sharing the same REPL instance and constantly calling `.clear` on it) - using the test runner with appropriate descriptions to make clearer what is being tested - extracting some tests in their own js test files (to increase isolation of the tests and help with issues such as flakiness)
Co-authored-by: Antoine du Hamel <[email protected]>
40bfac8
to
0d6cb84
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Landed in aee9bc0 |
refactor the test/parallel/test-repl-tab-complete.js file by: - making the tests in the file self-contained (instead of all of them sharing the same REPL instance and constantly calling `.clear` on it) - using the test runner with appropriate descriptions to make clearer what is being tested - extracting some tests in their own js test files (to increase isolation of the tests and help with issues such as flakiness) PR-URL: #58636 Reviewed-By: Giovanni Bucci <[email protected]> Reviewed-By: James M Snell <[email protected]>
This doesn't land cleanly on v22.x-staging, we'd need a backport PR |
refactor the test/parallel/test-repl-tab-complete.js file by:
.clear
on it)