Skip to content

Refactor test http exceptions #18506

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.

Sign up for GitHub

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

Closed
Bamieh wants to merge 8 commits into from
Closed

Refactor test http exceptions #18506

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f802a56
Fixes: https://github.com/nodejs/node/issues/17169 (test-http-excepti…
Bamieh Nov 21, 2017
4cd8fbc
mustCall inside countdown
Bamieh Nov 23, 2017
4286f71
Add test case for countdown
Bamieh Nov 23, 2017
1eb069c
Update countdown docs to hint the change
Bamieh Nov 23, 2017
7ad8ce7
Update README.md
Bamieh Nov 23, 2017
6f125e1
Update README.md
Bamieh Nov 23, 2017
0060979
update readme
Bamieh Feb 1, 2018
6840cf3
up up up one line
Bamieh Feb 1, 2018
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
Prev Previous commit
Next Next commit
mustCall inside countdown
  • Loading branch information
@Bamieh
Bamieh committed Feb 7, 2018
commit 4cd8fbca6e492b4be9ef3e6b5a9cd57139a61795
3 changes: 2 additions & 1 deletion test/common/countdown.js
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions test/parallel/test-common-countdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');
const Countdown = require('../common/countdown');

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);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-after-connect.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
21 changes: 13 additions & 8 deletions test/parallel/test-http-exceptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +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));
const countdown = new Countdown(NUMBER_OF_EXCEPTIONS, () => {
process.exit(0);
Copy link
Copy Markdown
Contributor

@apapirovski apapirovski Feb 1, 2018

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
Copy Markdown
Contributor Author

@Bamieh Bamieh Feb 1, 2018 •
edited
Loading

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 @@ -33,15 +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)
);
Copy link
Copy Markdown
Contributor

@apapirovski apapirovski Feb 1, 2018

Choose a reason for hiding this comment

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

Can this be brought up to the previous line? The dangling ); is awkward.

Copy link
Copy Markdown
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.

sadly it cant be moved up since it will break the linter due to reaching the max limit of 80 characters in one line

Copy link
Copy Markdown
Contributor

@apapirovski apapirovski Feb 1, 2018

Choose a reason for hiding this comment

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

It's only 70 chars as far as I can tell?


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

process.on('uncaughtException', function(err) {
console.log(`Caught an exception: ${err}`);
if (err.name === 'AssertionError') throw err;
countdown.dec();
});
2 changes: 1 addition & 1 deletion test/parallel/test-http2-no-more-streams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ 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();
}));
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-server-rst-stream.js
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion test/parallel/test-timers-socket-timeout-removes-other-socket-unref-timer.js
View file
Open in desktop
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