-
Notifications
You must be signed in to change notification settings - Fork 51
Test cases #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test cases #64
Conversation
# Conflicts: # fsevents_test.go
It seems the main bit that's missing here is that it doesn't actually print and parse the list of events? It seems always 0:
Here's what I would do:
You can also use a different approach if that works better for you, but the end result should be something that has tests for at least a bunch of common situations: "make a new directory", "remove watched directory", etc. etc. |
The tests are based on on the behaviour of a singular watcher across paths. fsevents event streams are simply not used in such a manner - they natively work recursively. There is no need to test for nested path handling.
The test runs inconsistently. At times fsevents reports all the delete events, at other times it only reports the root directory remove. Removing for now.
941c937
to
8ddeb19
Compare
Added some tests now. The tests that are here run and pass (locally). Behaviour is different from fsnotify, and I've mostly just replaced the test asserts with what is happening. These aren't really correctness tests at this point, just regression. I also haven't really had anything to compare against, as there are not too many other tests here. That is something to keep in mind. I was watching what was happening for each test though, and the events emitted do make sense with regards to what the test does, and what I could find on how fsevents work on mac os. |
Yeah, that's fine. The important bit here is to ensure the behaviour doesn't radically change and that it doesn't panic. Also, can you add a test like so (completely untested code for illustrative purposes):
That is, create 100 files and directories simultaneously in a watched directory, then remove them, and ensure we've got the right events. After that and the minor comments it should be good to go. |
Test is being really flaky when using the number of events as the boundary. The number of events emitted is highly variable because the events are batched by the os. In the end, we verify correctness using emitted flags regardless.
Alright, I think this is mostly it. I also wrote a short description of the test scripts in the readme. I think I went through all your PR comments, also cleaned up some comments in code to remove stuff that was made irrelevant by being different from initial fsnotify code. Let me know if I've missed something. |
Seems good; thanks! |
#62 (comment)
PR to tests fsnotify style test scripts branched the main branch, as discussed in link above.