tschaub / mock-fs

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

3.12.0 breaks re-requires of native modules #178

Closed rosston closed 8 years ago

rosston commented 8 years ago

The require cache clearing in https://github.com/tschaub/mock-fs/pull/141 breaks re-requires of native modules (see https://github.com/nodejs/node/issues/6160). So if I have one test file that requires a native module, another test file that requires mock-fs, and a 3rd test file that requires the same native module; the final test file will crash because it cannot require the native module.

I have an (extremely-simplified) example here: https://github.com/appropos/mock-fs-native-module-bug

I originally planned to provide a failing test and working fix in a PR, but it's difficult to test. If I add an integration test that more or less does,

require('iconv');
require('../../lib/index');
require('iconv');

the test passes because the ../../lib/index require is cached, meaning the cache-clearing doesn't happen again. I tried explicitly clearing ../../lib/index from the require cache in my test, but that broke all other tests.

Tests or not, I imagine the fix is as simple as not clearing require.cache entries that end in .node.

EDIT: See https://github.com/tschaub/mock-fs/pull/180 for a test and fix.

tschaub commented 8 years ago

Thanks for the report and the proposed fix in #178. I'm inclined to revert #141 since that appears to have introduced other problems. I'll issue a patch release and then revisit the cache clearing issue (it may turn out to be the wrong solution altogether).

rosston commented 8 years ago

Thanks! #181 / 3.12.1 fix the issue.