test: refactor the code in test-http-connect - #10397
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
I would avoid adding common.mustCall() to the data handler. The number of events emitted can potentially be different, and it isn't really needed for the test.
Sorry, something went wrong.
There was a problem hiding this comment.
Same comment here.
Sorry, something went wrong.
There was a problem hiding this comment.
By using common.mustCall(), I think these variables can be eliminated completely, as can this exit event handler.
Sorry, something went wrong.
|
@cjihrig , just implemented your suggestions |
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Nit: can't this be removed now? I mean the clientRequestClosed flag.
Sorry, something went wrong.
|
@lpinca removed as suggested, please set the CI again |
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Nit: this can also probably be removed.
Sorry, something went wrong.
There was a problem hiding this comment.
Same here.
Sorry, something went wrong.
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables
|
New CI before landing: https://ci.nodejs.org/job/node-test-pull-request/5635/ |
Sorry, something went wrong.
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically * use let and const instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error and unnecessary variables PR-URL: #10397 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist
Affected core subsystem(s)
test
Description of change