Helper to bypass mock FS & expose real files/directories

[nonara]

Adds two new features:

1) mock.bypass() - Allows executing commands against the actual FS 2) mock.load() - Reads the filesystem to map actual files and directories from the filesystem

Options: `recursive`, `lazyLoad`
- If lazyLoad is true, file content isn't loaded until they're explicitly read. (keeps memory usage low, making it feasible to load large filesystems)

See readme for more detail

closes #62 closes #213 (possibly others?)

[nonara]

Will update README tomorrow and @types/mock-fs as soon as it the PR is approved. Would appreciate a comment to let me know it's been / being reviewed.

Thanks!

[nonara]

This can be considered complete (unless changes requested)

Recent updates:

• Made automatically created directories inherit stats.
  • This lead to discovering an issue with windows permissions. (See my code comment above for detail)
• Updated readme
• Made sure non-lazy-loaded files inherit stats

[nonara]

Ok! I think you'll find the new API much more intuitive and flexible! I believe the new readme should be easy to understand, as well.

We now have three new mapper methods: mock.mapPaths(), mock.mapDir(), and mock.mapFile().

I re-wrote everything which also allowed for better tests & better overall architecture.

[nonara]

Addressed comments.

[3cp]

There is a design issue. When user wants to bypass an async func, your implementation doesn't work.

mock.bypass(async () => {
  const stats = await fs.promises.stat('/path/to/file.txt');
  ...
  const data = await fs.promises.readFile('path/to/file.txt');
  ...
});

Your implementation probably needs to wrap the return of fn() in Promise.resolve(fn() and use promise api to reset mock. But this might introduce unwanted code when user only wants to use it in sync mode.

It might need to check whether the return of fn() responds to .then func, and deal with sync and async modes differently.

[nonara]

There is a design issue. When user wants to bypass an async func, your implementation doesn't work.

It was actually designed intentionally to be synchronous. Supporting asynchronous code is likely to produce a lot of problems.

If you'd really like, I can make it support async and add a typescript JSDoc overload which includes a warning + add it to the readme.

[3cp]

I also think async mode is troublesome, so I am not sure if my suggestion is a good idea. As long as you document it clearly.

[nonara]

I will clarify in the readme that mock.bypass() should be used for synchronous calls.

One possible route is to add mock.enable() and mock.disable() helpers for those who know what they're doing and want to risk async calls.

Thoughts on that?

[3cp]

Maybe keep bypass() func as internal? I think your other APIs can cover those github issues. @tschaub any thought?

Because, to end user, bypass() is not a show stopper. They can still do mockfs.restore(); fn(); mockfs(fakeFs_again); manually.

[nonara]

Because, to end user, bypass() is not a show stopper. They can still do mockfs.restore(); fn(); mockfs(fakeFs_again); manually.

Actually a lot of the reason I wanted to do the PR was because bypass is extremely helpful. If someone is pre-loading a large amount of data into the file-system, it's a hassle and can get expensive to re-create it all from scratch when I just need to do a quick in-line read from the real FS.

I highly recommend we keep it and just let people know that it's intended for synchronous calls.

[nonara]

A good thing to keep in mind here is that because we now support lazy-loading, if you unmock, all your lazy load data gets wiped, which is certainly not ideal for tests situations where things need to be fast.

I think the suggestion of adding the advanced features disable and enable + clarifying documentation for bypass is clear and supports a wide range of use cases.

I'm not seeing a downside to adding the functionality, but I'm open to discussion if you think that there are some?

[nonara]

See latest for new readme which clarifies bypass methods and provides advanced methodology for users who know what they're doing.

[nonara]

@tschaub Addressed your comments and made new modules.

[nonara]

Updated according to last review:

• Remove enable, disable from exports
• Add promise support to mock.bypass()
• Merge mapDir() and mapFile() functions to load()
• Remove mapPaths()
• Use path.basename (May affect cygwin, but Linux allows backslashes in filename, which takes precedence)
• Remove custom error message for non-string parameter to load()
• Updated tests to accommodate changes
• Updated readme according to suggestions

Note

mock.bypass() is built to work with Promises, however, async callback functions won't work. That was part of the idea behind enable, disable.

Is this acceptable? Would you like to alter the readme or handle it somehow?

[nonara]

Investigating the TypeError with Buffer in Windows Node 12, now. At a bit of a loss on this. I can't see how any changes would have affected that. Will debug offline.

[3cp]

mock.bypass() is built to work with Promises, however, async callback functions won't work. That was part of the idea behind enable, disable.

I am quite sure your new implementation supports await mock.bypass(async () => {...});, which you can add to unit tests. Async return value is a promise.

[nonara]

I am quite sure your new implementation supports await mock.bypass(async () => {...});, which you can add to unit tests. Async return value is a promise.

Should clarify - I'm referring to fs functions which utilize callbacks, like fs.stat(). Any further fs access within the callback would not work.

[nonara]

I just checked out the current source from upstream master and confirmed that the buffer TypeError is happening on Windows for Node 12.

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Buffer

[3cp]

For the buffer thing, can you find the exact line?

Nodejs fs APIs support string/buffer on path, for example in the doc:

fs.access(path[, mode], callback)[src]#

History
path <string> | <Buffer> | <URL>

But Nodejs path APIs only support string.

path.basename(path[, ext])#

History
path <string>

This means if you forward a path got from a fs API to a path API, you need to take care of the buffer thing. I believe you can find there are some treatments in the existing code.

[nonara]

It's in regard to fs.symlink() and is happening on master. I don't have a Windows Node 12 environment setup, I just created a new branch to test GH Actions to verify that is was not related to this PR

Error:

  1) fs.symlink(srcpath, dstpath, [type], callback)
       supports Buffer input:
     TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Buffer
      at validateString (internal/validators.js:120:11)
      at Object.isAbsolute (path.js:353:5)
      at preprocessSymlinkDestination (internal/fs/utils.js:317:18)
      at Object.symlink (fs.js:1073:19)
      at Context.<anonymous> (test\lib\fs.link-symlink.spec.js:192:8)
      at processImmediate (internal/timers.js:456:21)

Originating code:

https://github.com/tschaub/mock-fs/blob/master/test/lib/fs.link-symlink.spec.js#L191-L204

[3cp]

I see, could be a regression related to latest nodejs v12.18.3.

[3cp]

Confirmed it's a win32 only regression in latest nodejs v12, I will try to investigate.

I think we can ignore that failure in this PR.

[nonara]

@3cp How'd it go? Any luck on determining a solution?

[3cp]

I have not spent time on the win32 problem yet. Will do in coming week.

[nonara]

Thanks for this great contribution @nonara.

Sure thing! Thanks for taking the time to review.

[3cp]

The win32 issue is indeed a regression in nodejs code base. I will create a PR or issue there.

[3cp]

The win32 issue is fixed in nodejs, but not patched in v12 yet. https://github.com/nodejs/node/issues/34514

[harryhorton]

Any idea when this will be released? Working on a project that could really use it ❤️

[3cp]

There are some edge cases to be cleaned up (https://github.com/tschaub/mock-fs/pull/306#issuecomment-671129346). I will PR the fix shortly after merging #303.

[dqisme]

Hi, thanks for this wonderful upgrade when will we have this feature in the next release, please?

[frederikheld]

@nonara Many thanks for your effort! That's a great addition to mockFs that will make testing much easier :-)

[nonara]

@frederikheld You're very welcome. Thank you for the kind comment!