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

add test for fs.appendFile for valid file system flag #463

Open
MuchtarSalimov opened this issue Sep 23, 2018 · 1 comment · May be fixed by #546
Open

add test for fs.appendFile for valid file system flag #463

MuchtarSalimov opened this issue Sep 23, 2018 · 1 comment · May be fixed by #546
Assignees

Comments

@MuchtarSalimov
Copy link

MuchtarSalimov commented Sep 23, 2018

When calling fs.appendFile(path, data[, options], callback), within the options object we may include a file system flag (defaults to 'a' so that the file is opened for appending).

I would like to:

  • add a test for explicitly setting the mode to 'a' when using fs.appendFile.

However it is not obvious to me why the user might have the option of choosing a flag other than 'a'. Is there any value in leaving it as an option? It seems to me that for this method, any selection would either cause it to behave as if it were opened using 'a', or else cause some kind of error.

Update: I ended up making two test cases for testing the default values ( #494 ). This revealed a bug in the the implementation of appendFile. I have submitted another PR ( #546 ) that fixes this (and tests against it).

@MuchtarSalimov MuchtarSalimov changed the title add tests for fs.appendFile for valid file system flags add test for fs.appendFile for valid file system flags Sep 23, 2018
@MuchtarSalimov MuchtarSalimov changed the title add test for fs.appendFile for valid file system flags add test for fs.appendFile for valid file system flag Sep 23, 2018
@humphd
Copy link
Contributor

humphd commented Sep 24, 2018

See https://nodejs.org/api/fs.html#fs_file_system_flags and https://github.com/nodejs/node/blob/eef072fa083f05f84fa6ca1908472eb228095a38/lib/fs.js#L1227-L1238 for what node does. Filer needs to support the same options as node, even if we end up ignoring the. I agree that if you pass certain flags it should fail, but that's a reasonable test to write. My advice: try passing different flags to node's version, see what it does, write tests for Filer that expect the same behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants