Pull request #378 + test

[erezarnon]

If we add a watch on directory A, mkdir A/B, rm -r A/B, mkdir A/B,
mkdir A/B/C, no event comes from fs.watch on creation of A/B/C

Encountered on Ubuntu 14.04.3 LTS, Linux 3.16.0-50-generic x86_64

[es128]

Is there really a need for a delay between these two mkdirSyncs?

[erezarnon]

Yes, probably because the timeout below needs to be more than 900ms, which is the max that d() allows.

[es128]

Those values are fairly arbitrary - based on what seemed to work consistently for the other tests. You're free to use setTimeout directly if it works out more cleanly for this particular situation.

[erezarnon]

Added a test to demonstrate.

This fix doesn't make sense to me. Look at what the _remove and unwatch methods do, and see what's already happening a few lines higher up than this patch.

Are you referring to parent.remove(item)? It doesn't call _closers(path), so FsWatchInstances isn't cleared, so the new directory doesn't get a new fs.watch instance. If you tell me the correct way to fix, I can make the change.

[es128]

This assertion should move up to happen right after the directory is removed, and the recreation to happen within that callback after this assertion has passed.

[es128]

It doesn't call _closers(path), so FsWatchInstances isn't cleared

Ah, ok. I'll try to evaluate more closely to see if I can identify a better alternative than this approach. It is causing a regression on another test, so we'll have to figure out how to correct that.

[es128]

Your own test is actually failing as well currently.

Thanks for the new PR, though. Provides a much better basis from which to confirm the problem and work on a fix.

Btw your commits are not lining up with your github account. See https://help.github.com/articles/setting-your-email-in-git/

[erezarnon]

Yeah, my laptop is configured for work, I'll change that. Now that I'm playing around with the test, I get intermittent failures, too. Probably need a longer timeout.

[es128]

Already have a lot of delays... Kinda scary that it would need even more for this to work consistently.

But I guess yeah let's do whatever it takes to get it working first, then we can look for opportunities to optimize.

[erezarnon]

Ok, all tests are passing and the commits have my name on them. I still have 2 tests failing locally:

 1) chokidar polling watch options awaitWriteFinish should emit change event after the file is fully written:
     Error: timeout of 5000ms exceeded. Ensure the done() callback is being called in this test.

  2) chokidar polling watch options awaitWriteFinish should not raise any event for a file that was deleted before fully written:
     Uncaught expected spy to have been called with arguments /home/erez/workspace/chokidar/test-fixtures/late-change.txt
  AssertionError: expected spy to have been called with arguments test-fixtures/late-change.txt
      at test.js:1372:46
      at finish (test.js:90:7)

But they don't fail if I run them individually or via _mocha --grep "polling watch options awaitWriteFinish" only via _mocha --grep "polling watch options" So looks like a dependency in the tests.

[es128]

Yeah don't worry about those two. I need to do more work to stabilize them.

[erezarnon]

Bad response: 422 {"message":"Couldn't find a repository matching this job.","error":true} on one of the build jobs. Is that me?

[es128]

No, that's just coveralls being flaky. Don't worry about it.

[es128]

Cool. Appreciate your persistence on this. Thanks.