-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: improve fs error message test #18277
Conversation
- Reuse error validators for sync and async tests - Validate properties of the errors
assert.strictEqual(nonexistentFile, err.path); | ||
// Could be resolved to an absolute path | ||
assert.ok(err.dest.endsWith('foo'), | ||
`expect ${err.dest} to ends with 'foo'`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: to end
assert.strictEqual(nonexistentFile, err.path); | ||
// Could be resolved to an absolute path | ||
assert.ok(err.dest.endsWith('foo'), | ||
`expect ${err.dest} to ends with 'foo'`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
} catch (err) { | ||
errors.push('opens'); | ||
assert.ok(err.message.includes(fn)); | ||
// rmdir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: rmdir on a file
} catch (err) { | ||
errors.push('readdir'); | ||
assert.ok(err.message.includes(fn)); | ||
// read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readFile
@targos Thanks for the review. Updated.
|
e127886
to
15061d2
Compare
Only one unrelated failure in the last CI. Fired another one just in case: https://ci.nodejs.org/job/node-test-pull-request/12664/ |
Landed in 63f78f5, thanks! |
- Reuse error validators for sync and async tests - Validate properties of the errors PR-URL: #18277 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Should this be backported to |
- Reuse error validators for sync and async tests - Validate properties of the errors PR-URL: nodejs#18277 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
In preparation for more fs error migrations..
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test, fs