test: refactor/cleanup a number of cluster tests#8261
test: refactor/cleanup a number of cluster tests#8261jasnell wants to merge 1 commit intonodejs:masterfrom
Conversation
jasnell
added
cluster
There was a problem hiding this comment.
Nit: Since you're already working nearby, maybe change == to ===?
There was a problem hiding this comment.
While you're here... I think process.exit(0) can be removed.
|
The changes LGTM with a couple of comments / suggestions. |
* Move shared code into common * Favor use of strictEqual * Add some missing common.mustCalls * Other general cleanup
jasnell
force-pushed
the
refactor-cluster-tests
branch
from
e03376c to
f6a2c45
Compare
|
@santigimeno ... updated! PTAL! |
|
LGTM with one tiny comment that you can ignore. Thanks! |
|
Trying again due to a build bot failure: https://ci.nodejs.org/job/node-test-pull-request/3913/ |
|
CI was green. woo! |
* Move shared code into common * Favor use of strictEqual * Add some missing common.mustCalls * Other general cleanup PR-URL: #8261 Reviewed-By: Santiago Gimeno <[email protected]>
|
Landed in baa0ffd |
|
Looking at the AIX runs I've seen a few failures of: parallel/test-cluster-dgram-1 I wonder if its related to this change ? |
|
If the failures are new, entirely possible. Have a stack trace? |
|
opened this issue to track #8380 |
5 participants
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test, cluster
Description of change
Some general refactoring / improvements to various cluster related tests: