search

tschaub / mock-fs

Configurable mock for the fs module
https://npmjs.org/package/mock-fs
Other
911 stars 86 forks source link

Fix tests on node 20 #387

Closed everett1992 closed 2 months ago

everett1992 commented 2 months ago

This includes @andrewnicols's changes from https://github.com/tschaub/mock-fs/pull/381

All unit tests pass on node 20 (and 22) I have not written any new tests, or checked that any new APIs work correctly. This at-least partially fixes https://github.com/tschaub/mock-fs/issues/384. I tested these changes in an application that was failing with mock-fs 5.2.0 and node 20

tschaub commented 2 months ago

Thanks, @everett1992!

Could you add 20 to the list of node versions here: https://github.com/tschaub/mock-fs/blob/b0808c5ac2de4c88facaddd8e6d5745d250784c2/.github/workflows/test.yml#L26-L28

And then the readme can be updated here: https://github.com/tschaub/mock-fs/blob/ea6e334a9b6a2423a5c620989b9778d33a37a1c5/readme.md?plain=1#L273

It looks like tests also pass on Node 22. If you find the same, we could add 22 to the list as well.

tschaub commented 2 months ago

Oh, I forgot about all the ignored tests. So not right to imply that all "tests pass" on Node 20 and 22.

Do you know if there are tests that are currently ignored that would pass with your changes?

everett1992 commented 2 months ago

Doesn't look like I fixed any of those

1) The API
       mock.fs()
         generates a mock fs module with a mock file system:
     TypeError: mock.fs is not a function
      at Context.<anonymous> (test/lib/index.spec.js:350:27)
      at process.processImmediate (node:internal/timers:491:21)

  2) The API
       mock.fs()
         passes options to the FileSystem constructor:
     TypeError: mock.fs is not a function
      at Context.<anonymous> (test/lib/index.spec.js:361:27)
      at process.processImmediate (node:internal/timers:491:21)

  3) The API
       mock.fs()
         accepts an arbitrary nesting of files and directories:
     TypeError: mock.fs is not a function
      at Context.<anonymous> (test/lib/index.spec.js:376:27)
      at process.processImmediate (node:internal/timers:491:21)
everett1992 commented 2 months ago

I think those disabled mock.fs tests should be removed, since mock.fs was removed in v4

Upgrading to version 4 The mock.fs() function has been removed. This returned an object with fs-like methods without overriding the built-in fs module.

https://github.com/tschaub/mock-fs/blob/main/changelog.md#400

tschaub commented 2 months ago

I think those disabled mock.fs tests should be removed, since mock.fs was removed in v4

Thanks for the reminder! Those tests could have been removed instead of ignored in https://github.com/tschaub/mock-fs/pull/182/files#diff-e78e0b8d4a6016fffbd3160920548d5860afed84b611d22f4f6e0c42609d7ce6.

tschaub commented 2 months ago

Thank you @everett1992 and @andrewnicols!