Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor test-http-exceptions file to use countdown #17199

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ Name of the temp directory used by tests.
The `Countdown` module provides a simple countdown mechanism for tests that
require a particular action to be taken after a given number of completed
tasks (for instance, shutting down an HTTP server after a specific number of
requests).
requests). Note that _the Countdown will fail the test if the remainder
did not reach 0_.

<!-- eslint-disable strict, required-modules -->
```js
Expand Down
3 changes: 2 additions & 1 deletion test/common/countdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
const assert = require('assert');
const kLimit = Symbol('limit');
const kCallback = Symbol('callback');
const common = require('./');

class Countdown {
constructor(limit, cb) {
assert.strictEqual(typeof limit, 'number');
assert.strictEqual(typeof cb, 'function');
this[kLimit] = limit;
this[kCallback] = cb;
this[kCallback] = common.mustCall(cb);
}

dec() {
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/failcounter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const Countdown = require('../common/countdown');
new Countdown(2, () => {});
22 changes: 20 additions & 2 deletions test/parallel/test-common-countdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,31 @@
const common = require('../common');
const assert = require('assert');
const Countdown = require('../common/countdown');
const fixtures = require('../common/fixtures');
const { execFile } = require('child_process');

let done = '';

const countdown = new Countdown(2, common.mustCall(() => done = true));
const countdown = new Countdown(2, () => done = true);
assert.strictEqual(countdown.remaining, 2);
countdown.dec();
assert.strictEqual(countdown.remaining, 1);
countdown.dec();
assert.strictEqual(countdown.remaining, 0);
assert.strictEqual(done, true);

const failFixtures = [
[
fixtures.path('failcounter.js'),
'Mismatched <anonymous> function calls. Expected exactly 1, actual 0.',
]
];

for (const p of failFixtures) {
const [file, expected] = p;
execFile(process.argv[0], [file], common.mustCall((ex, stdout, stderr) => {
assert.ok(ex);
assert.strictEqual(stderr, '');
const firstLine = stdout.split('\n').shift();
assert.strictEqual(firstLine, expected);
}));
}
2 changes: 1 addition & 1 deletion test/parallel/test-http-after-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const server = http.createServer(common.mustCall((req, res) => {
setTimeout(() => res.end(req.url), 50);
}, 2));

const countdown = new Countdown(2, common.mustCall(() => server.close()));
const countdown = new Countdown(2, () => server.close());

server.on('connect', common.mustCall((req, socket) => {
socket.write('HTTP/1.1 200 Connection established\r\n\r\n');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-agent-destroyed-socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const server = http.createServer(common.mustCall((req, res) => {
assert(request1.socket.destroyed);
// assert not reusing the same socket, since it was destroyed.
assert.notStrictEqual(request1.socket, request2.socket);
const countdown = new Countdown(2, common.mustCall(() => server.close()));
const countdown = new Countdown(2, () => server.close());
request2.socket.on('close', common.mustCall(() => countdown.dec()));
response.on('end', common.mustCall(() => countdown.dec()));
response.resume();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-agent-maxsockets-regress-4050.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const server = http.createServer(common.mustCall((req, res) => {
res.end('hello world');
}, 6));

const countdown = new Countdown(6, common.mustCall(() => server.close()));
const countdown = new Countdown(6, () => server.close());

function get(path, callback) {
return http.get({
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-agent-maxsockets.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ function get(path, callback) {
}, callback);
}

const countdown = new Countdown(2, common.mustCall(() => {
const countdown = new Countdown(2, () => {
const freepool = agent.freeSockets[Object.keys(agent.freeSockets)[0]];
assert.strictEqual(freepool.length, 2,
`expect keep 2 free sockets, but got ${freepool.length}`);
agent.destroy();
server.close();
}));
});

function dec() {
process.nextTick(() => countdown.dec());
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-http-client-abort.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const Countdown = require('../common/countdown');

const N = 8;

const countdown = new Countdown(N, common.mustCall(() => server.close()));
const countdown = new Countdown(N, () => server.close());

const server = http.Server(common.mustCall((req, res) => {
res.writeHead(200);
Expand All @@ -37,9 +37,9 @@ const server = http.Server(common.mustCall((req, res) => {
server.listen(0, common.mustCall(() => {

const requests = [];
const reqCountdown = new Countdown(N, common.mustCall(() => {
const reqCountdown = new Countdown(N, () => {
requests.forEach((req) => req.abort());
}));
});

const options = { port: server.address().port };

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-client-parse-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const http = require('http');
const net = require('net');
const Countdown = require('../common/countdown');

const countdown = new Countdown(2, common.mustCall(() => server.close()));
const countdown = new Countdown(2, () => server.close());

const payloads = [
'HTTP/1.1 302 Object Moved\r\nContent-Length: 0\r\n\r\nhi world',
Expand Down
27 changes: 17 additions & 10 deletions test/parallel/test-http-exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const Countdown = require('../common/countdown');
const http = require('http');
const NUMBER_OF_EXCEPTIONS = 4;
const countdown = new Countdown(NUMBER_OF_EXCEPTIONS, () => {
process.exit(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be server.close instead? (of course this line would need to be placed after the server creation)

Copy link
Contributor Author

@Bamieh Bamieh Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joyeecheung calling server.close() does not close the node process for some reason. resulting in a --- TIMEOUT --- in the tests. These are just refactors, and the initial code also calls process.exit. What do you suggest here?

});

const server = http.createServer(function(req, res) {
intentionally_not_defined(); // eslint-disable-line no-undef
Expand All @@ -30,16 +35,18 @@ const server = http.createServer(function(req, res) {
res.end();
});

function onUncaughtException(err) {
console.log(`Caught an exception: ${err}`);
if (err.name === 'AssertionError') throw err;
countdown.dec();
}

process.on('uncaughtException',
common.mustCall(onUncaughtException, NUMBER_OF_EXCEPTIONS)
);

server.listen(0, function() {
for (let i = 0; i < 4; i += 1) {
for (let i = 0; i < NUMBER_OF_EXCEPTIONS; i += 1) {
http.get({ port: this.address().port, path: `/busy/${i}` });
}
});

let exception_count = 0;

process.on('uncaughtException', function(err) {
console.log(`Caught an exception: ${err}`);
if (err.name === 'AssertionError') throw err;
if (++exception_count === 4) process.exit(0);
});
4 changes: 2 additions & 2 deletions test/parallel/test-http2-no-more-streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ server.listen(0, common.mustCall(() => {

assert.strictEqual(client.state.nextStreamID, nextID);

const countdown = new Countdown(2, common.mustCall(() => {
const countdown = new Countdown(2, () => {
server.close();
client.destroy();
}));
});

{
// This one will be ok
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ server.listen(0, common.mustCall(() => {
const port = server.address().port;
const client = http2.connect(`http://localhost:${port}`);

const countdown = new Countdown(2, common.mustCall(() => {
const countdown = new Countdown(2, () => {
server.close();
client.destroy();
}));
});

const req = client.request({ ':method': 'POST' });
req.on('response', common.mustCall());
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-server-rst-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ server.on('stream', (stream, headers) => {
server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

const countdown = new Countdown(tests.length, common.mustCall(() => {
const countdown = new Countdown(tests.length, () => {
client.destroy();
server.close();
}));
});

tests.forEach((test) => {
const req = client.request({
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-performanceobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ assert.strictEqual(counts[NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION], 0);
new PerformanceObserver(common.mustCall(callback, 3));

const countdown =
new Countdown(3, common.mustCall(() => {
new Countdown(3, () => {
observer.disconnect();
assert.strictEqual(counts[NODE_PERFORMANCE_ENTRY_TYPE_MARK], 1);
}));
});

function callback(list, obs) {
assert.strictEqual(obs, observer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const server = net.createServer(function onClient(client) {
});

server.listen(0, common.localhostIPv4, common.mustCall(() => {
const countdown = new Countdown(2, common.mustCall(() => server.close()));
const countdown = new Countdown(2, () => server.close());

{
const client = net.connect({ port: server.address().port });
Expand Down