Batch changes (and remove options[:every])

[zachahn]

This removes the `every` option when initializing Filewatcher.

Though backwards incompatible, it's fairly simple to emulate the
previous behavior.

    filewatcher.watch do |changes|
      # to emulate every=true
      changes.each do |filename, event|
        # ...
      end

      # to emulate every=false
      filename, event = changes.first
      # ...
    end

The CLI option --every remains unchanged.

[AlexWayfer]

Looks good, thank you. I'll look into tests, they also looks correct.

[AlexWayfer]

Oh, I see errors like:

expected [{}].empty? to return true, got false

I think it's easy to fix.

Let me know when there will be complicate fails.

[AlexWayfer]

Tests should work better after #116 (the second commit), so I recommend you to update (rebase) your branch.

[zachahn]

Thanks for the review! I have a few questions, but I'll work on some of the comments you gave and push it up soon 😄

[zachahn]

Hm it looks like there's a flaky test here. I had the same issue locally, but I didn't look into it much.

Interestingly as I was running it repeatedly, I got a slightly different error.

D, [2020-04-20T17:33:49.370980 #67025] DEBUG -- : Thread #70136262500420 processed = []
#<Thread:0x00007f93be1fa708@/path/to/filewatcher/spec/spec_helper.rb:135 run> terminated with exception (report_on_exception is true):
Traceback (most recent call last):
	9: from /path/to/filewatcher/spec/spec_helper.rb:135:in `block in thread_initialize'
	8: from /path/to/filewatcher/spec/spec_helper.rb:140:in `setup_filewatcher'
	7: from /path/to/filewatcher/lib/filewatcher.rb:42:in `watch'
	6: from /path/to/filewatcher/lib/filewatcher/cycles.rb:14:in `main_cycle'
	5: from /path/to/filewatcher/lib/filewatcher/cycles.rb:30:in `watching_cycle'
	4: from /path/to/filewatcher/lib/filewatcher.rb:111:in `filesystem_updated?'
	3: from /path/to/filewatcher/lib/filewatcher.rb:104:in `mtime_snapshot'
	2: from /path/to/filewatcher/lib/filewatcher.rb:104:in `each'
	1: from /path/to/filewatcher/lib/filewatcher.rb:105:in `block in mtime_snapshot'
/path/to/filewatcher/lib/filewatcher.rb:105:in `mtime': No such file or directory @ rb_file_s_mtime - /path/to/filewatcher/spec/tmp/tmp_file.txt (Errno::ENOENT)

[AlexWayfer]

Interestingly as I was running it repeatedly, I got a slightly different error.

Oh, thank you. I usually see fails like this (from your branch): https://github.com/filewatcher/filewatcher/pull/115/checks?check_run_id=603280959

I'm trying to improve the situation in #119, but… with macOS, JRuby, CLI, files, threads, etc — it's very hard!

Sorry about this. I'll notice about updates.

[AlexWayfer]

@zachahn hello! Can you please rebase the branch? #119 and many other improvements of specs and CI was merged into master.

[AlexWayfer]

@zachahn sorry, but there are conflicts again… CI should pass faster and more accurate! Please, update the PR if you have a chance. I can try to help if you want.

[zachahn]

@AlexWayfer No worries! Rebased and pushed

[AlexWayfer]

Pretty nice PR, thank you. I'm waiting for your response about new debug method.

[zachahn]

@AlexWayfer Updated! I opted to use changes.inspect, but you're right, there was no difference in the output

[AlexWayfer]

@zachahn I see a fail with Ruby 2.5: https://cirrus-ci.com/task/5304049320853504?command=test#L1175

And it seems syntax error for this version, not something phantom. So, can you please fix it?

[zachahn]

Strange! The only thing I changed was that debugging thing, so I'm inclined to believe it's a flaky test. Did you try rerunning the test? (Here's a diff showing what I changed, but since I rebased I'm not sure how long this link will work https://github.com/filewatcher/filewatcher/compare/929c5da3..d9f8914c. And it looks like 929c5da passed for Ruby 2.5)

I see that the test expects one hash, but it received two hashes. I'm not sure if this is the best way, but perhaps the best way to proceed would be to update the test to merge all the hashes together with something like changes.reduce(:merge).

I won't have time to look at this for the next few days. I'd appreciate it if you have a chance to fix it! I think I checked the box that lets maintainers push onto this branch. But otherwise I'll take a look at it later this week.

[AlexWayfer]

Did you try rerunning the test?

No.

Now I've tried and it passes. Hm…

I see that the test expects one hash, but it received two hashes. I'm not sure if this is the best way, but perhaps the best way to proceed would be to update the test to merge all the hashes together with something like changes.reduce(:merge).

I've understood the reason: there were two Filewatcher's triggers instead of expected one. I think, it's maybe OK to do reduce(:merge), or even add a sleep. And… merge can silent other unusual errors. So, because there is one action, create of a file, I suggest to wait more time. Due to this was splitted into two hashes, increase the interval of Filewatcher instance for this test.

I won't have time to look at this for the next few days. I'd appreciate it if you have a chance to fix it! I think I checked the box that lets maintainers push onto this branch. But otherwise I'll take a look at it later this week.

OK, thank you for your work and messages. I've tried and pushed a commit to this PR, so let's see.

[AlexWayfer]

Now it looks correct and passing, thank you again for your contributions.

[zachahn]

Thank you for taking a look and fixing it! Happy to contribute, it was a pleasure :)

[AlexWayfer]

We forgot to update the README…