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 #18506

Closed
wants to merge 8 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
2 changes: 1 addition & 1 deletion test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ Synchronous version of `spawnPwd`.
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). 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
26 changes: 16 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.

Does server.close() not do the trick here?

Copy link
Contributor Author

@Bamieh Bamieh Feb 1, 2018

Choose a reason for hiding this comment

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

it does not, probably because of the intended exception throwing intentionally_not_defined. it keeps res from being closed with .end();.

});

const server = http.createServer(function(req, res) {
intentionally_not_defined(); // eslint-disable-line no-undef
Expand All @@ -30,16 +35,17 @@ 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));
Copy link
Member

Choose a reason for hiding this comment

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

Nit, the common.mustCall is obsolete here, because the test will not execute countdown.dec() in case it would not be called and would then fail because the wrapper around the countdown would fail.


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.close();
}));
});

{
// This one will be ok
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 @@ -32,10 +32,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.close();
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 @@ -65,10 +65,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