test: refactor and deflake test-tls-sni-server-client

- Run all tests in parallel. - Move `socket.end()` call to client. - Use `common.mustCall()` and `common.mustNotCall()`. Fixes: #27219 Refs: #27300 PR-URL: #27426 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · May 2, 2019 · bc0a330 · bc0a330
1 parent e5c8be2
commit bc0a330
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 69 deletions.
diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
@@ -23,8 +23,6 @@ test-http2-client-upload-reject: PASS,FLAKY
 [$system==linux]
 
 [$system==macos]
-# https://github.com/nodejs/node/issues/27219
-test-tls-sni-server-client: PASS,FLAKY
 # https://github.com/nodejs/node/issues/26938
 test-tls-js-stream: PASS,FLAKY
 

diff --git a/test/parallel/test-tls-sni-server-client.js b/test/parallel/test-tls-sni-server-client.js
@@ -54,73 +54,77 @@ const SNIContexts = {
   }
 };
 
-const clientsOptions = [{
-  port: undefined,
-  ca: [loadPEM('ca1-cert')],
-  servername: 'a.example.com',
-  rejectUnauthorized: false
-}, {
-  port: undefined,
-  ca: [loadPEM('ca2-cert')],
-  servername: 'b.test.com',
-  rejectUnauthorized: false
-}, {
-  port: undefined,
-  ca: [loadPEM('ca2-cert')],
-  servername: 'a.b.test.com',
-  rejectUnauthorized: false
-}, {
-  port: undefined,
-  ca: [loadPEM('ca1-cert')],
-  servername: 'c.wrong.com',
-  rejectUnauthorized: false
-}, {
-  port: undefined,
-  ca: [loadPEM('ca1-cert')],
-  servername: 'chain.example.com',
-  rejectUnauthorized: false
-}];
-
-const serverResults = [];
-const clientResults = [];
-
-const server = tls.createServer(serverOptions, function(c) {
-  serverResults.push(c.servername);
-  c.end();
-});
-
-server.addContext('a.example.com', SNIContexts['a.example.com']);
-server.addContext('*.test.com', SNIContexts['asterisk.test.com']);
-server.addContext('chain.example.com', SNIContexts['chain.example.com']);
-
-server.listen(0, startTest);
-
-function startTest() {
-  let i = 0;
-  function start() {
-    // No options left
-    if (i === clientsOptions.length)
-      return server.close();
-
-    const options = clientsOptions[i++];
-    options.port = server.address().port;
-    const client = tls.connect(options, function() {
-      clientResults.push(
-        client.authorizationError &&
-        (client.authorizationError === 'ERR_TLS_CERT_ALTNAME_INVALID'));
-
-      // Continue
-      start();
+test(
+  {
+    ca: [loadPEM('ca1-cert')],
+    servername: 'a.example.com'
+  },
+  true,
+  'a.example.com'
+);
+
+test(
+  {
+    ca: [loadPEM('ca2-cert')],
+    servername: 'b.test.com',
+  },
+  true,
+  'b.test.com'
+);
+
+test(
+  {
+    ca: [loadPEM('ca2-cert')],
+    servername: 'a.b.test.com',
+  },
+  false,
+  'a.b.test.com'
+);
+
+test(
+  {
+    ca: [loadPEM('ca1-cert')],
+    servername: 'c.wrong.com',
+  },
+  false,
+  'c.wrong.com'
+);
+
+test(
+  {
+    ca: [loadPEM('ca1-cert')],
+    servername: 'chain.example.com',
+  },
+  true,
+  'chain.example.com'
+);
+
+function test(options, clientResult, serverResult) {
+  const server = tls.createServer(serverOptions, (c) => {
+    assert.strictEqual(c.servername, serverResult);
+    assert.strictEqual(c.authorized, false);
+  });
+
+  server.addContext('a.example.com', SNIContexts['a.example.com']);
+  server.addContext('*.test.com', SNIContexts['asterisk.test.com']);
+  server.addContext('chain.example.com', SNIContexts['chain.example.com']);
+
+  server.on('tlsClientError', common.mustNotCall());
+
+  server.listen(0, () => {
+    const client = tls.connect({
+      ...options,
+      port: server.address().port,
+      rejectUnauthorized: false
+    }, () => {
+      const result = client.authorizationError &&
+        (client.authorizationError === 'ERR_TLS_CERT_ALTNAME_INVALID');
+      assert.strictEqual(result, clientResult);
+      client.end();
     });
-  }
 
-  start();
+    client.on('close', common.mustCall(() => {
+      server.close();
+    }));
+  });
 }
-
-process.on('exit', function() {
-  assert.deepStrictEqual(serverResults, [
-    'a.example.com', 'b.test.com', 'a.b.test.com', 'c.wrong.com',
-    'chain.example.com'
-  ]);
-  assert.deepStrictEqual(clientResults, [true, true, false, false, true]);
-});