From f7e7d92ab102d1382019dcf8f0c595939d78cabc Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Fri, 16 Sep 2016 11:40:28 -0700 Subject: [PATCH 1/3] test: fix test-debug-signal-cluster.js flakyness Do not assume any order and buffering/atomicity of output from child processes' debugger agents. Fixes #3796. --- test/parallel/test-debug-signal-cluster.js | 66 +++++++++++++--------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/test/parallel/test-debug-signal-cluster.js b/test/parallel/test-debug-signal-cluster.js index 8cda7d3567726c..1e02a8491fdcb5 100644 --- a/test/parallel/test-debug-signal-cluster.js +++ b/test/parallel/test-debug-signal-cluster.js @@ -3,6 +3,7 @@ const common = require('../common'); const assert = require('assert'); const spawn = require('child_process').spawn; +const os = require('os'); const path = require('path'); const port = common.PORT; @@ -11,13 +12,24 @@ const args = [`--debug-port=${port}`, serverPath]; const options = { stdio: ['inherit', 'inherit', 'pipe', 'ipc'] }; const child = spawn(process.execPath, args, options); -const outputLines = []; -var waitingForDebuggers = false; +var expectedContent = [ + 'Starting debugger agent.', + 'Debugger listening on 127.0.0.1:' + (port + 0), + 'Starting debugger agent.', + 'Debugger listening on 127.0.0.1:' + (port + 1), + 'Starting debugger agent.', + 'Debugger listening on 127.0.0.1:' + (port + 2), +].join(os.EOL); +expectedContent += os.EOL; // the last line also contains an EOL character + +var debuggerAgentsOutput = ''; +var debuggerAgentsStarted = false; var pids; child.stderr.on('data', function(data) { - const lines = data.toString().replace(/\r/g, '').trim().split('\n'); + const childStderrOutputString = data.toString(); + const lines = childStderrOutputString.replace(/\r/g, '').trim().split('\n'); lines.forEach(function(line) { console.log('> ' + line); @@ -30,24 +42,26 @@ child.stderr.on('data', function(data) { pids = msg.pids; console.error('got pids %j', pids); - waitingForDebuggers = true; process._debugProcess(child.pid); + debuggerAgentsStarted = true; }); child.send({ type: 'getpids' }); - } else if (waitingForDebuggers) { - outputLines.push(line); } - }); - if (outputLines.length === expectedLines.length) - onNoMoreLines(); + + if (debuggerAgentsStarted) { + debuggerAgentsOutput += childStderrOutputString; + if (debuggerAgentsOutput.length === expectedContent.length) { + onNoMoreDebuggerAgentsOutput(); + } + } }); -function onNoMoreLines() { - assertOutputLines(); +function onNoMoreDebuggerAgentsOutput() { + assertDebuggerAgentsOutput(); process.exit(); } @@ -63,21 +77,19 @@ process.on('exit', function onExit() { }); }); -const expectedLines = [ - 'Starting debugger agent.', - 'Debugger listening on 127.0.0.1:' + (port + 0), - 'Starting debugger agent.', - 'Debugger listening on 127.0.0.1:' + (port + 1), - 'Starting debugger agent.', - 'Debugger listening on 127.0.0.1:' + (port + 2), -]; - -function assertOutputLines() { - // Do not assume any particular order of output messages, - // since workers can take different amout of time to - // start up - outputLines.sort(); - expectedLines.sort(); +function assertDebuggerAgentsOutput() { + // Workers can take different amout of time to start up, and child processes' + // output may be interleaved arbitrarily. Moreover, child processes' output + // may be written using an arbitrary number of system calls, and no assumption + // on buffering or atomicity of output should be made. Thus, we process the + // output of all chid processes' debugger agents character by character, and + // remove each character from the set of expected characters. Once all the + // output from all debugger agents has been processed, we consider that we got + // the content we expected if there's no character left in the initial + // expected content. + debuggerAgentsOutput.split('').forEach(function gotChar(char) { + expectedContent = expectedContent.replace(char, ''); + }); - assert.deepStrictEqual(outputLines, expectedLines); + assert.equal(expectedContent, ''); } From 522b969748b93e3e9b04fc6141cda17f0496a0db Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Fri, 16 Sep 2016 14:32:18 -0700 Subject: [PATCH 2/3] changes according to code review comments --- test/parallel/test-debug-signal-cluster.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-debug-signal-cluster.js b/test/parallel/test-debug-signal-cluster.js index 1e02a8491fdcb5..c5931b7de4021e 100644 --- a/test/parallel/test-debug-signal-cluster.js +++ b/test/parallel/test-debug-signal-cluster.js @@ -82,7 +82,7 @@ function assertDebuggerAgentsOutput() { // output may be interleaved arbitrarily. Moreover, child processes' output // may be written using an arbitrary number of system calls, and no assumption // on buffering or atomicity of output should be made. Thus, we process the - // output of all chid processes' debugger agents character by character, and + // output of all child processes' debugger agents character by character, and // remove each character from the set of expected characters. Once all the // output from all debugger agents has been processed, we consider that we got // the content we expected if there's no character left in the initial @@ -91,5 +91,5 @@ function assertDebuggerAgentsOutput() { expectedContent = expectedContent.replace(char, ''); }); - assert.equal(expectedContent, ''); + assert.strictEqual(expectedContent, ''); } From 98039195404389b1cb12f12f238502506d935ff1 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Fri, 16 Sep 2016 14:42:59 -0700 Subject: [PATCH 3/3] actually remove test from flaky list --- test/parallel/parallel.status | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 02fb2983c6669e..8c3239909b255c 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -19,7 +19,6 @@ test-tick-processor-cpp-core : PASS,FLAKY test-tick-processor-unknown : PASS,FLAKY [$system==solaris] # Also applies to SmartOS -test-debug-signal-cluster : PASS,FLAKY [$system==freebsd] @@ -34,9 +33,5 @@ test-fs-watch-encoding : FAIL, PASS #being worked under https://github.com/nodejs/node/issues/7973 test-stdio-closed : PASS, FLAKY -#covered by https://github.com/nodejs/node/issues/3796 -# but more frequent on AIX ? -test-debug-signal-cluster : PASS, FLAKY - #covered by https://github.com/nodejs/node/issues/8271 test-child-process-fork-dgram : PASS, FLAKY