Skip to content
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

failing test: parallel/test-vm-debug-context #1291

Closed
brendanashworth opened this issue Mar 28, 2015 · 6 comments
Closed

failing test: parallel/test-vm-debug-context #1291

brendanashworth opened this issue Mar 28, 2015 · 6 comments
Labels
test Issues and PRs related to the tests.

Comments

@brendanashworth
Copy link
Contributor

I just got this failed test on my local machine (MBP, ~2013), and I've seen it before multiple times on the CI. This is one that killed an Ubuntu 14.04 run.

=== release test-vm-debug-context ===                                          
Path: parallel/test-vm-debug-context
assert.js:88
  throw new assert.AssertionError({
        ^
AssertionError: false == true
    at ChildProcess.<anonymous> (io.js/test/parallel/test-vm-debug-context.js:65:3)
    at ChildProcess.<anonymous> (io.js/test/common.js:311:15)
    at ChildProcess.g (events.js:257:16)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:169:7)
    at Process.ChildProcess._handle.onexit (child_process.js:1044:12)
@brendanashworth brendanashworth added the test Issues and PRs related to the tests. label Mar 28, 2015
@bnoordhuis
Copy link
Member

Can you apply this diff and post the output?

diff --git a/test/parallel/test-vm-debug-context.js b/test/parallel/test-vm-debug-context.js
index da2a447..8d0dedf 100644
--- a/test/parallel/test-vm-debug-context.js
+++ b/test/parallel/test-vm-debug-context.js
@@ -56,6 +56,8 @@ assert.strictEqual(vm.runInDebugContext(undefined), undefined);
 var script = common.fixturesDir + '/vm-run-in-debug-context.js';
 var proc = spawn(process.execPath, [script]);
 var data = [];
+proc.stdout.pipe(process.stdout);
+proc.stderr.pipe(process.stderr);
 proc.stdout.on('data', assert.fail);
 proc.stderr.on('data', data.push.bind(data));
 proc.once('exit', common.mustCall(function(exitCode, signalCode) {

@brendanashworth
Copy link
Contributor Author

@bnoordhuis, at HEAD:

$ iojs -v
v1.6.3
$ iojs test/parallel/test-vm-debug-context
undefined:1
*
^
SyntaxError: Unexpected token *
$ iojs test/parallel/test-vm-debug-context

assert.js:88
  throw new assert.AssertionError({
        ^
AssertionError: false == true
    at ChildProcess.<anonymous> (/Users/brendanashworth/Programming/JavaScript/io.js/test/parallel/test-vm-debug-context.js:67:3)
    at ChildProcess.<anonymous> (/Users/brendanashworth/Programming/JavaScript/io.js/test/common.js:311:15)
    at ChildProcess.g (events.js:257:16)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:169:7)
    at Process.ChildProcess._handle.onexit (child_process.js:1044:12)

The first run succeeds, the second doesn't.

It is failling about 1.5 in twenty times, so it isn't that bad, but it is affecting the CI.

@bnoordhuis
Copy link
Member

EDIT: Never mind, looks like my build was out of date. Retested and can reproduce but only very infrequently.

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Mar 29, 2015
Fix a race condition in parallel/test-vm-debug-context where the 'exit'
event for the child process is emitted before the first and only 'data'
event for the child process's stderr stream.

I considered deferring the 'exit' event in lib/child_process.js until
all stdio streams have been closed but I realized that's not going to
work when the child process spins off grandchildren that keep the stdio
file descriptors alive.

Fixes: nodejs#1291
PR-URL: nodejs#1294
Reviewed-By: Brendan Ashworth <[email protected]>
@bnoordhuis
Copy link
Member

Fixed by 8d1c87e.

@brendanashworth
Copy link
Contributor Author

It looks to me as if this just popped up again, failing this Ubuntu 12.04 run... @bnoordhuis ?

@bnoordhuis
Copy link
Member

@brendanashworth That run was for a PR that predates the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

2 participants