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

Update fs.stat(), fs.lstat(), and fs.fstat() for Node 7.7+ (fixes #197) #198

Merged

Conversation

not-an-aardvark
Copy link
Contributor

Node 7.7 changed the behavior of the binding.{stat,lstat,fstat} functions (see nodejs/node#11522). This updates the binding functions used in mock-fs to match the new Node binding behavior, while still maintaining compatibility with old Node versions. The new behavior is detected when the second argument to binding.{stat,lstat,fstat} is a Float64Array, which would be an invalid argument for previous versions of the binding.

This commit does not add any tests because the existing tests were already broken by the Node update. Given that they're passing now, I think the behavior is already covered, but let me know if there are any tests I should add.

…haub#197)

Node 7.7 changed the behavior of the `binding.{stat,lstat,fstat}` functions (see nodejs/node#11522). This updates the binding functions used in mock-fs to match the new Node binding behavior, while still maintaining compatibility with old Node versions. The new behavior is detected when the second argument to `binding.{stat,lstat,fstat}` is a `Float64Array`, which would be an invalid argument for previous versions of the binding.
@tschaub
Copy link
Owner

tschaub commented Mar 13, 2017

Thanks for the fix @not-an-aardvark!

@not-an-aardvark not-an-aardvark deleted the update-binding-stat-lstat-fstat branch March 13, 2017 20:08
@not-an-aardvark not-an-aardvark restored the update-binding-stat-lstat-fstat branch March 13, 2017 20:08
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 this pull request may close these issues.

2 participants