From 916e694b2b05b98b646f646806858607e67cfb37 Mon Sep 17 00:00:00 2001
From: Stefan Budeanu <stefan@budeanu.com>
Date: Mon, 2 May 2016 16:17:13 -0400
Subject: [PATCH] test: avoid test-cluster-master-* flakiness

Removed reliance on worker exit before arbitrary timeout. Instead of failing
the test after 200 or 1000 ms wait indefinitely for child process exit. If
the test hangs the test harness global timeout will kick in and fail the test.

Note that if the orphaned children are not reaped correctly (in the absence
of init, e.g. Docker) the test will hang and the harness will fail it.

PR-URL: https://github.com/nodejs/node/pull/6531
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
---
 test/parallel/test-cluster-master-error.js | 45 ++++++++++------------
 test/parallel/test-cluster-master-kill.js  | 21 ++++------
 2 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/test/parallel/test-cluster-master-error.js b/test/parallel/test-cluster-master-error.js
index ae0f655bb8e5d0..1f0cb8c9098c62 100644
--- a/test/parallel/test-cluster-master-error.js
+++ b/test/parallel/test-cluster-master-error.js
@@ -30,7 +30,7 @@ if (cluster.isWorker) {
     }
   });
 
-  // Throw accidently error when all workers are listening
+  // Throw accidental error when all workers are listening
   var listeningNum = 0;
   cluster.on('listening', function listeningEvent() {
 
@@ -39,10 +39,10 @@ if (cluster.isWorker) {
       // Stop listening
       cluster.removeListener('listening', listeningEvent);
 
-      // throw accidently error
+      // Throw accidental error
       process.nextTick(function() {
         console.error('about to throw');
-        throw new Error('accidently error');
+        throw new Error('accidental error');
       });
     }
 
@@ -68,8 +68,8 @@ if (cluster.isWorker) {
     }
   };
 
-  var existMaster = false;
-  var existWorker = false;
+  var masterExited = false;
+  var workersExited = false;
 
   // List all workers
   var workers = [];
@@ -89,36 +89,33 @@ if (cluster.isWorker) {
   // When cluster is dead
   master.on('exit', function(code) {
 
-    // Check that the cluster died accidently
-    existMaster = !!code;
+    // Check that the cluster died accidentally (non-zero exit code)
+    masterExited = !!code;
 
-    // Give the workers time to shut down
-    var timeout = 200;
-    if (common.isAix) {
-      // AIX needs more time due to default exit performance
-      timeout = 1000;
-    }
-    setTimeout(checkWorkers, timeout);
-
-    function checkWorkers() {
-      // When master is dead all workers should be dead to
+    var pollWorkers = function() {
+      // When master is dead all workers should be dead too
       var alive = false;
       workers.forEach(function(pid) {
         if (isAlive(pid)) {
           alive = true;
         }
       });
-
-      // If a worker was alive this did not act as expected
-      existWorker = !alive;
-    }
+      if (alive) {
+        setTimeout(pollWorkers, 50);
+      } else {
+        workersExited = true;
+      }
+    };
+
+    // Loop indefinitely until worker exit
+    pollWorkers();
   });
 
   process.once('exit', function() {
-    var m = 'The master did not die after an error was throwed';
-    assert.ok(existMaster, m);
+    var m = 'The master did not die after an error was thrown';
+    assert.ok(masterExited, m);
     m = 'The workers did not die after an error in the master';
-    assert.ok(existWorker, m);
+    assert.ok(workersExited, m);
   });
 
 }
diff --git a/test/parallel/test-cluster-master-kill.js b/test/parallel/test-cluster-master-kill.js
index 32f22b0f968b7b..549655f1b64cbc 100644
--- a/test/parallel/test-cluster-master-kill.js
+++ b/test/parallel/test-cluster-master-kill.js
@@ -55,26 +55,21 @@ if (cluster.isWorker) {
   var alive = true;
   master.on('exit', function(code) {
 
-    // make sure that the master died by purpose
+    // make sure that the master died on purpose
     assert.equal(code, 0);
 
     // check worker process status
-    var timeout = 200;
-    if (common.isAix) {
-      // AIX needs more time due to default exit performance
-      timeout = 1000;
-    }
-    setTimeout(function() {
+    var pollWorker = function() {
       alive = isAlive(pid);
-    }, timeout);
+      if (alive) {
+        setTimeout(pollWorker, 50);
+      }
+    };
+    // Loop indefinitely until worker exit.
+    pollWorker();
   });
 
   process.once('exit', function() {
-    // cleanup: kill the worker if alive
-    if (alive) {
-      process.kill(pid);
-    }
-
     assert.equal(typeof pid, 'number', 'did not get worker pid info');
     assert.equal(alive, false, 'worker was alive after master died');
   });