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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #561 - Extend fs.access to support R_OK, W_OK, and X_OK #601

Merged
merged 2 commits into from Dec 4, 2018

Conversation

@deepanjali19
Copy link
Contributor

deepanjali19 commented Dec 4, 2018

I have added the support test for fs.access() to test various conditions with R_OK, W_OK and X_OK.

There is one test that is skipped for now. The test should return an error if file is not executable and mode = X_OK.

We create a new bug for this issue. Thank you 馃憤

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 4, 2018

Codecov Report

Merging #601 into master will increase coverage by 0.03%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
+ Coverage   86.13%   86.16%   +0.03%     
==========================================
  Files          16       16              
  Lines        1716     1720       +4     
==========================================
+ Hits         1478     1482       +4     
  Misses        238      238
Impacted Files Coverage 螖
src/errors.js 100% <酶> (酶) 猬嗭笍
src/filesystem/implementation.js 83.21% <75%> (+0.06%) 猬嗭笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 0f94c47...dfc636c. Read the comment docs.

Copy link
Contributor

humphd left a comment

A couple small things, and this is ready to go!

});
});

it.skip('should return an error if file is not executable and mode = X_OK', function (done) {

This comment has been minimized.

Copy link
@humphd

humphd Dec 4, 2018

Contributor

Can you add a comment above this that says:

// See bug https://github.com/filerjs/filer/issues/602

This comment has been minimized.

Copy link
@humphd

humphd Dec 4, 2018

Contributor

I filed #602 to deal with this later.

if(mode === Constants.F_OK){
return callback(null);
}
if (!(mode & Constants.X_OK) || (node.mode & (Constants.fsConstants.S_IXUSR | Constants.fsConstants.S_IXGRP | Constants.fsConstants.S_IXOTH)))

This comment has been minimized.

Copy link
@humphd

humphd Dec 4, 2018

Contributor

Please wrap the return in a block:

if( ... ) {
  ...
}
@humphd
humphd approved these changes Dec 4, 2018
@humphd humphd merged commit 0f93a04 into filerjs:master Dec 4, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@humphd

This comment has been minimized.

Copy link
Contributor

humphd commented Dec 4, 2018

Great job @deepanjali19, you did it.

@deepanjali19

This comment has been minimized.

Copy link
Contributor Author

deepanjali19 commented Dec 4, 2018

Thanks to you @humphd ! You are the best mentor I could have ever asked for. You stood strong beside me for helping me throughout this. I truly appreciate it 馃挴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.