tschaub / mock-fs

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

ENOENT when setting fs.watch #246

Closed cburroughs closed 6 years ago

cburroughs commented 6 years ago
'use strict';

const fs = require('fs');
const mockfs = require('mock-fs');

mockfs({'/foo': {'a': '', 'b': '', 'c': ''}});
console.log('dir contents',  fs.readdirSync('/foo'));
fs.watch('/foo', {}, function _onDirEvent(evt) {
    console.log('evt', evt);
});
$ node foo.js 
dir contents [ 'a', 'b', 'c' ]
fs.js:1236
    throw error;
    ^

Error: watch /foo ENOENT

I could not find fs.watch defined anywhere in mock-fs, but also saw that #207 removed a caveat about fs.watch not being implemented. I'm not sure if I'm missing something obvious or if fs.watch is unsupported.

maxwellgerber commented 6 years ago

Nope, not supported yet. fs.watch uses FSEvent Event Emitters, which look like they might be out of scope of this lib. fs.watchFile uses StatWatchers - which haven't been implemented yet : https://github.com/tschaub/mock-fs/blob/28d65a7cca6d80745cc27fafc1b10f4de69084f0/lib/binding.js#L1214-L1218

tschaub commented 6 years ago

Yes. Not yet implemented. This is the first I've seen of a request for it, so it is hard to say how widely used it would be. But if anyone is up for implementing it, a well tested pull request would be welcome.

dpryden commented 4 years ago

For what it's worth, I had a use case for fs.watch, so I took a stab at putting together a pull request. It turns out to be much more complicated than it first appears, since the FSEvent class is in a separate binding,fs_event_wrap. The way this binding is loaded is different in different versions of Node. In Node 10+ it's lazily loaded so it is possible to replace it with a proxy that can be mocked/unmocked like the other binding dependencies. However, in earlier Node versions, the FSEvent function object is imported eagerly by the fs module and I don't see an easy way to replace it, so perhaps we would additionally need to monkeypatch fs.watch on earlier Node versions?

In any case, I've decided not to invest any more time in this right now, unfortunately. I'm going to rewrite the affected tests in my project instead.

However, if anyone finds it useful, I've pushed up a PR with my changes, which do work on Node 10 and Node 12 (and have a couple of small tests demonstrating that). See #300 (Edit: new PR is #308 )

BadIdeaException commented 2 years ago

Any news on this? At least as an optional functionality on Node 10+?

On a related note, might want to add the caveat about it being unsupported back into the readme. It took me a good while to find out that this is unsupported, and not just me being too dumb to use it right. ;)