Skip to content

Commit 7e99521

Browse files
devinivydevinivy
authored
Fix tests for citgm (hapijs#4221)
* Fix missing lsof on node v14 for citgm tests * Attempt to fix flaky tests on macos in CI. Should also address AIX for citgm. * Skip tests without lsof rather than allow them to pass * Tighten-up flaky tests for citgm based on review * More test tightening from review of fixes for citgm.
1 parent f3e0ed1 commit 7e99521

File tree

4 files changed

+93
-69
lines changed

4 files changed

+93
-69
lines changed

‎test/common.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
const ChildProcess = require('child_process');
4+
5+
const internals = {};
6+
7+
internals.hasLsof = () => {
8+
9+
try {
10+
ChildProcess.execSync(`lsof -p ${process.pid}`, { stdio: 'ignore' });
11+
}
12+
catch (err) {
13+
return false;
14+
}
15+
16+
return true;
17+
};
18+
19+
exports.hasLsof = internals.hasLsof();

‎test/core.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ const Stream = require('stream');
1111
const TLS = require('tls');
1212

1313
const Boom = require('@hapi/boom');
14-
const Bounce = require('@hapi/bounce');
1514
const CatboxMemory = require('@hapi/catbox-memory');
1615
const Code = require('@hapi/code');
1716
const Handlebars = require('handlebars');
@@ -22,6 +21,7 @@ const Lab = require('@hapi/lab');
2221
const Vision = require('@hapi/vision');
2322
const Wreck = require('@hapi/wreck');
2423

24+
const Common = require('./common');
2525

2626
const internals = {};
2727

@@ -1732,7 +1732,7 @@ describe('Core', () => {
17321732
expect(res.result.isBoom).to.equal(true);
17331733
});
17341734

1735-
it('cleans unused file stream when response is overridden', { skip: process.platform === 'win32' }, async () => {
1735+
it('cleans unused file stream when response is overridden', { skip: !Common.hasLsof }, async () => {
17361736

17371737
const server = Hapi.server();
17381738
await server.register(Inert);
@@ -1755,12 +1755,6 @@ describe('Core', () => {
17551755
const cmd = ChildProcess.spawn('lsof', ['-p', process.pid]);
17561756
let lsof = '';
17571757

1758-
cmd.on('error', (err) => {
1759-
1760-
// Allow the test to pass on platforms with no lsof
1761-
Bounce.ignore(err, { errno: 'ENOENT' });
1762-
});
1763-
17641758
cmd.stdout.on('data', (buffer) => {
17651759

17661760
lsof += buffer.toString();

‎test/request.js

Lines changed: 69 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const Http = require('http');
44
const Net = require('net');
55
const Stream = require('stream');
66
const Url = require('url');
7+
const Events = require('events');
78

89
const Boom = require('@hapi/boom');
910
const Code = require('@hapi/code');
@@ -275,45 +276,57 @@ describe('Request', () => {
275276

276277
describe('active()', () => {
277278

278-
it('exits handler early when request is no longer active', async () => {
279+
it('exits handler early when request is no longer active', { retry: true }, async (flags) => {
280+
281+
let testComplete = false;
282+
283+
const onCleanup = [];
284+
flags.onCleanup = async () => {
285+
286+
testComplete = true;
287+
288+
for (const cleanup of onCleanup) {
289+
await cleanup();
290+
}
291+
};
279292

280293
const server = Hapi.server();
281-
const team = new Teamwork.Team();
282-
const team2 = new Teamwork.Team();
294+
const leaveHandlerTeam = new Teamwork.Team();
283295

284-
let rounds = 0;
285296
server.route({
286297
method: 'GET',
287298
path: '/',
288299
options: {
289300
handler: async (request, h) => {
290301

291-
team2.attend();
292-
for (let i = 0; i < 100; ++i) {
293-
++rounds;
294-
await Hoek.wait(10);
302+
req.abort();
295303

296-
if (!request.active()) {
297-
break;
298-
}
304+
while (request.active() && !testComplete) {
305+
await Hoek.wait(10);
299306
}
300307

301-
team.attend();
308+
leaveHandlerTeam.attend({
309+
active: request.active(),
310+
testComplete
311+
});
312+
302313
return null;
303314
}
304315
}
305316
});
306317

307318
await server.start();
319+
onCleanup.unshift(() => server.stop());
308320

309-
const req = Http.get(server.info.uri, (res) => { });
321+
const req = Http.get(server.info.uri, Hoek.ignore);
310322
req.on('error', Hoek.ignore);
311-
await team2.work;
312-
req.abort();
313-
await server.stop();
314323

315-
await team.work;
316-
expect(rounds).to.be.below(10);
324+
const note = await leaveHandlerTeam.work;
325+
326+
expect(note).to.equal({
327+
active: false,
328+
testComplete: false
329+
});
317330
});
318331
});
319332

@@ -847,10 +860,10 @@ describe('Request', () => {
847860

848861
describe('_postCycle()', () => {
849862

850-
it('skips onPreResponse when validation terminates request', async () => {
863+
it('skips onPreResponse when validation terminates request', { retry: true }, async (flags) => {
851864

852865
const server = Hapi.server();
853-
const team = new Teamwork.Team();
866+
const abortedReqTeam = new Teamwork.Team();
854867

855868
let called = false;
856869
server.ext('onPreResponse', (request, h) => {
@@ -863,28 +876,38 @@ describe('Request', () => {
863876
method: 'GET',
864877
path: '/',
865878
options: {
866-
handler: () => null,
879+
handler: (request) => {
880+
881+
// Stash raw so that we can access it on response validation
882+
Object.assign(request.app, request.raw);
883+
884+
return null;
885+
},
867886
response: {
868887
status: {
869-
200: async () => {
888+
200: async (_, { context }) => {
870889

871890
req.abort();
872-
await Hoek.wait(10);
873-
team.attend();
891+
892+
const raw = context.app.request;
893+
await Events.once(raw.req, 'aborted');
894+
895+
abortedReqTeam.attend();
874896
}
875897
}
876898
}
877899
}
878900
});
879901

880902
await server.start();
903+
flags.onCleanup = () => server.stop();
881904

882-
const req = Http.get(server.info.uri, (res) => { });
905+
const req = Http.get(server.info.uri, Hoek.ignore);
883906
req.on('error', Hoek.ignore);
884907

885-
await team.work;
886-
await Hoek.wait(100);
887-
await server.stop();
908+
await abortedReqTeam.work;
909+
910+
await server.events.once('response');
888911

889912
expect(called).to.be.false();
890913
});
@@ -2279,41 +2302,41 @@ describe('Request', () => {
22792302
expect(res.statusCode).to.equal(200);
22802303
});
22812304

2282-
it('handles race condition between equal client and server timeouts', async () => {
2305+
it('handles race condition between equal client and server timeouts', async (flags) => {
2306+
2307+
const onCleanup = [];
2308+
flags.onCleanup = async () => {
2309+
2310+
for (const cleanup of onCleanup) {
2311+
await cleanup();
2312+
}
2313+
};
22832314

22842315
const server = Hapi.server({ routes: { timeout: { server: 100 }, payload: { timeout: 100 } } });
22852316
server.route({ method: 'POST', path: '/timeout', options: { handler: Hoek.block } });
22862317

22872318
await server.start();
2319+
onCleanup.unshift(() => server.stop());
22882320

22892321
const timer = new Hoek.Bench();
22902322
const options = {
2291-
hostname: '127.0.0.1',
2323+
hostname: 'localhost',
22922324
port: server.info.port,
22932325
path: '/timeout',
22942326
method: 'POST'
22952327
};
22962328

2297-
await new Promise(async (resolve) => {
2329+
const req = Http.request(options);
2330+
onCleanup.unshift(() => req.destroy());
22982331

2299-
const req = Http.request(options, (res) => {
2332+
req.write('\n');
23002333

2301-
expect([503, 408]).to.contain(res.statusCode);
2302-
expect(timer.elapsed()).to.be.at.least(80);
2303-
resolve();
2304-
});
2334+
const [res] = await Events.once(req, 'response');
23052335

2306-
req.on('error', (err) => {
2336+
expect([503, 408]).to.contain(res.statusCode);
2337+
expect(timer.elapsed()).to.be.at.least(80);
23072338

2308-
expect(err).to.not.exist();
2309-
});
2310-
2311-
req.write('\n');
2312-
await Hoek.wait(200);
2313-
req.end();
2314-
});
2315-
2316-
await server.stop({ timeout: 1 });
2339+
await Events.once(req, 'close'); // Ensures that req closes without error
23172340
});
23182341
});
23192342

‎test/transmit.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const Stream = require('stream');
88
const Zlib = require('zlib');
99

1010
const Boom = require('@hapi/boom');
11-
const Bounce = require('@hapi/bounce');
1211
const Code = require('@hapi/code');
1312
const Hapi = require('..');
1413
const Hoek = require('@hapi/hoek');
@@ -17,6 +16,7 @@ const Lab = require('@hapi/lab');
1716
const Teamwork = require('@hapi/teamwork');
1817
const Wreck = require('@hapi/wreck');
1918

19+
const Common = require('./common');
2020

2121
const internals = {};
2222

@@ -86,7 +86,7 @@ describe('transmission', () => {
8686
expect(res.statusCode).to.equal(200);
8787
});
8888

89-
it('closes file handlers when not reading file stream', { skip: process.platform === 'win32' }, async () => {
89+
it('closes file handlers when not reading file stream', { skip: !Common.hasLsof }, async () => {
9090

9191
const server = Hapi.server();
9292
await server.register(Inert);
@@ -101,12 +101,6 @@ describe('transmission', () => {
101101
const cmd = ChildProcess.spawn('lsof', ['-p', process.pid]);
102102
let lsof = '';
103103

104-
cmd.on('error', (err) => {
105-
106-
// Allow the test to pass on platforms with no lsof
107-
Bounce.ignore(err, { errno: 'ENOENT' });
108-
});
109-
110104
cmd.stdout.on('data', (buffer) => {
111105

112106
lsof += buffer.toString();
@@ -128,7 +122,7 @@ describe('transmission', () => {
128122
});
129123
});
130124

131-
it('closes file handlers when not using a manually open file stream', { skip: process.platform === 'win32' }, async () => {
125+
it('closes file handlers when not using a manually open file stream', { skip: !Common.hasLsof }, async () => {
132126

133127
const server = Hapi.server();
134128
server.route({ method: 'GET', path: '/file', handler: (request, h) => h.response(Fs.createReadStream(__dirname + '/../package.json')).header('etag', 'abc') });
@@ -142,12 +136,6 @@ describe('transmission', () => {
142136
const cmd = ChildProcess.spawn('lsof', ['-p', process.pid]);
143137
let lsof = '';
144138

145-
cmd.on('error', (err) => {
146-
147-
// Allow the test to pass on platforms with no lsof
148-
Bounce.ignore(err, { errno: 'ENOENT' });
149-
});
150-
151139
cmd.stdout.on('data', (buffer) => {
152140

153141
lsof += buffer.toString();

0 commit comments

Comments
 (0)