Skip to content

Commit 389f294

Browse files
jklepatchMylesBorins
authored andcommittedAug 16, 2017
test: make test-http(s)-set-timeout-server alike
Make test-http(s)-set-timeout-server tests more similar and resolve the following issues: * `test-http-set-timeout-server.js` had a `secReceived` check in `serverResponseTimeoutWithPipeline()` that was added to prevent flakiness, but this did not exist in the https counterpart. * `test-https-set-timeout-server.js` utilized `common.mustCall()`, `common.mustNotCall()`, etc., while the http counterpart utilized the old method of checking flags on exit of the process. Refs: #13588 PR-URL: #13625 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Alexey Orlenko <[email protected]>
1 parent 5e9b203 commit 389f294

File tree

2 files changed

+22
-55
lines changed

2 files changed

+22
-55
lines changed
 

‎test/parallel/test-http-set-timeout-server.js

+14-52
Original file line numberDiff line numberDiff line change
@@ -23,38 +23,28 @@ function run() {
2323
}
2424

2525
test(function serverTimeout(cb) {
26-
let caughtTimeout = false;
27-
process.on('exit', function() {
28-
assert(caughtTimeout);
29-
});
3026
const server = http.createServer(function(req, res) {
3127
// just do nothing, we should get a timeout event.
3228
});
3329
server.listen(common.mustCall(function() {
3430
http.get({ port: server.address().port }).on('error', common.noop);
3531
}));
36-
const s = server.setTimeout(50, function(socket) {
37-
caughtTimeout = true;
32+
const s = server.setTimeout(50, common.mustCall(function(socket) {
3833
socket.destroy();
3934
server.close();
4035
cb();
41-
});
36+
}));
4237
assert.ok(s instanceof http.Server);
4338
});
4439

4540
test(function serverRequestTimeout(cb) {
46-
let caughtTimeout = false;
47-
process.on('exit', function() {
48-
assert(caughtTimeout);
49-
});
5041
const server = http.createServer(function(req, res) {
5142
// just do nothing, we should get a timeout event.
52-
const s = req.setTimeout(50, function() {
53-
caughtTimeout = true;
54-
req.socket.destroy();
43+
const s = req.setTimeout(50, common.mustCall(function(socket) {
44+
socket.destroy();
5545
server.close();
5646
cb();
57-
});
47+
}));
5848
assert.ok(s instanceof http.IncomingMessage);
5949
});
6050
server.listen(common.mustCall(function() {
@@ -67,18 +57,13 @@ test(function serverRequestTimeout(cb) {
6757
});
6858

6959
test(function serverResponseTimeout(cb) {
70-
let caughtTimeout = false;
71-
process.on('exit', function() {
72-
assert(caughtTimeout);
73-
});
7460
const server = http.createServer(function(req, res) {
7561
// just do nothing, we should get a timeout event.
76-
const s = res.setTimeout(50, function() {
77-
caughtTimeout = true;
78-
res.socket.destroy();
62+
const s = res.setTimeout(50, common.mustCall(function(socket) {
63+
socket.destroy();
7964
server.close();
8065
cb();
81-
});
66+
}));
8267
assert.ok(s instanceof http.OutgoingMessage);
8368
});
8469
server.listen(common.mustCall(function() {
@@ -88,21 +73,11 @@ test(function serverResponseTimeout(cb) {
8873
});
8974

9075
test(function serverRequestNotTimeoutAfterEnd(cb) {
91-
let caughtTimeoutOnRequest = false;
92-
let caughtTimeoutOnResponse = false;
93-
process.on('exit', function() {
94-
assert(!caughtTimeoutOnRequest);
95-
assert(caughtTimeoutOnResponse);
96-
});
9776
const server = http.createServer(function(req, res) {
9877
// just do nothing, we should get a timeout event.
99-
const s = req.setTimeout(50, function(socket) {
100-
caughtTimeoutOnRequest = true;
101-
});
78+
const s = req.setTimeout(50, common.mustNotCall());
10279
assert.ok(s instanceof http.IncomingMessage);
103-
res.on('timeout', function(socket) {
104-
caughtTimeoutOnResponse = true;
105-
});
80+
res.on('timeout', common.mustCall());
10681
});
10782
server.on('timeout', function(socket) {
10883
socket.destroy();
@@ -148,29 +123,16 @@ test(function serverResponseTimeoutWithPipeline(cb) {
148123
});
149124

150125
test(function idleTimeout(cb) {
151-
let caughtTimeoutOnRequest = false;
152-
let caughtTimeoutOnResponse = false;
153-
let caughtTimeoutOnServer = false;
154-
process.on('exit', function() {
155-
assert(!caughtTimeoutOnRequest);
156-
assert(!caughtTimeoutOnResponse);
157-
assert(caughtTimeoutOnServer);
158-
});
159126
const server = http.createServer(function(req, res) {
160-
req.on('timeout', function(socket) {
161-
caughtTimeoutOnRequest = true;
162-
});
163-
res.on('timeout', function(socket) {
164-
caughtTimeoutOnResponse = true;
165-
});
127+
req.on('timeout', common.mustNotCall());
128+
res.on('timeout', common.mustNotCall());
166129
res.end();
167130
});
168-
const s = server.setTimeout(50, function(socket) {
169-
caughtTimeoutOnServer = true;
131+
const s = server.setTimeout(50, common.mustCall(function(socket) {
170132
socket.destroy();
171133
server.close();
172134
cb();
173-
});
135+
}));
174136
assert.ok(s instanceof http.Server);
175137
server.listen(common.mustCall(function() {
176138
const port = server.address().port;

‎test/sequential/test-https-set-timeout-server.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,24 @@ test(function serverRequestNotTimeoutAfterEnd(cb) {
116116

117117
test(function serverResponseTimeoutWithPipeline(cb) {
118118
let caughtTimeout = '';
119+
let secReceived = false;
119120
process.on('exit', function() {
120121
assert.strictEqual(caughtTimeout, '/2');
121122
});
122123
const server = https.createServer(serverOptions, function(req, res) {
124+
if (req.url === '/2')
125+
secReceived = true;
123126
res.setTimeout(50, function() {
124127
caughtTimeout += req.url;
125128
});
126129
if (req.url === '/1') res.end();
127130
});
128131
server.on('timeout', function(socket) {
129-
socket.destroy();
130-
server.close();
131-
cb();
132+
if (secReceived) {
133+
socket.destroy();
134+
server.close();
135+
cb();
136+
}
132137
});
133138
server.listen(0, function() {
134139
const options = {

0 commit comments

Comments
 (0)
Please sign in to comment.