-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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: make test-cli-node-options more robust #26994
Conversation
The test launches over 20 processes asynchronously. That may be too many for underpowered machines in CI with limited PID space. Fixes: nodejs#25028
@Trott Sadly, an error occurred when I tried to trigger a build. :( |
Stress test against master: https://ci.nodejs.org/job/node-stress-single-test/2196/nodes=ubuntu1604-arm64/ Stress test against this PR: https://ci.nodejs.org/job/node-stress-single-test/nodes=ubuntu1604-arm64/2197/ (scheduled) |
Welp, that didn't go as planned.... 10:26:19 not ok 445 parallel/test-cli-node-options
10:26:19 ---
10:26:19 duration_ms: 7.544
10:26:19 severity: fail
10:26:19 exitcode: 1
10:26:19 stack: |-
10:26:19 /home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64/test/parallel/test-cli-node-options.js:83
10:26:19 throw e;
10:26:19 ^
10:26:19
10:26:19 Error: Command failed: /home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64/out/Release/node -e console.log("B")
10:26:19
10:26:19
10:26:19 #
10:26:19 # Fatal error in , line 0
10:26:19 # Check failed: (perf_output_handle_) != nullptr.
10:26:19 #
10:26:19 #
10:26:19 #
10:26:19 #FailureMessage Object: 0xffffe677c798
10:26:19 at checkExecSyncError (child_process.js:622:11)
10:26:19 at execFileSync (child_process.js:640:13)
10:26:19 at expect (/home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64/test/parallel/test-cli-node-options.js:80:14)
10:26:19 at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64/test/parallel/test-cli-node-options.js:43:3)
10:26:19 at Module._compile (internal/modules/cjs/loader.js:837:30)
10:26:19 at Object.Module._extensions..js (internal/modules/cjs/loader.js:848:10)
10:26:19 at Module.load (internal/modules/cjs/loader.js:704:32)
10:26:19 at tryModuleLoad (internal/modules/cjs/loader.js:636:12)
10:26:19 at Function.Module._load (internal/modules/cjs/loader.js:628:3)
10:26:19 at Function.Module.runMain (internal/modules/cjs/loader.js:903:10)
10:26:19 ... |
Btw, I’d personally say that it’s not bad to test that we can spawn a large-ish number of child processes which do very little work each all at once, no matter what the outcome here is. |
Agreed. I'd prefer a test be specifically designed to test that, rather than it be a side effect of an otherwise unrelated test. In fact, I think we already have a test like that: test/pummel/test-child-process-spawn-loop.js (EDIT: Of course, a big limitation of that test being in |
The test launches over 20 processes asynchronously. That may be too many
for underpowered machines in CI with limited PID space.
Fixes: #25028
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes