Helper to bypass mock FS & expose real files/directories #304

nonara · 2020-07-27T04:42:22Z

Adds two new features:

mock.bypass() - Allows executing commands against the actual FS
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 · 2020-07-27T05:33:00Z

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!

- Lazy-loaded files were not able to be written to if they hadn't been read yet - Descriptors defaulted to writable: false after setting value

- Included mock.createDirectoryInfoFromPaths() - Included mock.bypass()

…tes, etc)

lib/filesystem.js

nonara · 2020-07-27T21:04:26Z

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

lib/index.js

readme.md

- Added mock.mapFile(), mock.mapDir(), and mock.mapPaths() - Created improved tests for the new methods (comparing stats with real files, etc)

nonara · 2020-07-28T05:07:53Z

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.

Just to be safe

nonara · 2020-08-04T04:07:17Z

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 · 2020-08-04T04:10:05Z

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 · 2020-08-04T04:16:35Z

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.

lib/bypass.js

nonara · 2020-08-04T04:28:59Z

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 · 2020-08-04T04:31:19Z

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 · 2020-08-04T04:37:34Z

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 · 2020-08-04T04:49:23Z

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 · 2020-08-04T06:04:07Z

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

3cp · 2020-08-04T07:33:14Z

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 · 2020-08-09T03:15:03Z

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

3cp · 2020-08-09T03:21:58Z

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

tschaub

Thanks for this great contribution @nonara.

I'll follow up with a separate pull request to address the additional comments.

tschaub · 2020-08-09T21:59:52Z

lib/bypass.js

