Skip to content

Commit

Permalink
test: fix flaky child-process-exec-kill-throws
Browse files Browse the repository at this point in the history
This is a fix for test-child-process-exec-kill-throws which is currently
flaky on Windows.

A bug in the test was causing the child process to fail for reasons
other than those intended by the test. Instead of failing for exceeding
the `maxBuffer` setting, the test was failing because it was trying to
load `internal/child_process` without being passed the
`expose-internals` flag. Move that module to where only the parent
process (which gets the flag) loads it.

Additionally, improve an assertion message to help debug problems like
this.

PR-URL: #12111
Fixes: #12053
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
Trott committed Mar 31, 2017
1 parent 92de91d commit a10e657
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 7 deletions.
3 changes: 0 additions & 3 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ prefix parallel
[true] # This section applies to all platforms

[$system==win32]
# See https://github.com/nodejs/node/issues/12053, this test may be exposing a
# genuine bug with kill() on Windows.
test-child-process-exec-kill-throws : PASS,FLAKY

[$system==linux]

Expand Down
9 changes: 5 additions & 4 deletions test/parallel/test-child-process-exec-kill-throws.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');
const internalCp = require('internal/child_process');

if (process.argv[2] === 'child') {
// Keep the process alive and printing to stdout.
setInterval(() => { console.log('foo'); }, 1);
// Since maxBuffer is 0, this should trigger an error.
console.log('foo');
} else {
const internalCp = require('internal/child_process');

// Monkey patch ChildProcess#kill() to kill the process and then throw.
const kill = internalCp.ChildProcess.prototype.kill;

Expand All @@ -21,7 +22,7 @@ if (process.argv[2] === 'child') {
const options = { maxBuffer: 0 };
const child = cp.exec(cmd, options, common.mustCall((err, stdout, stderr) => {
// Verify that if ChildProcess#kill() throws, the error is reported.
assert(/^Error: mock error$/.test(err));
assert.strictEqual(err.message, 'mock error', err);
assert.strictEqual(stdout, '');
assert.strictEqual(stderr, '');
assert.strictEqual(child.killed, true);
Expand Down

0 comments on commit a10e657

Please sign in to comment.