test: add common.mustCall#14262
test: add common.mustCall #14262
Uh oh!
There was an error while loading. Please reload this page.
Conversation
bengl added the code-and-learn 
TimothyGu commented Jul 16, 2017
Trott commented Jul 16, 2017
This doesn't look quite correct. Instead of wrapping |
Trott commented Jul 16, 2017
(continued) So, for example, remove this: process.on('exit',()=>{assert.strictEqual(requestCount,3);});...and add code that instead checks that whatever function is incrementing constserver=http.createServer(common.mustCall((req,res)=>{requestCount++;res.end();},3)); |
TimothyGu added code-and-learn 
Suixinlei commented Jul 16, 2017
@Trott I have take a change, please take a look |
Suixinlei commented Jul 16, 2017
@Trott I have change again, remove |
Trott commented Jul 16, 2017
Still LGTM, I think! |
vsemozhetbyt commented Jul 16, 2017
Trott commented Jul 16, 2017
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. |
| constserver=http.createServer((req,res)=>{ | ||
| requestCount++; | ||
| }); | ||
| constserver=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:
constserver=http.createServer(common.mustCall(3));| }); | ||
| constserver=https.createServer( | ||
| serverOptions, | ||
| common.mustCall((req,res)=>{},3)); |
There was a problem hiding this comment.
ditto here...
constserver=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
Trott commented Jul 26, 2017
Trott commented Jul 26, 2017
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]>
12 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test