Skip to content
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

Tests on OSX. #27

Merged
merged 4 commits into from
Sep 7, 2015
Merged

Tests on OSX. #27

merged 4 commits into from
Sep 7, 2015

Conversation

octplane
Copy link
Contributor

Tests now pass on OSX (necessary to reproduce #23 ).

  • add OS conditionals for full path resolving (FSEvent API does send full resolved path to its callback), code courtesy of @jakerr.
  • added some timeout during complex tests: FSEvent API sends past events to its listener, no matter what. The only solution is to wait.
  • use @jakerr validate_recv method to match results in test cases and timeout (solves Tests hang if no events are received #26 ).
  • enhance validate_recv to support aggregated events: op::CREATE|op::REMOVE will match [op::CREATE, op::REMOVE] (useful when your observing api can split or merge nearby events). Fixes a huge issue for OSX testing, has probably no impact on Linux.
  • pollwatch test is now broken on OSX because of this full path stuff (worth another bug ? sibling of Errors when watcher backend is given broken symlinks #21 ?)
  • untested on linux. If the timeouts are too annoying, I still can refactor these to make runs faster on linux.

@passcod
Copy link
Member

passcod commented Sep 5, 2015

Back from holiday! This looks awesome :) I'll run/test on Linux and merge review.

latency: 0.1,
flags: fs::kFSEventStreamCreateFlagFileEvents,
latency: 0.0,
flags: fs::kFSEventStreamCreateFlagFileEvents | fs::kFSEventStreamCreateFlagNoDefer,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, can you explain what that change does?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defer will wait several event to aggregate them and deliver to the callback. This can mess tests (at least) and this seems to be a reasonable default, until somebody tells us this messes things. Same goes with latency.

@passcod
Copy link
Member

passcod commented Sep 5, 2015

We also need to have @jakerr added to the Cargo.toml.

@passcod
Copy link
Member

passcod commented Sep 5, 2015

And finally, could you rebase on top of master? I've added OS X Travis CI testing, let's see what that does.

passcod added a commit that referenced this pull request Sep 7, 2015
@passcod passcod merged commit c310e5f into notify-rs:master Sep 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants