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

Fix#461 - Added test for fs.read when attempting to read a file that does not exist #472

Merged
merged 4 commits into from Oct 16, 2018

Conversation

Projects
None yet
3 participants
@CometS1
Contributor

CometS1 commented Sep 24, 2018

Created a test for fs.read attempting to read a file that does not exist to check to see if an error is received.

@0xazure

Overall looks good! I made a few inline comments for improving the test case.

done();
});
});
});

This comment has been minimized.

@0xazure

0xazure Sep 24, 2018

Contributor

nit: file should end in a blank line

var fd = 0;
fs.read(fd, buffer, 0, buffer.length, undefined, function(error, result) {

This comment has been minimized.

@0xazure

0xazure Sep 24, 2018

Contributor

instead of position = undefined, i would model this test case more closely after the test for reading from a file. we don't want to change too many variables at a time in our tests, because each test should only test one thing. in this case, we're testing what happens when fd is zero and position is undefined, instead of just one or the other.

@CometS1 CometS1 changed the title from Added test for fs.read when attempting to read a file that does not exist to Fix#461 - Added test for fs.read when attempting to read a file that does not exist Sep 24, 2018

@humphd

Looks good, one extra check is needed to deal with the specific error code.

rbuffer.fill(0);
fs.read(fd, rbuffer, 0, rbuffer.length, 0, function(error, result) {
expect(error).to.exist;

This comment has been minimized.

@humphd

humphd Oct 9, 2018

Contributor

We should also deal with the specific error code. In Filer that would (currently) be 'EBADF'. In note that in node, it would be: Error: ENXIO: no such device or address, read errno: -6, code: 'ENXIO'. This would require a fix to our implementation if we want to match 100% (not sure it's that important).

@humphd humphd merged commit 1eab5f0 into filerjs:master Oct 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment