-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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: move test-exec.js out of pummel #25686
Conversation
Move test/pummel/test-exec.js to test/sequential/test-child-process-exec.js. The test does not seem to use too much resources to be put in sequential. (It does, at least as written, fail if it has too many other things to compete with, so let's leave it in sequential for now rather than 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.
Looks like something that we could also put into parallel if we want?
It does, however, fail locally for me on master, most likely because of @Fishrock123’s freshly-landed #24951?
@addaleax Make sure you grab latest master, it has a fix. |
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
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.
Do we need to do this? Do we even need this test? What isn't covered by existing parallel/test-child-processes-exec*
tests?
Code coverage doesn't seem to indicate anything glaring..
I'd be OK removing it. |
Remove redundant test/pummel/test-exec.js. Refs: nodejs#25686 (review)
Remove redundant test/pummel/test-exec.js. Refs: nodejs#25686 (review) PR-URL: nodejs#25722 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Closed in favor of #25722 |
Remove redundant test/pummel/test-exec.js. Refs: #25686 (review) PR-URL: #25722 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Move test/pummel/test-exec.js to
test/sequential/test-child-process-exec.js. The test does not seem to
use too much resources to be put in sequential. (It does, at least as
written, fail if it has too many other things to compete with, so let's
leave it in sequential for now rather than parallel.)
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes