test: fix test-inspector-port-zero-cluster

* re-implemented test to parse args instead of post binding (exit 12)
* saved failing case as known issue

PR-URL: nodejs#13373
Fixes: nodejs#13343
Reviewed-By: James M Snell <jasnell@gmail.com>

test/inspector/inspector.status

@@ -5,6 +5,5 @@ prefix inspector
 # sample-test                        : PASS,FLAKY
 
 [true] # This section applies to all platforms
-test-inspector-port-zero-cluster     : PASS,FLAKY
 
 [$system==win32]

test/inspector/test-inspector-port-zero-cluster.js

@@ -4,31 +4,48 @@ const common = require('../common');
 
 common.skipIfInspectorDisabled();
 
+// Assert that even when started with `--inspect=0` workers are assigned
+// consecutive (i.e. deterministically predictable) debug ports
+
 const assert = require('assert');
 const cluster = require('cluster');
 
-if (cluster.isMaster) {
-  const ports = [];
-  for (const worker of [cluster.fork(),
-                        cluster.fork(),
-                        cluster.fork()]) {
-    worker.on('message', common.mustCall((message) => {
-      ports.push(message.debugPort);
-      worker.kill();
+function serialFork() {
+  return new Promise((res) => {
+    const worker = cluster.fork();
+    worker.on('exit', common.mustCall((code, signal) => {
+      // code 0 is normal
+      // code 12 can happen if inspector could not bind because of a port clash
+      if (code !== 0 && code !== 12)
+        assert.fail(`code: ${code}, signal: ${signal}`);
+      const port = worker.process.spawnargs
+        .map((a) => (/=(?:.*:)?(\d{2,5})$/.exec(a) || [])[1])
+        .filter((p) => p)
+        .pop();
+      res(Number(port));
     }));
-    worker.send('debugPort');
-  }
-  process.on('exit', () => {
-    ports.sort();
-    assert.strictEqual(ports.length, 3);
-    assert(ports.every((port) => port > 0));
-    assert(ports.every((port) => port < 65536));
-    assert.strictEqual(ports[0] + 1, ports[1]);  // Ports should be consecutive.
-    assert.strictEqual(ports[1] + 1, ports[2]);
   });
+}
+
+if (cluster.isMaster) {
+  Promise.all([serialFork(), serialFork(), serialFork()])
+    .then(common.mustCall((ports) => {
+      ports.push(process.debugPort);
+      ports.sort();
+      // 4 = [master, worker1, worker2, worker3].length()
+      assert.strictEqual(ports.length, 4);
+      assert(ports.every((port) => port > 0));
+      assert(ports.every((port) => port < 65536));
+      // Ports should be consecutive.
+      assert.strictEqual(ports[0] + 1, ports[1]);
+      assert.strictEqual(ports[1] + 1, ports[2]);
+      assert.strictEqual(ports[2] + 1, ports[3]);
+    }))
+    .catch(
+      (err) => {
+        console.error(err);
+        process.exit(1);
+      });
 } else {
-  process.on('message', (message) => {
-    if (message === 'debugPort')
-      process.send({ debugPort: process.debugPort });
-  });
+  process.exit(0);
 }

test/known_issues/test-inspector-cluster-port-clash.js

@@ -0,0 +1,63 @@
+// Flags: --inspect=0
+'use strict';
+const common = require('../common');
+
+// With the current behavior of Node.js (at least as late as 8.1.0), this
+// test fails with the following error:
+// `AssertionError [ERR_ASSERTION]: worker 2 failed to bind port`
+// Ideally, there would be a way for the user to opt out of sequential port
+// assignment.
+//
+// Refs: https://github.com/nodejs/node/issues/13343
+
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const cluster = require('cluster');
+const net = require('net');
+
+const ports = [process.debugPort];
+const clashPort = process.debugPort + 2;
+function serialFork() {
+  return new Promise((res) => {
+    const worker = cluster.fork();
+    worker.on('error', (err) => assert.fail(err));
+    // no common.mustCall since 1 out of 3 should fail
+    worker.on('online', () => {
+      worker.on('message', common.mustCall((message) => {
+        ports.push(message.debugPort);
+      }));
+    });
+    worker.on('exit', common.mustCall((code, signal) => {
+      assert.strictEqual(signal, null);
+      // worker 2 should fail because of port clash with `server`
+      if (code === 12) {
+        return assert.fail(`worker ${worker.id} failed to bind port`);
+      }
+      assert.strictEqual(0, code);
+    }));
+    worker.on('disconnect', common.mustCall(res));
+  });
+}
+
+if (cluster.isMaster) {
+  cluster.on('online', common.mustCall((worker) => worker.send('dbgport'), 2));
+
+  // block one of the ports with a listening socket
+  const server = net.createServer();
+  server.listen(clashPort, common.localhostIPv4, common.mustCall(() => {
+    // try to fork 3 workers No.2 should fail
+    Promise.all([serialFork(), serialFork(), serialFork()])
+      .then(common.mustNotCall())
+      .catch((err) => console.error(err));
+  }));
+  server.unref();
+} else {
+  const sentinel = common.mustCall();
+  process.on('message', (message) => {
+    if (message !== 'dbgport') return;
+    process.send({ debugPort: process.debugPort });
+    sentinel();
+    process.disconnect();
+  });
+}