Skip to content

test: refactor code in test-cluster-http-pipe - #10297

Closed
edsadr wants to merge 1 commit into
nodejs:masterfrom
edsadr:test-cluster-http
Closed

test: refactor code in test-cluster-http-pipe#10297
edsadr wants to merge 1 commit into
nodejs:masterfrom
edsadr:test-cluster-http

Conversation

@edsadr

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change
  • remove the manual control for functions execution
  • use common.mustCall to control the functions execution automatically
  • use const instead of var
  • use assert.strictEqual instead assert.equal
  • use assert.ifError instead of throw error

@nodejs-github-botnodejs-github-bot added test Issues and PRs related to the tests. lts-watch-v6.x labels Dec 16, 2016
@mscdexmscdex added cluster Issues and PRs related to the cluster subsystem. http Issues or PRs related to the http subsystem. labels Dec 16, 2016
@italoacasas

Copy link
Copy Markdown

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Calling process.exit(); is not necessary

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would wrap the callback with common.mustCall()

* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
@edsadr

Copy link
Copy Markdown
ContributorAuthor

@santigimeno implemented your suggestions here, @italoacasas please set another CI task I guess?

@cjihrigcjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with a new CI run.

@JungMinu

Copy link
Copy Markdown
Member

@JungMinuJungMinu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Dec 23, 2016
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell

Copy link
Copy Markdown
Member

Landed in 9073fda

@jasnelljasnell closed this Dec 23, 2016
@evanlucasevanlucas mentioned this pull request Jan 3, 2017
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jan 24, 2017
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: #10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clusterIssues and PRs related to the cluster subsystem.httpIssues or PRs related to the http subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@edsadr@italoacasas@JungMinu@jasnell@santigimeno@lpinca@cjihrig@mscdex@MylesBorins@nodejs-github-bot