limetext / backend

Backend for LimeText
BSD 2-Clause "Simplified" License
538 stars 87 forks source link

Fix errors in packages tests #121

Closed fmanno closed 7 years ago

fmanno commented 8 years ago

[Linux/macOS] Fix data races: Added channel to watcher structure so that the watcher.Close function can safely wait until the goroutine is done and avoid data races that could occur with just the mutex locks.

[macOS] Fix json_test and watcher_test in packages: The way FSEvents handles events we need to use the flushDir function or the notify framework won't generate events for files if the directory had already files watched inside. The flushDir was being called at the end of the Watch method and therefore it wasn't executing in some cases where the method was returning before the call.

fmanno commented 8 years ago

Hopefully tests should all pass with these fixes in Linux and macOS and then we can keep them clean and I can start working in actually implementing some features.

fmanno commented 8 years ago

Ok... this didn't go as planned 👎 any idea why the linux build can be failing on this change and not in https://travis-ci.org/limetext/backend/jobs/172770715?

fmanno commented 8 years ago

I found a different approach to the race conditions and discovered that the kqueue backend for the notify library works better in macOS

ricochet1k commented 8 years ago

Interesting. I don't like the idea of being forced to use tags, we can almost install LimeText by just doing go get github.com/limetext/lime-qml... But I guess Mac users are going to need a different build step anyway. It might be a good idea to do OS detection on the tags to make it clear that they are only for Mac.

fmanno commented 8 years ago

When I get more time I will try to narrow down the problem so I can create an issue in the notify project to fix the FSevents implementation. The notify package already checks forbthe architecture and only uses the kqueue tag in darwin. But I agree it should be mote visible I'll try to sort out the OS checks in the Makefile this week.

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling c3302081ec43b86691d323d0f0195009d2306c7f on fmanno:fix-packages-tests into \ on limetext:master**.

fmanno commented 8 years ago

Last commit makes the tags only applied in case of Darwin systems (kqueue is the default otherwise even in mac systems)

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling 5b3e3598000875ef22ec2a39b9332735c228b0cf on fmanno:fix-packages-tests into \ on limetext:master**.