Skip to content

Commit c77063e

Browse files
jasnellpanva
authored andcommitted
test: implement low-risk test optimizations
Reviewing the performance of http tests in the smartos CI runs revealed that a handful of HTTP related tests are running way longer than they should, with some hitting upwards of 66 seconds! - Ten HTTP/HTTPS tests now close connections deterministically instead of waiting 66 seconds. - Two HTTP/2 tests use an explicit 64KiB stream window and 3MB payload, while restoring timeout assertions. - Estimated SmartOS CI saving: about 4.5 minutes at -j4. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode PR-URL: #65926 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent ed6f3bc commit c77063e

12 files changed

Lines changed: 59 additions & 20 deletions

test/parallel/test-http-catch-uncaughtexception.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const server = http.createServer(function(req, res) {
1414
res.writeHead(200, { 'Content-Type': 'text/plain' });
1515
res.end('bye');
1616
}).listen(0, function() {
17-
http.get({ port: this.address().port }, function(res) {
17+
http.get({ port: this.address().port, agent: false }, function(res) {
1818
res.resume();
1919
throw new Error('get did fail');
2020
}).on('close', function() {

test/parallel/test-http-flush-headers.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
1616
port: this.address().port,
1717
});
1818
req.setHeader('foo', 'bar');
19+
req.setHeader('Connection', 'close');
1920
req.flushHeaders();
2021
}));

test/parallel/test-http-insecure-parser.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ const server = http.createServer(common.mustCallAtLeast((req, res) => {
1111
req.pipe(res);
1212
}));
1313

14+
// The malformed request intentionally has no valid Connection header.
15+
// So we have to set an explicitly shorter-than-default timeout.
16+
server.keepAliveTimeout = common.platformTimeout(100);
17+
server.keepAliveTimeoutBuffer = 0;
18+
1419
server.listen(0, common.mustCall(function() {
1520
const bufs = [];
1621
const client = net.connect(

test/parallel/test-http-keep-alive-drop-requests.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,20 @@ server.on('dropRequest', common.mustCall((request, socket) => {
2424

2525
server.listen(0, common.mustCall(() => {
2626
const socket = net.connect(server.address().port);
27+
let response = '';
2728
socket.on('connect', common.mustCall(() => {
2829
request(socket);
2930
request(socket);
3031
}));
31-
socket.on('data', common.mustCallAtLeast());
32-
socket.on('close', common.mustCall());
32+
socket.on('data', common.mustCallAtLeast((chunk) => {
33+
response += chunk;
34+
if (response.includes('HTTP/1.1 503 Service Unavailable'))
35+
socket.end();
36+
}));
37+
socket.on('close', common.mustCall(() => {
38+
assert.match(response, /HTTP\/1\.1 200 OK/);
39+
assert.match(response, /HTTP\/1\.1 503 Service Unavailable/);
40+
}));
3341
}));
3442

3543
server.maxRequestsPerSocket = 1;

test/parallel/test-http-pipeline-assertionerror-finish.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ const server = http
2727
.listen(0, function() {
2828
const s = net.connect(this.address().port);
2929

30-
const big = 'GET / HTTP/1.1\r\nHost: example.com\r\n\r\n'.repeat(COUNT);
30+
const big = 'GET / HTTP/1.1\r\nHost: example.com\r\n\r\n'.repeat(COUNT - 1) +
31+
'GET / HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n';
3132

3233
s.write(big);
3334
s.resume();

test/parallel/test-http-req-res-close.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ const assert = require('assert');
4040
}));
4141

4242
server.listen(0, common.mustCall(() => {
43-
http.get({ port: server.address().port }, common.mustCall());
43+
http.get({ port: server.address().port, agent: false }, common.mustCall());
4444
}));
4545
}
4646

@@ -81,7 +81,7 @@ const assert = require('assert');
8181
}));
8282

8383
server.listen(0, common.mustCall(() => {
84-
http.get({ port: server.address().port }, common.mustCall());
84+
http.get({ port: server.address().port, agent: false }, common.mustCall());
8585
}));
8686
}
8787

@@ -126,6 +126,6 @@ const assert = require('assert');
126126
}));
127127

