test: refactor repl save-load tests #58715
Merged
Conversation
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
nodejs-github-bot
added
needs-ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58715 +/- ##
==========================================
- Coverage 90.13% 90.11% -0.02%
==========================================
Files 639 639
Lines 188192 188192
Branches 36916 36905 -11
==========================================
- Hits 169633 169598 -35
- Misses 11324 11339 +15
- Partials 7235 7255 +20 🚀 New features to boost your workflow:
|
It was discussed ad ad nauseam and there is no real benefit to these refactorings. Can you please add comments instead or split the tests into multiple files? |
The tests needed refactoring and since I was already at it I didn't see much harm in going with the test runner (which is my personal preference). That is not the focal point of this refactoring so I will refactor this not to use the test runner not to get into already overly discussed arguments. |
dario-piotrowicz
force-pushed
the
dario/refactor-test-repl-save-load
branch
from
aca8dd8 to
54657a5
Compare
|
test runner usage removed |
dario-piotrowicz
force-pushed
the
dario/refactor-test-repl-save-load
branch
from
54657a5 to
e6ff309
Compare
lpinca
reviewed
Jun 16, 2025
refactor the test/parallel/test-repl-save-load.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)
- clearly separating and commenting the various tests to make
clearer what is being tested
dario-piotrowicz
force-pushed
the
dario/refactor-test-repl-save-load
branch
from
e6ff309 to
97e90bf
Compare
split tests over multiple files
lpinca
approved these changes
Jun 19, 2025
github-actions
bot
removed
the
request-ci
This comment was marked as outdated.
This comment was marked as outdated.
|
thanks @lpinca 🫶 |
dario-piotrowicz
added
author ready
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.
dario-piotrowicz
added
the
commit-queue
nodejs-github-bot
added
commit-queue-failed
Commit Queue failed- Loading data for nodejs/node/pull/58715 ✔ Done loading data for nodejs/node/pull/58715 ----------------------------------- PR info ------------------------------------ Title test: refactor repl save-load tests (#58715) Author Dario Piotrowicz <dario.piotrowicz@gmail.com> (@dario-piotrowicz) Branch dario-piotrowicz:dario/refactor-test-repl-save-load -> nodejs:main Labels test, author ready, needs-ci, commit-queue-squash Commits 2 - test: refactor repl save-load tests - fixup! test: refactor repl save-load tests Committers 1 - Dario Piotrowicz <dario.piotrowicz@gmail.com> PR-URL: https://github.com/nodejs/node/pull/58715 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58715 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 15 Jun 2025 12:55:54 GMT ✔ Approvals: 1 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58715#pullrequestreview-2943994619 ✘ This PR needs to wait 24 more hours to land (or 0 hours if there is one more approval) ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-06-20T17:08:02Z: https://ci.nodejs.org/job/node-test-pull-request/67573/ - Querying data for job/node-test-pull-request/67573/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/15795695104 |
dario-piotrowicz
removed
the
commit-queue-failed
dario-piotrowicz
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 910a8af |
RafaelGSS
pushed a commit
that referenced
this pull request
Jun 23, 2025
refactor the test/parallel/test-repl-save-load.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)
- clearly separating and commenting the various tests to make
clearer what is being tested
PR-URL: #58715
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This was referenced Jun 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor the test/parallel/test-repl-save-load.js file by:
.clearon it)Pretty much the same as #58636 🙂