Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: fix nits in test-fs-mkdir-rmdir.js #13680

Closed
wants to merge 1 commit into from
Closed

test: fix nits in test-fs-mkdir-rmdir.js #13680

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Jun 14, 2017 •
edited
Loading

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

doc, fs

  • Add common.mustCall().
  • Add check for error existence, not only for error details.
  • Use test(), not match() in a boolean context.
  • Properly reverse meanings of assert messages.

Sorry, GitHub diff is disoriented a bit.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 14, 2017
@vsemozhetbyt vsemozhetbyt added the fs Issues and PRs related to the fs subsystem / file system. label Jun 14, 2017
@vsemozhetbyt
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/8649/

richardlau
richardlau reviewed Jun 14, 2017
View reviewed changes
test/parallel/test-fs-mkdir-rmdir.js Outdated
@@ -28,9 +28,10 @@ fs.mkdir(d, 0o666, function(err) {
assert.ifError(err);

fs.mkdir(d, 0o666, function(err) {
Copy link
Member

Choose a reason for hiding this comment

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

While we're here, should this be common.mustCall()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richardlau Done, twice.

@vsemozhetbyt
test: fix nits in test-fs-mkdir-rmdir.js
6f939ed 
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 14, 2017 •
edited
Loading

Add common.mustCall().

New CI: https://ci.nodejs.org/job/node-test-pull-request/8654/

One flaky test fail seems unrelated.

@vsemozhetbyt
Copy link
Contributor Author

Landed in 50b3546

vsemozhetbyt added a commit that referenced this pull request Jun 16, 2017
@vsemozhetbyt
test: fix nits in test-fs-mkdir-rmdir.js
50b3546 
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.

PR-URL: #13680
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@vsemozhetbyt vsemozhetbyt deleted the test-fs-mkdir-rmdir branch June 16, 2017 14:50
addaleax pushed a commit that referenced this pull request Jun 17, 2017
@vsemozhetbyt @addaleax
test: fix nits in test-fs-mkdir-rmdir.js
49d8fd6 
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.

PR-URL: #13680
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@addaleax addaleax mentioned this pull request Jun 17, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@vsemozhetbyt @addaleax
test: fix nits in test-fs-mkdir-rmdir.js
80e6524 
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.

PR-URL: #13680
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
@vsemozhetbyt @MylesBorins
test: fix nits in test-fs-mkdir-rmdir.js
fa75be7 
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.

PR-URL: #13680
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vsemozhetbyt @jasnell @lpinca @cjihrig @richardlau @mhdawson @MylesBorins @nodejs-github-bot