Skip to content

test: s/assert.fail/common.fail as appropriate - #7735

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:fail
Jul 15, 2016
Merged

test: s/assert.fail/common.fail as appropriate#7735
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:fail

Conversation

@cjihrig

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

Many tests use assert.fail(null, null, msg) where it would be simpler to use common.fail(msg). This is largely because common.fail() is fairly new. This commit makes the replacement when applicable.

R= @nodejs/testing

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jul 14, 2016
@thefourtheye

Copy link
Copy Markdown
Contributor

LGTM, if CI is green.

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.

Dead code.

@bnoordhuis

Copy link
Copy Markdown
Member

LGTM

@cjihrig
cjihrigforce-pushed the fail branch 2 times, most recently from 1801500 to 1a14f45CompareJuly 15, 2016 14:42
@cjihrig

Copy link
Copy Markdown
ContributorAuthor

@Fishrock123

Copy link
Copy Markdown
Contributor

LGTM if CI is green

@cjihrig

Copy link
Copy Markdown
ContributorAuthor

CI was green, minus one buildbot that appears to have gone offline (which is consistent with the last few CI runs). Trying again. CI: https://ci.nodejs.org/job/node-test-pull-request/3301/

@cjihrig

Copy link
Copy Markdown
ContributorAuthor

Still with the ppcbe-ubuntu1404 buildbot. Taking it as a green CI.

Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: nodejs#7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@cjihrig
cjihrig merged commit 6510eb5 into nodejs:masterJul 15, 2016
@cjihrig
cjihrig deleted the fail branch July 15, 2016 19:55
evanlucas pushed a commit that referenced this pull request Jul 19, 2016
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: #7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@evanlucasevanlucas mentioned this pull request Jul 19, 2016
evanlucas pushed a commit that referenced this pull request Jul 20, 2016
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: #7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: #7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: #7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.
PR-URL: #7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@MylesBorinsMylesBorins mentioned this pull request Oct 26, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@cjihrig@thefourtheye@bnoordhuis@Fishrock123@MylesBorins@nodejs-github-bot