Fix #435: add a test for fsPromise.unlink() to delete a file that doe… #437
Conversation
tests/spec/fs.unlink.spec.js
Outdated
| describe('fsPromises.stat', function() { | ||
| beforeEach(util.setup); | ||
| afterEach(util.cleanup); | ||
| it('should return an error if trying to delete a file that does not exist', function(done) { |
There was a problem hiding this comment.
To make code more promise-friendly, please remove done from the argument
There was a problem hiding this comment.
thank you for your suggestion :)
tests/spec/fs.unlink.spec.js
Outdated
| it('should return an error if trying to delete a file that does not exist', function(done) { | ||
| var fsPromises = util.fs().promises; | ||
|
|
||
| fsPromises.unlink('/myFile') |
There was a problem hiding this comment.
To make this more promise-friendly, please return this function.
There was a problem hiding this comment.
thank you for your suggestion :)
tests/spec/fs.unlink.spec.js
Outdated
| expect(error).to.exist; | ||
| expect(error.code).to.equal('ENOENT'); | ||
| }) | ||
| .then(() => done()); |
There was a problem hiding this comment.
To make this code more promise-friendly, please remove this line as done() is no longer needed.
There was a problem hiding this comment.
thank you for your suggestion :)
tests/spec/fs.unlink.spec.js
Outdated
| var fsPromises = util.fs().promises; | ||
|
|
||
| fsPromises.unlink('/myFile') | ||
| .then(result => expect(result).not.to.exist) |
There was a problem hiding this comment.
This line is also unneeded in a promise-based test, so please remove this line.
There was a problem hiding this comment.
@humphd Would you please help me on this issue? Do I need to include it?
There was a problem hiding this comment.
I think you can remove it, yes. It's not wrong to have it, but it's not doing anything we care about. With callbacks, you need to do this, because the callback gets called in both the error and success cases, so you have to examine state for both. With Promises, only one of the two conditions will happen.
|
@susantruong This looks good, but there's now a merge conflict (not your fault) because I've merged some other students' code. Let's fix this together in class later this week. |
|
@humphd I have made a change that you and my classmate recommended. Would you please review it? Thanks. |
susantruong
force-pushed
the
issue-435
branch
from
22ff3f5
to
4f130ff
Compare
…hat does not exist Make changes to `fsPromises.unlink()` to be more promise-friendly Added a newline to the end of the file Made a change to fsPromises.unlink() to be more friendly Changed the description of the promises test
susantruong
force-pushed
the
issue-435
branch
from
4f130ff
to
0d040b9
Compare
I wrote a test for
fsPromise.unlink()to delete a file that does not exist. If the file does not exist, it can not be opened, and throw an error. #435