128128
server.listen(0, common.mustCall(() => {
129-
http.get({ port: server.address().port }, common.mustCall());
129+
http.get({ port: server.address().port, agent: false }, common.mustCall());
130130
}));
131131
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,6 @@ test(function fastTimeout(cb) {
192192

193193
const server = http.createServer(common.mustCall((req, res) => {
194194
req.on('timeout', common.mustNotCall());
195-
res.end();
196195
connectionHandlerInvoked = true;
197196
invokeCallbackIfDone();
198197
}));

test/parallel/test-https-keep-alive-drop-requests.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,19 @@ server.listen(0, common.mustCall(() => {
3939
rejectUnauthorized: false
4040
},
4141
common.mustCall(() => {
42+
let response = '';
4243
request(socket);
4344
request(socket);
4445
socket.on('error', common.mustNotCall());
45-
socket.on('data', common.mustCallAtLeast());
46-
socket.on('close', common.mustCall());
46+
socket.on('data', common.mustCallAtLeast((chunk) => {
47+
response += chunk;
48+
if (response.includes('HTTP/1.1 503 Service Unavailable'))
49+
socket.end();
50+
}));
51+
socket.on('close', common.mustCall(() => {
52+
assert.match(response, /HTTP\/1\.1 200 OK/);
53+
assert.match(response, /HTTP\/1\.1 503 Service Unavailable/);
54+
}));
4755
})
4856
);
4957
}));

test/parallel/test-https-set-timeout-server.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ test(function fastTimeout(cb) {
216216
const server = https.createServer(serverOptions, common.mustCall(
217217
(req, res) => {
218218
req.on('timeout', common.mustNotCall());
219-
res.end();
220219
connectionHandlerInvoked = true;
221220
invokeCallbackIfDone();
222221
}

test/sequential/test-http2-timeout-large-write-file.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const common = require('../common');
33
if (!common.hasCrypto)
44
common.skip('missing crypto');
5+
const assert = require('assert');
56
const fixtures = require('../common/fixtures');
67
const fs = require('fs');
78
const http2 = require('http2');
@@ -21,10 +22,11 @@ tmpdir.refresh();
2122
// that the backing stream is still active and writing
2223
// 4) Our timer fires, we resume the socket and start at 1)
2324

24-
const writeSize = 33554432;
25+
const writeSize = 3000000;
2526
const minReadSize = 500000;
2627
const serverTimeout = common.platformTimeout(500);
2728
let offsetTimeout = common.platformTimeout(100);
29+
let didReceiveData = false;
2830
const content = Buffer.alloc(writeSize, 0x44);
2931
const filepath = tmpdir.resolve('http2-large-write.tmp');
3032
fs.writeFileSync(filepath, content, 'binary');
@@ -44,11 +46,15 @@ server.on('stream', common.mustCall((stream) => {
4446
stream.end();
4547
}));
4648
server.setTimeout(serverTimeout);
47-
server.on('timeout', common.mustCallAtLeast(0));
49+
server.on('timeout', common.mustCallAtLeast(() => {
50+
assert.ok(!didReceiveData, 'Should not timeout');
51+
}, 0));
4852

4953
server.listen(0, common.mustCall(() => {
50-
const client = http2.connect(`https://localhost:${server.address().port}`,
51-
{ rejectUnauthorized: false });
54+
const client = http2.connect(`https://localhost:${server.address().port}`, {
55+
rejectUnauthorized: false,
56+
settings: { initialWindowSize: 65535 },
57+
});
5258

5359
const req = client.request({ ':path': '/' });
5460
req.end();
@@ -58,11 +64,13 @@ server.listen(0, common.mustCall(() => {
5864
let firstReceivedAt;
5965
req.on('data', common.mustCallAtLeast((buf) => {
6066
if (receivedBufferLength === 0) {
67+
didReceiveData = false;
6168
firstReceivedAt = Date.now();
6269
}
6370
receivedBufferLength += buf.length;
6471
if (receivedBufferLength >= minReadSize &&
6572
receivedBufferLength < writeSize) {
73+
didReceiveData = true;
6674
receivedBufferLength = 0;
6775
req.pause();
6876
setTimeout(

0 commit comments

Comments
 (0)