From 01780011636330bfcd3e326c8c582beb592c7e69 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sun, 17 Jan 2016 19:50:09 -0800
Subject: [PATCH] test: make test-cluster-disconnect-leak reliable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows.
* The test should be reliable on any new platform regardless of CPU and
RAM.

Ref: https://github.com/nodejs/node/pull/4674

PR-URL: https://github.com/nodejs/node/pull/4736
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
---
 .../test-cluster-disconnect-leak.js           | 46 ++++++-------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/test/sequential/test-cluster-disconnect-leak.js b/test/sequential/test-cluster-disconnect-leak.js
index 33476dd427a61a..a4916c95ba4ea7 100644
--- a/test/sequential/test-cluster-disconnect-leak.js
+++ b/test/sequential/test-cluster-disconnect-leak.js
@@ -1,47 +1,29 @@
 'use strict';
-// Flags: --expose-internals
+
+// Test fails in Node v5.4.0 and passes in v5.4.1 and newer.
 
 const common = require('../common');
-const assert = require('assert');
 const net = require('net');
 const cluster = require('cluster');
-const handles = require('internal/cluster').handles;
-const os = require('os');
 
-if (common.isWindows) {
-  console.log('1..0 # Skipped: This test does not apply to Windows.');
-  return;
-}
+const noop = () => {};
 
 cluster.schedulingPolicy = cluster.SCHED_NONE;
 
 if (cluster.isMaster) {
-  const cpus = os.cpus().length;
-  const tries = cpus > 8 ? 128 : cpus * 16;
-
-  const worker1 = cluster.fork();
-  worker1.on('message', common.mustCall(() => {
-    worker1.disconnect();
-    for (let i = 0; i < tries; ++ i) {
-      const w = cluster.fork();
-      w.on('online', common.mustCall(w.disconnect));
-    }
-  }));
-
-  cluster.on('exit', common.mustCall((worker, code) => {
-    assert.strictEqual(code, 0, 'worker exited with error');
-  }, tries + 1));
-
-  process.on('exit', () => {
-    assert.deepEqual(Object.keys(cluster.workers), []);
-    assert.strictEqual(Object.keys(handles).length, 0);
-  });
+  const worker = cluster.fork();
+
+  // This is the important part of the test: Confirm that `disconnect` fires.
+  worker.on('disconnect', common.mustCall(noop));
+
+  // These are just some extra stuff we're checking for good measure...
+  worker.on('exit', common.mustCall(noop));
+  cluster.on('exit', common.mustCall(noop));
 
+  cluster.disconnect();
   return;
 }
 
-var server = net.createServer();
+const server = net.createServer();
 
-server.listen(common.PORT, function() {
-  process.send('listening');
-});
+server.listen(common.PORT);