From 7446fa402bc426a87a99d63fa62d84a42bdb2e1f Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sun, 17 Jan 2016 23:09:26 -0800
Subject: [PATCH 1/2] test: improve test-cluster-disconnect-suicide-race

Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

Ref: #4674

cc @santigimeno @iWuzHere
---
 .../test-cluster-disconnect-suicide-race.js   | 46 +++++++++----------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/test/sequential/test-cluster-disconnect-suicide-race.js b/test/sequential/test-cluster-disconnect-suicide-race.js
index e05c420e1fdd9b..29d5707f9087b6 100644
--- a/test/sequential/test-cluster-disconnect-suicide-race.js
+++ b/test/sequential/test-cluster-disconnect-suicide-race.js
@@ -1,32 +1,28 @@
 'use strict';
+
+// Test should fail in Node.js 5.4.1 and pass in later versions.
+
 const common = require('../common');
 const assert = require('assert');
 const cluster = require('cluster');
-const os = require('os');
 
 if (cluster.isMaster) {
-  function forkWorker(action) {
-    const worker = cluster.fork({ action });
-    worker.on('disconnect', common.mustCall(() => {
-      assert.strictEqual(worker.suicide, true);
-    }));
-
-    worker.on('exit', common.mustCall(() => {
-      assert.strictEqual(worker.suicide, true);
-    }));
-  }
-
-  const cpus = os.cpus().length;
-  const tries = cpus > 8 ? 64 : cpus * 8;
-
-  cluster.on('exit', common.mustCall((worker, code) => {
-    assert.strictEqual(code, 0, 'worker exited with error');
-  }, tries * 2));
-
-  for (let i = 0; i < tries; ++i) {
-    forkWorker('disconnect');
-    forkWorker('kill');
-  }
-} else {
-  cluster.worker[process.env.action]();
+  cluster.on('exit', (worker, code) => {
+    if (code)
+      common.fail('worker exited with error');
+  });
+
+  return cluster.fork();
 }
+
+var eventFired = false;
+
+cluster.worker.disconnect();
+
+process.nextTick(common.mustCall(() => {
+  assert.strictEqual(eventFired, false, 'disconnect event should wait for ack');
+}));
+
+cluster.worker.on('disconnect', common.mustCall(() => {
+  eventFired = true;
+}));

From 898a22685620c8b8bb3097e6778aac90f43cb5fb Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Mon, 18 Jan 2016 07:53:46 -0800
Subject: [PATCH 2/2] fixup: improve assert

---
 test/sequential/test-cluster-disconnect-suicide-race.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test/sequential/test-cluster-disconnect-suicide-race.js b/test/sequential/test-cluster-disconnect-suicide-race.js
index 29d5707f9087b6..6f23b23fd2d4ff 100644
--- a/test/sequential/test-cluster-disconnect-suicide-race.js
+++ b/test/sequential/test-cluster-disconnect-suicide-race.js
@@ -8,8 +8,7 @@ const cluster = require('cluster');
 
 if (cluster.isMaster) {
   cluster.on('exit', (worker, code) => {
-    if (code)
-      common.fail('worker exited with error');
+    assert.strictEqual(code, 0, 'worker exited with error');
   });
 
   return cluster.fork();