test: fix flaky test-net-socket-local-address

[cjihrig]

Prior to this commit, the test was flaky because it was executing the majority of its logic in a function called from the client and multiple events on the server. This commit simplifies the test by separating the server's connection and listening events, and isolating the client logic.

Refs: #4476 and #4644
R= @Trott

[jasnell]

LGTM

[Trott]

These changes mean that the test no longer triggers on the bug it was written for. This test is for a bug that was present in Node.js 3.0.0 and this version of the test (with appropriate modifications to remove fat arrows, etc. which were not supported in Node 3.0.0) does not fire an assertion whereas the current version does.

For the record, here's the way it ended up after removing fat arrows, etc. so that it could run in Node 3.0.0:

'use strict';
// const common = require('../common');
const assert = require('assert');
const net = require('net');

// skip test in FreeBSD jails
// if (common.inFreeBSDJail) {
//   console.log('1..0 # Skipped: In a FreeBSD jail');
//   return;
// }

var conns = 0;
var clientLocalPorts = [];
var serverRemotePorts = [];

const server = net.createServer(function(socket) {
  serverRemotePorts.push(socket.remotePort);
  socket.end();
});

server.on('close', function() {
  assert.deepEqual(clientLocalPorts, serverRemotePorts,
                   'client and server should agree on the ports used');
  assert.strictEqual(2, conns);
});

server.listen(12346, '127.0.0.1', connect);

function connect() {
  if (conns === 2) {
    server.close();
    return;
  }

  const client = new net.Socket();

  conns++;
  client.once('close', connect);
  client.connect(12346, '127.0.0.1', function() {
    clientLocalPorts.push(client.localPort);
  });
}

[cjihrig]

Good catch. Simple fix, just have to move the const client = new net.Socket(); up higher so that the same socket is reused. I'll update the PR.

[cjihrig]

@Trott updated. Verified that moving the client socket triggers the original bug in v3.0.0.

[jbergstroem]

Lets try it in CI: https://ci.nodejs.org/job/node-test-commit/1709/

[cjihrig]

CI has some red, but none in this test. We should probably run a stress test too.

[Trott]

Stress test: https://ci.nodejs.org/job/node-stress-single-test/341/nodes=win-vs2015/console

[Trott]

Confirmed expected test failure in Node.js 3.0.0 and success in Node.js 3.3.0. So that's good.

LGTM if the stress test comes out clean.

[cjihrig]

Thanks for the review. Stress test came through green. Landed in 6cfd0b5.