+    const res = fn();
+
+    // Handle promise return
+    if (typeof res.then === 'function') {


This will throw a TypeError if fn returns null or undefined.

tschaub · 2020-08-09T22:00:39Z

lib/bypass.js

+    // Handle promise return
+    if (typeof res.then === 'function') {
+      res.then(exports.enable);
+      res.catch(exports.enable);


This would be better handled with res.finally(exports.enable).

tschaub · 2020-08-09T22:01:43Z

lib/bypass.js

+    return res;
+  } catch (e) {
+    exports.enable();
+    throw e;


This could be in a finally block.

tschaub · 2020-08-09T22:11:25Z

readme.md

+
+| Option    | Type    | Default | Description |
+| --------- | ------- | ------- | ------------
+| lazyLoad  | boolean | true    | File content isn't loaded until explicitly read


I think this could just be called lazy - avoiding a bit of repetition: load(dir, {lazy: true}).

nonara · 2020-08-09T22:16:39Z

Thanks for this great contribution @nonara.

Sure thing! Thanks for taking the time to review.

3cp · 2020-08-09T23:21:47Z

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

3cp · 2020-08-09T23:28:51Z

The win32 issue is fixed in nodejs, but not patched in v12 yet. nodejs/node#34514

harryhorton · 2020-08-10T19:53:10Z

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

3cp · 2020-08-10T23:26:59Z

There are some edge cases to be cleaned up (#306 (comment)). I will PR the fix shortly after merging #303.

dqisme · 2020-08-19T09:04:40Z

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

…_modules now that tschaub/mock-fs#304 is available

…ules now that tschaub/mock-fs#304 is available

frederikheld · 2021-01-03T17:15:39Z

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

nonara · 2021-01-03T22:23:05Z

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

nonara added 2 commits July 27, 2020 00:27

Added mock.bypass() and mock.createDirectoryInfoFromPaths()

26bfdf7

Fix broken xdescribe

be7bf43

nonara mentioned this pull request Jul 27, 2020

mock.restore() leaks (no garbage collection) #302

Closed

nonara added 4 commits July 27, 2020 00:52

Node 6 support

a39712e

Investigate missing entry (squash)

266569b

Fixed + improved test

95d3ff2

Added normalize path

34a4503

nonara added 6 commits July 27, 2020 01:36

Added coverage for non-string error

91ab56a

Remove artifact

fdeed02

prettier

0ad9857

LazyLoad Fixes

c3eddcb

- Lazy-loaded files were not able to be written to if they hadn't been read yet - Descriptors defaulted to writable: false after setting value

Updated readme

decb5de

- Included mock.createDirectoryInfoFromPaths() - Included mock.bypass()

Make automatically created directories inherit stats (permissions, da…

135ec12

…tes, etc)

nonara commented Jul 27, 2020

View reviewed changes

lib/filesystem.js Show resolved Hide resolved

This was referenced Jul 27, 2020

Mount a path to a real directory in the file system? #62

Closed

Feature-Request: support/instrument lazy requires #213

Closed

nonara added 2 commits July 27, 2020 17:15

Move badge to top of readme

61b8ac1

Fix: Make non-lazy loaded files retain stats

b195d8e

3cp reviewed Jul 27, 2020

View reviewed changes

lib/index.js Outdated Show resolved Hide resolved

3cp reviewed Jul 27, 2020

View reviewed changes

readme.md Outdated Show resolved Hide resolved

nonara added 3 commits July 27, 2020 19:08

Prefer existing permissions const

3b58f79

Re-engineered API

6950d39

- Added mock.mapFile(), mock.mapDir(), and mock.mapPaths() - Created improved tests for the new methods (comparing stats with real files, etc)

Update readme

9f284af

nonara added 3 commits July 28, 2020 01:18

Added tests for integration with mock() & fs.readFileSync

562c6ae

Just to be safe

Correct typo

2ca7d0e

Fix: fixWin32Permissions missing platform check

10f61da

nonara changed the title ~~mock.bypass, mock.createDirectoryInfoFromPaths, lazyLoading~~ Helper to bypass mock FS & add expose real files/directories Jul 28, 2020

nonara added 2 commits August 3, 2020 23:40

Prettier correction

0c7bbbb

Add withPromise to test

f431af2

3cp reviewed Aug 4, 2020

View reviewed changes

lib/bypass.js Outdated Show resolved Hide resolved

Changed to typeof function check

768224f

nonara requested a review from tschaub August 5, 2020 19:00

tschaub approved these changes Aug 9, 2020

View reviewed changes

tschaub merged commit ee50c67 into tschaub:master Aug 9, 2020

tschaub mentioned this pull request Aug 9, 2020

Bypass updates #306

Merged

travi added a commit to form8ion/project that referenced this pull request Aug 22, 2020

test(integration): simplified the stubbing of the filesystem for node…

8dbd780

…_modules now that tschaub/mock-fs#304 is available

travi added a commit to form8ion/project that referenced this pull request Aug 22, 2020

test(integration): simplified the stubbing of the filesystem for node…

0513568

…_modules now that tschaub/mock-fs#304 is available

travi added a commit to form8ion/javascript-scaffolder that referenced this pull request Aug 23, 2020

test(integration): simplified stubbing of the filesystem for node mod…

2aa5a84

…ules now that tschaub/mock-fs#304 is available

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Helper to bypass mock FS & expose real files/directories #304

Helper to bypass mock FS & expose real files/directories #304

nonara commented Jul 27, 2020 •

edited

Loading

nonara commented Jul 27, 2020

nonara commented Jul 27, 2020 •

edited

Loading

nonara commented Jul 28, 2020 •

edited

Loading

nonara commented Aug 4, 2020

nonara commented Aug 4, 2020

3cp commented Aug 4, 2020 •

edited

Loading

nonara commented Aug 4, 2020

nonara commented Aug 4, 2020

3cp commented Aug 4, 2020

nonara commented Aug 4, 2020 •

edited

Loading

3cp commented Aug 4, 2020

3cp commented Aug 4, 2020

nonara commented Aug 9, 2020

3cp commented Aug 9, 2020

tschaub left a comment

tschaub Aug 9, 2020

tschaub Aug 9, 2020

tschaub Aug 9, 2020

tschaub Aug 9, 2020

nonara commented Aug 9, 2020

3cp commented Aug 9, 2020

3cp commented Aug 9, 2020

harryhorton commented Aug 10, 2020

3cp commented Aug 10, 2020

dqisme commented Aug 19, 2020

frederikheld commented Jan 3, 2021

nonara commented Jan 3, 2021

Helper to bypass mock FS & expose real files/directories #304

Helper to bypass mock FS & expose real files/directories #304

Conversation

nonara commented Jul 27, 2020 • edited Loading

nonara commented Jul 27, 2020

nonara commented Jul 27, 2020 • edited Loading

nonara commented Jul 28, 2020 • edited Loading

nonara commented Aug 4, 2020

Updated according to last review:

Note

nonara commented Aug 4, 2020

3cp commented Aug 4, 2020 • edited Loading

nonara commented Aug 4, 2020

nonara commented Aug 4, 2020

3cp commented Aug 4, 2020

nonara commented Aug 4, 2020 • edited Loading

3cp commented Aug 4, 2020

3cp commented Aug 4, 2020

nonara commented Aug 9, 2020

3cp commented Aug 9, 2020

tschaub left a comment

Choose a reason for hiding this comment

tschaub Aug 9, 2020

Choose a reason for hiding this comment

tschaub Aug 9, 2020

Choose a reason for hiding this comment

tschaub Aug 9, 2020

Choose a reason for hiding this comment

tschaub Aug 9, 2020

Choose a reason for hiding this comment

nonara commented Aug 9, 2020

3cp commented Aug 9, 2020

3cp commented Aug 9, 2020

harryhorton commented Aug 10, 2020

3cp commented Aug 10, 2020

dqisme commented Aug 19, 2020

frederikheld commented Jan 3, 2021

nonara commented Jan 3, 2021

nonara commented Jul 27, 2020 •

edited

Loading

nonara commented Jul 27, 2020 •

edited

Loading

nonara commented Jul 28, 2020 •

edited

Loading

3cp commented Aug 4, 2020 •

edited

Loading

nonara commented Aug 4, 2020 •

edited

Loading