--watch crashes mocha on startup

[villesau]

I'm getting error below when running mocha in watch mode. As you can see, the file it tries to find is not even js file. Apparently the file link which breaks the mocha is somehow broken. It shouldn't break the mocha though. Without --watch, mocha runs just fine. i'm running mocha version 3.1.2 with following command from package.json:
"test-rc": "mocha 'frontend/*/.spec.js' --watch"

The error is following:

Error: ENOENT: no such file or directory, stat '/Users/me/code/my-project/public/index-apitest.php'
at Error (native)
at fs.statSync (fs.js:987:18)
at /Users/me/code/my-project/node_modules/mocha/lib/utils.js:255:11
at Array.forEach (native)
at Object.exports.files (/Users/me/code/my-project/node_modules/mocha/lib/utils.js:253:6)
at /Users/me/code/my-project/node_modules/mocha/lib/utils.js:256:17
at Array.forEach (native)
at Object.exports.files (/Users/me/code/my-project/node_modules/mocha/lib/utils.js:253:6)
at Object. (/Users/me/code/my-project/node_modules/mocha/bin/_mocha:408:26)
at Module._compile (module.js:556:32)
at Object.Module._extensions..js (module.js:565:10)
at Module.load (module.js:473:32)
at tryModuleLoad (module.js:432:12)
at Function.Module._load (module.js:424:3)
at Module.runMain (module.js:590:10)
at run (bootstrap_node.js:394:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:509:3

[villesau]

if following would be wrapped to try-catch, things does not break anymore:

https://github.com/mochajs/mocha/blob/master/lib/utils.js#L255

Would it be feasible solution?

[ScottFreeCode]

Simply ignoring errors at that point isn't likely to be correct behavior -- in this case, as far as I can tell that function shouldn't even be getting paths for files that don't exist, so there's almost certainly something else going wrong that should be addressed; more importantly, in general, suppressing errors could hide and/or worsen other problems.

Any idea where Mocha is getting the path public/index-apitest.php from? Do you have a mocha.opts file?

[villesau]

no idea. I'm not very familiar with mocha architecture, and not aware where that function is called. Those non-existing files are broken links. We have vagrant system which creates some links, totally non-related to our frontend, and they are broken on host machine where we handle testing the fe. It seems that mocha is going trough every folder in the project, even though it is pointed to look only at the frontend/ folder. Changing non-matching file does not rerun the test suite though. Same behaviour is in the version 2.5.0

[villesau]

It also seems that ignoring file system errors is pretty common pattern all over the mocha code base. should they be ignored in the place i pointed out too? At least non-existing(ENOENT) files.

[villesau]

I Probably identified the problem (a line which calls the files -function)
https://github.com/mochajs/mocha/blob/master/bin/_mocha#L408

This line should most likely either use variable files, OR lookupFiles function from here: https://github.com/mochajs/mocha/blob/master/bin/_mocha#L364

Maybe someone more familiar with mocha could validate if i'm correct? Both functions (files and lookupFiles) apparently returns a list of file names with a difference that lookupFiles seems more robust.

edit: Apparently solution i found ignores regular .js (not matching the regex/path given as parameter) files from watch which is bad. Thus i think try/catch would be good enough solution.

[ScottFreeCode]

Ah, I misread the files function looking at that first line you pointed out; I thought the line getting the error had been given a list of files, but it's actually part of finding the list of files based on looking at the directory recursively and then grabbing only the ones with the right file extensions. That explains why it was looking at the php file at all. So that definitely needs to be fixed.

I'm not sure I'd describe lookupFiles as more robust -- between the fact that it's trying to handle at least three different usage methods (globbing, non-recursive directory/file specification and recursive directory specification) and the fact that it's basically written to silently ignore just about any error interacting with the filesystem at all, I worry that it's already bugged.

But more practically, it doesn't look like lookupFiles is ignoring node_modules and .git the way files is, so I don't think we can just switch over to that... Maybe in the next backwards-incompatible release we could if we made lookupFiles ignore node_modules and .git like files does, because that would also make it much easier to use globbing while avoiding dependencies' test files (which comes up occassionally). Otherwise or until then, I guess we're just going to have to do something to get files to avoid blowing up from broken symlinks.

I suppose suppressing errors the way lookupFiles is doing would technically do that, although I'm still rather wary of it -- it'd probably make the two parts more consistently reliable or unreliable, but I'm not so sure which... Maybe as a more robust alternative we could rewrite files to use globbing (directly)?

[villesau]

i think it would maybe also make sense to just go trough directories & files only once, and then subsetting them as needed (globbing etc). Mochas startup was noticeably faster when i just used same set of files than few lines above, returned by lookupFiles. This also would make files() redundant, but lookupFiles should be splitted into smaller pieces.

My current fix for files() was like this:#2538
This limits the catch only to non-existing files. In future files and lookupFiles should imo use same logic.

[boneskull]

@villesau please see my comments in #2538

I'll tag this confirmed, because even though I haven't tried it, it's highly plausible. 😄