Migrating to Asynchronous Mocking for Tests

[nelsonni]

Describe the bug
mock-fs is the standard library for mocking a filesystem in JavaScript/TypeScript. However, it has limitations based on how it patches the fs module by binding to the underlying file system. The README includes the following warning:

Asynchronous calls are supported, however, they are not recommended as they could produce unintended consequences if anything else tries to access the mocked filesystem before they've completed.

As @maxwellgerber commented on Oct 18, 2018:

mock-fs works by overloading Node's process.bindings for the internal C++ FS module. So, you call require('fs').someMethod and that someMethod calls process.bindings('fs').someMethodInternal in turn. After a mock is spun up, the fs module calls into mock-fs instead of the C++ code, allowing us to alter the behavior. Because the lib is tightly coupled to internal APIs, there's no guarantee that it will continue to function on new versions.

[NOTE: This is being phased out entirely in Node 11 due to the deprecation of bindings]

The fs.promises module isn't a wrapper around the original fs module. It does its own thing, and interacts with the underlying process bindings differently. The lib hasn't been updated to handle this new behavior, which is why people get errors when using fs.promises but not with util.promisify(fs.someFunction).

You can see the javascript-land code for fs.promises here.

The biggest change is that there's a symbol passed in to the callback param of the Binding function called kUsePromises. Conceivably, we could check for that symbol inside mock-fs here and respond appropriately. I haven't spent enough time on it to say whether or not some other issues might come up.

Additionally, jest only supports asynchronous code when it is wrapped appropriately. It even includes the following warning:

Tests must be defined synchronously for Jest to be able to collect your tests.
...
When Jest runs your test to collect the tests it will not find any because we have set the definition to happen asynchronously on the next tick of the event loop.

Note: This means when you are using test.each you cannot set the table asynchronously within a beforeEach / beforeAll.

We have also been seeing some flaky tests (particularly on IO-bound files such as io.ts) which include [Error: ENXIO: no such device or address, read] and [Error: EBADF: bad file descriptor, read] when mocked files are reset before an asynchronous IO-bound operation can return a response.

Versions (please complete the following information):

• OS: Linux, MacOS, Windows
• Synectic Version: 1.0.0-beta (branch: development)

Additional context
memfs was examined as a possible replacement for mock-fs, but appears to also thinly support asynchronous IO-bound operations and therefore results in the same errors.

Following from the blog post, How to Write File-Based JavaScript Tests With Real Files, which introduces with-local-tmp-dir for creating temporary testing directories, output-files for creating multiple files at once, and endent for declaring multi-line strings, we will do the following:

• Install tmp-promise which is used by with-local-tmp-dir under the hood.
• Implement a mock-fs-promise module that uses tmp-promise in order to create temporary files that exist in the current filesystem, but with an API that mimics that of mock-fs.
• Replace all mock-fs code with mock-fs-promise code in the test suite.
• Uninstall mock-fs, @types/mock-fs.

Note: As a bonus, with-local-tmp-dir includes brief documentation on how to used it for Git-Based Tests. We can replicate this type of usage pattern for testing any git functionality in Synectic.

[nelsonni]

2a2ea17 on branch feature/mock-fs-promise specifically addresses the second step (implementing mock-fs-promise).

[nelsonni]

GitHub Actions job run 3205998526 on #501 returns:

