Skip to content

Commit 06d8af9

Browse files
KernelDeimosKernelDeimos
committed
test: use ephemeral ports to avoid race conditions
Tests prior to this change use a mixture of hard-coded ports and ports found using `portfinder`. Hard-coded ports cause tests to fail depending on the conditions of the host environment they are running on. Ports obtained using portfinder cause stochastic port collisions because tap runs tests in parallel; two tests might find the same open port and both try to listen on that port simultaneously. This change updates several tests to use ephemeral. This is accomplished by specifying the port as `0` in the call to `.listen()`: > If port is omitted or is 0, the operating system will assign an > arbitrary unused port, which can be retrieved by using > server.address().port after the 'listening' event has been > emitted. source: https://nodejs.org/api/net.html#net_server_listen Since the http server is `union` with a wrapper class, `lib/http-server.js` is updated to inlcude the new method `address()` which allows access to the ephemeral port number that was selected.
1 parent 661d341 commit 06d8af9

File tree

8 files changed

+337
-336
lines changed

8 files changed

+337
-336
lines changed

‎lib/http-server.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,3 +212,7 @@ HttpServer.prototype.listen = function () {
212212
HttpServer.prototype.close = function () {
213213
return this.server.close();
214214
};
215+
216+
HttpServer.prototype.address = function () {
217+
return this.server.address();
218+
};

‎test/304.test.js‎

Lines changed: 161 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -11,161 +11,159 @@ const root = `${__dirname}/public`;
1111
const baseDir = 'base';
1212

1313
test('304_not_modified_strong', (t) => {
14-
portfinder.getPort((err, port) => {
15-
const file = 'a.txt';
16-
17-
const server = http.createServer(
18-
ecstatic({
19-
root,
20-
gzip: true,
21-
baseDir,
22-
autoIndex: true,
23-
showDir: true,
24-
weakEtags: false,
25-
weakCompare: false,
26-
})
27-
);
28-
29-
server.listen(port, () => {
30-
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
14+
const file = 'a.txt';
15+
16+
const server = http.createServer(
17+
ecstatic({
18+
root,
19+
gzip: true,
20+
baseDir,
21+
autoIndex: true,
22+
showDir: true,
23+
weakEtags: false,
24+
weakCompare: false,
25+
})
26+
);
27+
28+
server.listen(0, () => {
29+
const port = server.address().port;
30+
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
31+
32+
request.get({
33+
uri,
34+
followRedirect: false,
35+
}, (err, res) => {
36+
if (err) {
37+
t.fail(err);
38+
}
39+
40+
t.equal(res.statusCode, 200, 'first request should be a 200');
3141

3242
request.get({
3343
uri,
3444
followRedirect: false,
35-
}, (err, res) => {
36-
if (err) {
37-
t.fail(err);
45+
headers: { 'if-modified-since': res.headers['last-modified'] },
46+
}, (err2, res2) => {
47+
if (err2) {
48+
t.fail(err2);
3849
}
3950

40-
t.equal(res.statusCode, 200, 'first request should be a 200');
41-
42-
request.get({
43-
uri,
44-
followRedirect: false,
45-
headers: { 'if-modified-since': res.headers['last-modified'] },
46-
}, (err2, res2) => {
47-
if (err2) {
48-
t.fail(err2);
49-
}
50-
51-
t.equal(res2.statusCode, 304, 'second request should be a 304');
52-
t.equal(res2.headers.etag.indexOf('"'), 0, 'should return a strong etag');
53-
server.close();
54-
setTimeout(() => { t.end(); }, 0);
55-
});
51+
t.equal(res2.statusCode, 304, 'second request should be a 304');
52+
t.equal(res2.headers.etag.indexOf('"'), 0, 'should return a strong etag');
53+
server.close();
54+
setTimeout(() => { t.end(); }, 0);
5655
});
5756
});
5857
});
5958
});
6059

6160
test('304_not_modified_weak', (t) => {
62-
portfinder.getPort((err, port) => {
63-
const file = 'b.txt';
64-
65-
const server = http.createServer(
66-
ecstatic({
67-
root,
68-
gzip: true,
69-
baseDir,
70-
autoIndex: true,
71-
showDir: true,
72-
weakCompare: false,
73-
})
74-
);
75-
76-
server.listen(port, () => {
77-
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
78-
const now = (new Date()).toString();
61+
const file = 'b.txt';
62+
63+
const server = http.createServer(
64+
ecstatic({
65+
root,
66+
gzip: true,
67+
baseDir,
68+
autoIndex: true,
69+
showDir: true,
70+
weakEtags: true,
71+
weakCompare: false,
72+
})
73+
);
74+
75+
server.listen(0, () => {
76+
const port = server.address().port;
77+
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
78+
const now = (new Date()).toString();
79+
80+
request.get({
81+
uri,
82+
followRedirect: false,
83+
}, (err, res) => {
84+
if (err) {
85+
t.fail(err);
86+
}
87+
88+
t.equal(res.statusCode, 200, 'first request should be a 200');
7989

8090
request.get({
8191
uri,
8292
followRedirect: false,
83-
}, (err, res) => {
84-
if (err) {
85-
t.fail(err);
86-
}
87-
88-
t.equal(res.statusCode, 200, 'first request should be a 200');
89-
90-
request.get({
91-
uri,
92-
followRedirect: false,
93-
headers: { 'if-modified-since': now },
94-
}, (err2, res2) => {
95-
if (err2) t.fail(err2);
96-
97-
t.equal(res2.statusCode, 304, 'second request should be a 304');
98-
t.equal(res2.headers.etag.indexOf('W/'), 0, 'should return a weak etag');
99-
server.close();
100-
setTimeout(() => { t.end(); }, 0);
101-
});
93+
headers: { 'if-modified-since': now },
94+
}, (err2, res2) => {
95+
if (err2) t.fail(err2);
96+
97+
t.equal(res2.statusCode, 304, 'second request should be a 304');
98+
t.equal(res2.headers.etag.indexOf('W/'), 0, 'should return a weak etag');
99+
server.close();
100+
setTimeout(() => { t.end(); }, 0);
102101
});
103102
});
104103
});
105104
});
106105

107106
test('304_not_modified_strong_compare', (t) => {
108-
portfinder.getPort((err, port) => {
109-
const file = 'b.txt';
110-
111-
const server = http.createServer(
112-
ecstatic({
113-
root,
114-
gzip: true,
115-
baseDir,
116-
autoIndex: true,
117-
showDir: true,
118-
weakEtags: false,
119-
weakCompare: false,
120-
})
121-
);
122-
123-
server.listen(port, () => {
124-
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
125-
const now = (new Date()).toString();
126-
let etag = null;
107+
const file = 'b.txt';
108+
109+
const server = http.createServer(
110+
ecstatic({
111+
root,
112+
gzip: true,
113+
baseDir,
114+
autoIndex: true,
115+
showDir: true,
116+
weakEtags: false,
117+
weakCompare: false,
118+
})
119+
);
120+
121+
server.listen(0, () => {
122+
const port = server.address().port;
123+
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
124+
const now = (new Date()).toString();
125+
let etag = null;
126+
127+
request.get({
128+
uri,
129+
followRedirect: false,
130+
}, (err, res) => {
131+
if (err) {
132+
t.fail(err);
133+
}
134+
135+
t.equal(res.statusCode, 200, 'first request should be a 200');
136+
137+
etag = res.headers.etag;
127138

128139
request.get({
129140
uri,
130141
followRedirect: false,
131-
}, (err, res) => {
132-
if (err) {
133-
t.fail(err);
142+
headers: { 'if-modified-since': now, 'if-none-match': etag },
143+
}, (err2, res2) => {
144+
if (err2) {
145+
t.fail(err2);
134146
}
135147

136-
t.equal(res.statusCode, 200, 'first request should be a 200');
137-
138-
etag = res.headers.etag;
148+
t.equal(res2.statusCode, 304, 'second request with a strong etag should be 304');
139149

140150
request.get({
141151
uri,
142152
followRedirect: false,
143-
headers: { 'if-modified-since': now, 'if-none-match': etag },
144-
}, (err2, res2) => {
145-
if (err2) {
146-
t.fail(err2);
153+
headers: { 'if-modified-since': now, 'if-none-match': `W/${etag}` },
154+
}, (err3, res3) => {
155+
if (err3) {
156+
t.fail(err3);
147157
}
148158

149-
t.equal(res2.statusCode, 304, 'second request with a strong etag should be 304');
150-
151-
request.get({
152-
uri,
153-
followRedirect: false,
154-
headers: { 'if-modified-since': now, 'if-none-match': `W/${etag}` },
155-
}, (err3, res3) => {
156-
if (err3) {
157-
t.fail(err3);
158-
}
159-
160-
// Note that if both if-modified-since and if-none-match are
161-
// provided, the server MUST NOT return a response status of 304
162-
// unless doing so is consistent with all of the conditional
163-
// header fields in the request
164-
// https://www.ietf.org/rfc/rfc2616.txt
165-
t.equal(res3.statusCode, 200, 'third request with a weak etag should be 200');
166-
server.close();
167-
setTimeout(() => { t.end(); }, 0);
168-
});
159+
// Note that if both if-modified-since and if-none-match are
160+
// provided, the server MUST NOT return a response status of 304
161+
// unless doing so is consistent with all of the conditional
162+
// header fields in the request
163+
// https://www.ietf.org/rfc/rfc2616.txt
164+
t.equal(res3.statusCode, 200, 'third request with a weak etag should be 200');
165+
server.close();
166+
setTimeout(() => { t.end(); }, 0);
169167
});
170168
});
171169
});
@@ -174,61 +172,60 @@ test('304_not_modified_strong_compare', (t) => {
174172

175173

176174
test('304_not_modified_weak_compare', (t) => {
177-
portfinder.getPort((err, port) => {
178-
const file = 'c.js';
179-
180-
const server = http.createServer(
181-
ecstatic({
182-
root,
183-
gzip: true,
184-
baseDir,
185-
autoIndex: true,
186-
showDir: true,
187-
weakEtags: false,
188-
})
189-
);
190-
191-
server.listen(port, () => {
192-
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
193-
const now = (new Date()).toString();
194-
let etag = null;
175+
const file = 'c.js';
176+
177+
const server = http.createServer(
178+
ecstatic({
179+
root,
180+
gzip: true,
181+
baseDir,
182+
autoIndex: true,
183+
showDir: true,
184+
weakEtags: false,
185+
})
186+
);
187+
188+
server.listen(0, () => {
189+
const port = server.address().port;
190+
const uri = `http://localhost:${port}${path.join('/', baseDir, file)}`;
191+
const now = (new Date()).toString();
192+
let etag = null;
193+
194+
request.get({
195+
uri,
196+
followRedirect: false,
197+
}, (err, res) => {
198+
if (err) {
199+
t.fail(err);
200+
}
201+
202+
t.equal(res.statusCode, 200, 'first request should be a 200');
203+
204+
etag = res.headers.etag;
195205

196206
request.get({
197207
uri,
198208
followRedirect: false,
199-
}, (err, res) => {
200-
if (err) {
201-
t.fail(err);
209+
headers: { 'if-modified-since': now, 'if-none-match': etag },
210+
}, (err2, res2) => {
211+
if (err2) {
212+
t.fail(err2);
202213
}
203214

204-
t.equal(res.statusCode, 200, 'first request should be a 200');
205-
206-
etag = res.headers.etag;
215+
t.equal(res2.statusCode, 304, 'second request with a strong etag should be 304');
207216

208217
request.get({
209218
uri,
210219
followRedirect: false,
211-
headers: { 'if-modified-since': now, 'if-none-match': etag },
212-
}, (err2, res2) => {
213-
if (err2) {
214-
t.fail(err2);
220+
headers: { 'if-modified-since': now, 'if-none-match': `W/${etag}` },
221+
}, (err3, res3) => {
222+
if (err3) {
223+
t.fail(err3);
215224
}
216225

217-
t.equal(res2.statusCode, 304, 'second request with a strong etag should be 304');
218-
219-
request.get({
220-
uri,
221-
followRedirect: false,
222-
headers: { 'if-modified-since': now, 'if-none-match': `W/${etag}` },
223-
}, (err3, res3) => {
224-
if (err3) {
225-
t.fail(err3);
226-
}
227-
228-
t.equal(res3.statusCode, 304, 'third request with a weak etag should be 304');
229-
server.close();
230-
setTimeout(() => { t.end(); }, 0);
231-
});
226+
t.equal(res3.statusCode, 304, 'third request with a weak etag should be 304');
227+
server.close();
228+
setTimeout(() => { t.end(); }, 0);
232229
});
233230
});
234231
});

0 commit comments

Comments
 (0)