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.

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

test: refactor test-fs-non-number-arguments-throw #9844

Closed
wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented Nov 29, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test, fs

Description of change
  • Add RegExp arguments to throws assertions.
  • Use common.mustCall for emitter callback.

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

* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.
@targos targos added fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests. labels Nov 29, 2016

assert.throws(function() {
fs.createReadStream(tempFile, { start: 4, end: '6' });
}, "end as string didn't throw an error");
}, /^TypeError: "end" option must be a Number$/,
"end as string didn't throw an error");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add "for createReadStream" to the error message for consistency.

targos added a commit to targos/node that referenced this pull request Dec 2, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: nodejs#9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos
Copy link
Member Author

targos commented Dec 2, 2016

Landed in de495c0

@targos targos closed this Dec 2, 2016
@targos targos deleted the refactor-test-fs-readstream branch December 2, 2016 12:29
addaleax pushed a commit that referenced this pull request Dec 5, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: #9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: nodejs#9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: nodejs#9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins
Copy link
Contributor

This test is failing on v4.x. Is this expected behavior?

targos added a commit to targos/node that referenced this pull request Dec 21, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: nodejs#9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos
Copy link
Member Author

targos commented Dec 21, 2016

Backport PR: #10383

MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: #9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: #9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: #9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* Add RegExp arguments to throws assertions.
* Use common.mustCall for emitter callback.

PR-URL: #9844
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This was referenced Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants