Skip to content

Commit 079d054

Browse files
committed
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: #26610
1 parent 2e4ceb5 commit 079d054

File tree

1 file changed

+40
-33
lines changed

1 file changed

+40
-33
lines changed
Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict';
2-
const common = require('../common');
3-
const Countdown = require('../common/countdown');
2+
require('../common');
43
const assert = require('assert');
54
const async_hooks = require('async_hooks');
65
const http = require('http');
@@ -12,50 +11,58 @@ const http = require('http');
1211
const N = 50;
1312
const KEEP_ALIVE = 100;
1413

15-
const createdIds = [];
16-
const destroyedIds = [];
14+
// XXX (trott): 'destroy' event can happen more than once for some asyncIds.
15+
// Using a Set for now to work around the bug, but it can be simplified perhaps
16+
// once https://github.com/nodejs/node/issues/26961 is fixed.
17+
const createdIds = new Set();
18+
const destroyedIds = new Set();
1719
async_hooks.createHook({
18-
init: common.mustCallAtLeast((asyncId, type) => {
20+
init: (asyncId, type) => {
1921
if (type === 'HTTPPARSER') {
20-
createdIds.push(asyncId);
22+
createdIds.add(asyncId);
2123
}
22-
}, N),
24+
},
2325
destroy: (asyncId) => {
24-
destroyedIds.push(asyncId);
26+
if (createdIds.has(asyncId)) {
27+
destroyedIds.add(asyncId);
28+
// Each HTTP request results in two createdIds and two destroyedIds.
29+
if (destroyedIds.size === N * 2)
30+
server.close();
31+
}
2532
}
2633
}).enable();
2734

28-
const server = http.createServer(function(req, res) {
29-
res.end('Hello');
30-
});
35+
const server = http.createServer((req, res) => { res.end('Hello'); });
3136

3237
const keepAliveAgent = new http.Agent({
3338
keepAlive: true,
3439
keepAliveMsecs: KEEP_ALIVE,
3540
});
3641

37-
const countdown = new Countdown(N, () => {
38-
server.close(() => {
39-
// Give the server sockets time to close (which will also free their
40-
// associated parser objects) after the server has been closed.
41-
setTimeout(() => {
42-
createdIds.forEach((createdAsyncId) => {
43-
assert.ok(destroyedIds.indexOf(createdAsyncId) >= 0);
44-
});
45-
}, KEEP_ALIVE * 2);
46-
});
47-
});
48-
49-
server.listen(0, function() {
42+
server.listen(0, () => {
5043
for (let i = 0; i < N; ++i) {
51-
(function makeRequest() {
52-
http.get({
53-
port: server.address().port,
54-
agent: keepAliveAgent
55-
}, function(res) {
56-
countdown.dec();
57-
res.resume();
58-
});
59-
})();
44+
http.get(
45+
{ port: server.address().port, agent: keepAliveAgent },
46+
(res) => { res.resume(); }
47+
);
6048
}
6149
});
50+
51+
function checkOnExit() {
52+
assert.deepStrictEqual(destroyedIds, createdIds);
53+
// Each HTTP request results in two createdIds and two destroyedIds.
54+
assert.strictEqual(createdIds.size, N * 2);
55+
}
56+
57+
// Ordinary exit.
58+
process.on('exit', checkOnExit);
59+
60+
// Test runner tools/test.py will send SIGTERM if timeout.
61+
// Signals aren't received in workers, but that's OK. The test will still fail
62+
// if it times out. It just won't provide the additional information that this
63+
// handler does.
64+
process.on('SIGTERM', () => {
65+
console.error('Received SIGTERM. (Timed out?)');
66+
checkOnExit();
67+
process.exit(1);
68+
});

0 commit comments

Comments
 (0)