test: add common.mustCall #14262
test: add common.mustCall #14262
Conversation
bengl
added
the
code-and-learn
|
This doesn't look quite correct. Instead of wrapping |
|
(continued) So, for example, remove this: process.on('exit', () => {
assert.strictEqual(requestCount, 3);
});...and add code that instead checks that whatever function is incrementing const server = http.createServer(common.mustCall((req, res) => {
requestCount++;
res.end();
}, 3)); |
TimothyGu
added
code-and-learn
|
@Trott I have take a change, please take a look |
|
@Trott I have change again, remove |
|
Still LGTM, I think! |
|
CI was green, but I canceled the Raspberry PI 1 run because of our CI backlog right now. This can land, though, IMO. Would prefer to fast-track it, but would want a few more Collaborator approvals first. However, even if no one else provides a review, this can land after being open for 72 hours. |
| const server = http.createServer((req, res) => { | ||
| requestCount++; | ||
| }); | ||
| const server = http.createServer(common.mustCall((req, res) => {}, 3)); |
There was a problem hiding this comment.
Since the function inside the mustCall() is a non-op, this can be shortened to just:
const server = http.createServer(common.mustCall(3));
| }); | ||
| const server = https.createServer( | ||
| serverOptions, | ||
| common.mustCall((req, res) => {}, 3)); |
There was a problem hiding this comment.
ditto here...
const server = http2.createServer(serverOptions, common.mustCall(3));There was a problem hiding this comment.
thx, I have already simplify common.mustCall.this made test more readable. @jasnell
|
Landed in 2e864df Thanks for the contribution! 🎉 |
PR-URL: nodejs#14262 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #14262 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test