From 079d054af8b73ffcafc3bd86ace919588ed94199 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Thu, 28 Mar 2019 11:35:18 -0700
Subject: [PATCH] test: simplify test-async-hooks-http-parser-destroy

Simplify test-async-hooks-http-parser-destroy and improve it's reporting
when failing. (Also makes it easier to run on older versions of Node.js
because it doesn't rely on internal test modules that won't work there.)

Before, failures looked like this (edited slightly to conform to our
commit message 72-char line length restriction):

    The expression evaluated to a falsy value:
    assert.ok(destroyedIds.indexOf(createdAsyncId) >= 0)

Now, you get a slightly better idea of what's up. (Is it missing one ID?
More than one? All of them?):

    Input A expected to strictly deep-equal input B:
    + expected - actual ... Lines skipped

      Set {
        1009,
    ...
        1025,
    -   158,
    -   159,
    -   161,
    -   162,
    -   164,
    -   165,
    -   167,
    -   168,
    -   170,
    ...
    +   159,
    +   162,
    +   165,
    +   168,
    +   171,
    +   174,
    +   177,
    +   180,
    +   183,

This test still fails as expected on 10.14.1 and passees on 10.14.2
(where the regression it tests for was fixed). (You will need to comment
on the `require('../common');` line first but that's now the only change
you need to make this run in older versions.)

Refs: https://github.com/nodejs/node/issues/26610
---
 .../test-async-hooks-http-parser-destroy.js   | 73 ++++++++++---------
 1 file changed, 40 insertions(+), 33 deletions(-)

diff --git a/test/parallel/test-async-hooks-http-parser-destroy.js b/test/parallel/test-async-hooks-http-parser-destroy.js
index d2e1071c280d66..794f53ff8c4a21 100644
--- a/test/parallel/test-async-hooks-http-parser-destroy.js
+++ b/test/parallel/test-async-hooks-http-parser-destroy.js
@@ -1,6 +1,5 @@
 'use strict';
-const common = require('../common');
-const Countdown = require('../common/countdown');
+require('../common');
 const assert = require('assert');
 const async_hooks = require('async_hooks');
 const http = require('http');
@@ -12,50 +11,58 @@ const http = require('http');
 const N = 50;
 const KEEP_ALIVE = 100;
 
-const createdIds = [];
-const destroyedIds = [];
+// XXX (trott): 'destroy' event can happen more than once for some asyncIds.
+// Using a Set for now to work around the bug, but it can be simplified perhaps
+// once https://github.com/nodejs/node/issues/26961 is fixed.
+const createdIds = new Set();
+const destroyedIds = new Set();
 async_hooks.createHook({
-  init: common.mustCallAtLeast((asyncId, type) => {
+  init: (asyncId, type) => {
     if (type === 'HTTPPARSER') {
-      createdIds.push(asyncId);
+      createdIds.add(asyncId);
     }
-  }, N),
+  },
   destroy: (asyncId) => {
-    destroyedIds.push(asyncId);
+    if (createdIds.has(asyncId)) {
+      destroyedIds.add(asyncId);
+      // Each HTTP request results in two createdIds and two destroyedIds.
+      if (destroyedIds.size === N * 2)
+        server.close();
+    }
   }
 }).enable();
 
-const server = http.createServer(function(req, res) {
-  res.end('Hello');
-});
+const server = http.createServer((req, res) => { res.end('Hello'); });
 
 const keepAliveAgent = new http.Agent({
   keepAlive: true,
   keepAliveMsecs: KEEP_ALIVE,
 });
 
-const countdown = new Countdown(N, () => {
-  server.close(() => {
-    // Give the server sockets time to close (which will also free their
-    // associated parser objects) after the server has been closed.
-    setTimeout(() => {
-      createdIds.forEach((createdAsyncId) => {
-        assert.ok(destroyedIds.indexOf(createdAsyncId) >= 0);
-      });
-    }, KEEP_ALIVE * 2);
-  });
-});
-
-server.listen(0, function() {
+server.listen(0, () => {
   for (let i = 0; i < N; ++i) {
-    (function makeRequest() {
-      http.get({
-        port: server.address().port,
-        agent: keepAliveAgent
-      }, function(res) {
-        countdown.dec();
-        res.resume();
-      });
-    })();
+    http.get(
+      { port: server.address().port, agent: keepAliveAgent },
+      (res) => { res.resume(); }
+    );
   }
 });
+
+function checkOnExit() {
+  assert.deepStrictEqual(destroyedIds, createdIds);
+  // Each HTTP request results in two createdIds and two destroyedIds.
+  assert.strictEqual(createdIds.size, N * 2);
+}
+
+// Ordinary exit.
+process.on('exit', checkOnExit);
+
+// Test runner tools/test.py will send SIGTERM if timeout.
+// Signals aren't received in workers, but that's OK. The test will still fail
+// if it times out. It just won't provide the additional information that this
+// handler does.
+process.on('SIGTERM', () => {
+  console.error('Received SIGTERM. (Timed out?)');
+  checkOnExit();
+  process.exit(1);
+});