●  Cannot log after tests are done. Did you forget to wait for something async in your test?
    Attempted to log "Warning: An update to GitConfigForm inside a test was not wrapped in act(...).

    When testing, code that causes React state updates should be wrapped into act(...):

    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */

    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act
        at GitConfigForm (/home/runner/work/synectic/synectic/src/components/GitConfigForm.tsx:53:51)
        at div
        at div
        at div
        at Paper (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Paper/Paper.js:55:23)
        at WithStyles(ForwardRef(Paper)) (/home/runner/work/synectic/synectic/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
        at div
        at Transition (/home/runner/work/synectic/synectic/node_modules/react-transition-group/cjs/Transition.js:133:30)
        at Fade (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Fade/Fade.js:50:24)
        at Unstable_TrapFocus (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Unstable_TrapFocus/Unstable_TrapFocus.js:30:24)
        at div
        at Portal (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Portal/Portal.js:39:24)
        at Modal (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Modal/Modal.js:94:36)
        at Dialog (/home/runner/work/synectic/synectic/node_modules/@material-ui/core/Dialog/Dialog.js:184:29)
        at WithStyles(ForwardRef(Dialog)) (/home/runner/work/synectic/synectic/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
        at MergeDialog (/home/runner/work/synectic/synectic/src/components/MergeDialog.tsx:57:21)
        at ModalComponent (/home/runner/work/synectic/synectic/src/components/ModalComponent.tsx:14:19)
        at div
        at CanvasComponent (/home/runner/work/synectic/synectic/src/components/CanvasComponent.tsx:35:33)
        at DndProvider (/home/runner/work/synectic/synectic/node_modules/react-dnd/dist/cjs/core/DndProvider.js:39:23)
        at TestContextWrapper (/home/runner/work/synectic/synectic/node_modules/react-dnd-test-utils/dist/cjs/wrappers.js:83:7)
        at ForwardedTestContextWrapper
        at Provider (/home/runner/work/synectic/synectic/node_modules/react-redux/lib/components/Provider.js:21:20)".

      51 |   const isUpdateReady = () => (isEmail(email) && (username != existingUsername || email != existingEmail));
      52 |   const usernameChange = (event: React.ChangeEvent<{ value: string }>) => setUsername(event.target.value);
    > 53 |   const emailChange = (event: React.ChangeEvent<{ value: string }>) => setEmail(event.target.value);
         |                                                   ^
      54 |
      55 |   const setConfigs = async () => {
      56 |     console.log(`updating git-config values: username => ${username}, email => ${email}`);

      at GitConfigForm (src/components/GitConfigForm.tsx:53:51)
          at div
          at div
          at div
      at Paper (node_modules/@material-ui/core/Paper/Paper.js:55:23)
      at WithStyles(ForwardRef(Paper)) (node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
          at div
      at Transition (node_modules/react-transition-group/cjs/Transition.js:133:30)
      at Fade (node_modules/@material-ui/core/Fade/Fade.js:50:24)
      at Unstable_TrapFocus (node_modules/@material-ui/core/Unstable_TrapFocus/Unstable_TrapFocus.js:30:24)
          at div
      at Portal (node_modules/@material-ui/core/Portal/Portal.js:39:24)
      at Modal (node_modules/@material-ui/core/Modal/Modal.js:94:36)
      at Dialog (node_modules/@material-ui/core/Dialog/Dialog.js:184:29)
      at WithStyles(ForwardRef(Dialog)) (node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
      at MergeDialog (src/components/MergeDialog.tsx:57:21)
      at ModalComponent (src/components/ModalComponent.tsx:14:19)
          at div
      at CanvasComponent (src/components/CanvasComponent.tsx:35:33)
      at DndProvider (node_modules/react-dnd/dist/cjs/core/DndProvider.js:39:23)
      at TestContextWrapper (node_modules/react-dnd-test-utils/dist/cjs/wrappers.js:83:7)
          at ForwardedTestContextWrapper
      at Provider (node_modules/react-redux/lib/components/Provider.js:21:20)".
      at console.error (node_modules/@jest/console/build/BufferedConsole.js:163:10)
      at printWarning (node_modules/react-dom/cjs/react-dom.development.js:67:30)
      at error (node_modules/react-dom/cjs/react-dom.development.js:43:5)
      at warnIfNotCurrentlyActingUpdatesInDEV (node_modules/react-dom/cjs/react-dom.development.js:24064:9)
      at dispatchAction (node_modules/react-dom/cjs/react-dom.development.js:16135:9)
      at getConfigs (src/components/GitConfigForm.tsx:66:13)

This is caused by CanvasComponent.spec.tsx, which uses ReduxStore.testStore and includes a type: 'MergeSelector' modal. Thus CanvasComponent attempts to load GitConfigForm.tsx, which is attempting to read the global git-config settings provided in the new mock-fs-promise fileysystem. Since mock-fs-promise does not create a new temporary filesystem, which is how mock-fs operates, we are erroneously attempting to read from files outside of the root temporary directory created by mock-fs-promise (thus the error message is thrown).

[nelsonni]

Resolved by #521.