From 7926886bf322ec9c69ec20afa26edfad17a47e08 Mon Sep 17 00:00:00 2001
From: Julien Gilli <julien.gilli@joyent.com>
Date: Fri, 16 Sep 2016 11:40:28 -0700
Subject: [PATCH] test: fix test-debug-signal-cluster.js flakyness

Do not assume any order and buffering/atomicity of output from child
processes' debugger agents.

Fixes: https://github.com/nodejs/node/issues/3796
PR-URL: https://github.com/nodejs/node/pull/8568
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
---
 test/parallel/parallel.status              |  5 --
 test/parallel/test-debug-signal-cluster.js | 66 +++++++++++++---------
 2 files changed, 39 insertions(+), 32 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
diff --git a/test/parallel/test-debug-signal-cluster.js b/test/parallel/test-debug-signal-cluster.js
index 8cda7d3567726c..c5931b7de4021e 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 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
+  // expected content.
+  debuggerAgentsOutput.split('').forEach(function gotChar(char) {
+    expectedContent = expectedContent.replace(char, '');
+  });
 
-  assert.deepStrictEqual(outputLines, expectedLines);
+  assert.strictEqual(expectedContent, '');
 }