Skip to content

Commit

Permalink
Fix tests for citgm (#4221)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@devinivy
devinivy authored Jan 22, 2021
1 parent f3e0ed1 commit 7e99521
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 69 deletions.
19 changes: 19 additions & 0 deletions test/common.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

const ChildProcess = require('child_process');

const internals = {};

internals.hasLsof = () => {

try {
ChildProcess.execSync(`lsof -p ${process.pid}`, { stdio: 'ignore' });
}
catch (err) {
return false;
}

return true;
};

exports.hasLsof = internals.hasLsof();
10 changes: 2 additions & 8 deletions test/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const Stream = require('stream');
const TLS = require('tls');

const Boom = require('@hapi/boom');
const Bounce = require('@hapi/bounce');
const CatboxMemory = require('@hapi/catbox-memory');
const Code = require('@hapi/code');
const Handlebars = require('handlebars');
Expand All @@ -22,6 +21,7 @@ const Lab = require('@hapi/lab');
const Vision = require('@hapi/vision');
const Wreck = require('@hapi/wreck');

const Common = require('./common');

const internals = {};

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

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

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

cmd.on('error', (err) => {

// Allow the test to pass on platforms with no lsof
Bounce.ignore(err, { errno: 'ENOENT' });
});

cmd.stdout.on('data', (buffer) => {

lsof += buffer.toString();
Expand Down
115 changes: 69 additions & 46 deletions test/request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const Http = require('http');
const Net = require('net');
const Stream = require('stream');
const Url = require('url');
const Events = require('events');

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

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

it('exits handler early when request is no longer active', async () => {
it('exits handler early when request is no longer active', { retry: true }, async (flags) => {

let testComplete = false;

const onCleanup = [];
flags.onCleanup = async () => {

testComplete = true;

for (const cleanup of onCleanup) {
await cleanup();
}
};

const server = Hapi.server();
const team = new Teamwork.Team();
const team2 = new Teamwork.Team();
const leaveHandlerTeam = new Teamwork.Team();

let rounds = 0;
server.route({
method: 'GET',
path: '/',
options: {
handler: async (request, h) => {

team2.attend();
for (let i = 0; i < 100; ++i) {
++rounds;
await Hoek.wait(10);
req.abort();

if (!request.active()) {
break;
}
while (request.active() && !testComplete) {
await Hoek.wait(10);
}

team.attend();
leaveHandlerTeam.attend({
active: request.active(),
testComplete
});

return null;
}
}
});

await server.start();
onCleanup.unshift(() => server.stop());

const req = Http.get(server.info.uri, (res) => { });
const req = Http.get(server.info.uri, Hoek.ignore);
req.on('error', Hoek.ignore);
await team2.work;
req.abort();
await server.stop();

await team.work;
expect(rounds).to.be.below(10);
const note = await leaveHandlerTeam.work;

expect(note).to.equal({
active: false,
testComplete: false
});
});
});

Expand Down Expand Up @@ -847,10 +860,10 @@ describe('Request', () => {

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

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

const server = Hapi.server();
const team = new Teamwork.Team();
const abortedReqTeam = new Teamwork.Team();

let called = false;
server.ext('onPreResponse', (request, h) => {
Expand All @@ -863,28 +876,38 @@ describe('Request', () => {
method: 'GET',
path: '/',
options: {
handler: () => null,
handler: (request) => {

// Stash raw so that we can access it on response validation
Object.assign(request.app, request.raw);

return null;
},
response: {
status: {
200: async () => {
200: async (_, { context }) => {

req.abort();
await Hoek.wait(10);
team.attend();

const raw = context.app.request;
await Events.once(raw.req, 'aborted');

abortedReqTeam.attend();
}
}
}
}
});

await server.start();
flags.onCleanup = () => server.stop();

const req = Http.get(server.info.uri, (res) => { });
const req = Http.get(server.info.uri, Hoek.ignore);
req.on('error', Hoek.ignore);

await team.work;
await Hoek.wait(100);
await server.stop();
await abortedReqTeam.work;

await server.events.once('response');

expect(called).to.be.false();
});
Expand Down Expand Up @@ -2279,41 +2302,41 @@ describe('Request', () => {
expect(res.statusCode).to.equal(200);
});

it('handles race condition between equal client and server timeouts', async () => {
it('handles race condition between equal client and server timeouts', async (flags) => {

const onCleanup = [];
flags.onCleanup = async () => {

for (const cleanup of onCleanup) {
await cleanup();
}
};

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

await server.start();
onCleanup.unshift(() => server.stop());

const timer = new Hoek.Bench();
const options = {
hostname: '127.0.0.1',
hostname: 'localhost',
port: server.info.port,
path: '/timeout',
method: 'POST'
};

await new Promise(async (resolve) => {
const req = Http.request(options);
onCleanup.unshift(() => req.destroy());

const req = Http.request(options, (res) => {
req.write('\n');

expect([503, 408]).to.contain(res.statusCode);
expect(timer.elapsed()).to.be.at.least(80);
resolve();
});
const [res] = await Events.once(req, 'response');

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

expect(err).to.not.exist();
});

req.write('\n');
await Hoek.wait(200);
req.end();
});

await server.stop({ timeout: 1 });
await Events.once(req, 'close'); // Ensures that req closes without error
});
});

Expand Down
18 changes: 3 additions & 15 deletions test/transmit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const Stream = require('stream');
const Zlib = require('zlib');

const Boom = require('@hapi/boom');
const Bounce = require('@hapi/bounce');
const Code = require('@hapi/code');
const Hapi = require('..');
const Hoek = require('@hapi/hoek');
Expand All @@ -17,6 +16,7 @@ const Lab = require('@hapi/lab');
const Teamwork = require('@hapi/teamwork');
const Wreck = require('@hapi/wreck');

const Common = require('./common');

const internals = {};

Expand Down Expand Up @@ -86,7 +86,7 @@ describe('transmission', () => {
expect(res.statusCode).to.equal(200);
});

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

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

cmd.on('error', (err) => {

// Allow the test to pass on platforms with no lsof
Bounce.ignore(err, { errno: 'ENOENT' });
});

cmd.stdout.on('data', (buffer) => {

lsof += buffer.toString();
Expand All @@ -128,7 +122,7 @@ describe('transmission', () => {
});
});

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

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

cmd.on('error', (err) => {

// Allow the test to pass on platforms with no lsof
Bounce.ignore(err, { errno: 'ENOENT' });
});

cmd.stdout.on('data', (buffer) => {

lsof += buffer.toString();
Expand Down

0 comments on commit 7e99521

Please sign in to comment.