-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
failed test: parallel/test-child-process-stdout-flush-exit #944
Comments
Can't reproduce this any longer. Closing for now. |
There's something flaky going on here:
OS X 10.10.2, io.js at 78581c8, happened only once so far. |
@bnoordhuis Could this have something to do with stdout being asynchronous? |
I've also seen this on os x 10.9.5. |
@piscisaureus That seems to be the issue, yes. The test failure is 100% reproducible when you add a delay in the parent process: diff --git a/test/parallel/test-child-process-stdout-flush-exit.js b/test/parallel/test-child-process-stdout-flush-exit.js
index eba8927..5fbb1e9 100644
--- a/test/parallel/test-child-process-stdout-flush-exit.js
+++ b/test/parallel/test-child-process-stdout-flush-exit.js
@@ -17,6 +17,8 @@ if (process.argv[2] === 'child') {
// spawn self as child
var child = spawn(process.argv[0], [process.argv[1], 'child']);
+ for (var t = Date.now(); t + 1e3 > Date.now(););
+
var gotHello = false;
var gotBye = false;
It doesn't seem to be an issue of libuv buffering data and not getting a chance to flush it because even when all writes hit the kernel, the test still fails. I speculate that data ends up in the kernel pipe buffer and is subsequently lost when the child process exits. |
Also cropped up on the latest (otherwise green) windows build: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/359/nodes=iojs-win2012r2/console |
been a semi-regular offender on the Windows CI machines of late actually |
I'm seeing this 100% of the time when a child process outputs a large amount of text (>8k) and calls subprocess.exit(). here is a gist that repros: https://gist.github.com/abrady0/f69de07b79d1b50b0ca0 if you comment line 43 out it will succeed. |
Should be fixed in |
Make that b926718. |
Make sure all the stdout data is available before performing the assertions. Fixes: nodejs/node#944 PR-URL: nodejs/node#1868 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
I'm seeing a rare test fail (been able to reproduce twice over ~100 test runs) on a mac os x 10.10 machine. Tested with io.js 8a1e22a:
The text was updated successfully, but these errors were encountered: