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: #6531
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>

Author: stefanmb

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);
   });
 
 }

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');
   });