This repository has been archived by the owner on Jan 24, 2019. It is now read-only.
Provide graceful shutdown of file watcher in tests #93
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
May 13, 2015 18:02
This test failure from bitly#92 inspired this change: https://travis-ci.org/bitly/google_auth_proxy/jobs/62425336 2015/05/13 16:27:33 using authenticated emails file /tmp/test_auth_emails_952353477 2015/05/13 16:27:33 watching /tmp/test_auth_emails_952353477 for updates 2015/05/13 16:27:33 validating: is [email protected] valid? true 2015/05/13 16:27:33 watching interrupted on event: "/tmp/test_auth_emails_952353477": CHMOD 2015/05/13 16:27:33 watching resumed for /tmp/test_auth_emails_952353477 2015/05/13 16:27:33 reloading after event: "/tmp/test_auth_emails_952353477": CHMOD 2015/05/13 16:27:33 watching interrupted on event: "/tmp/test_auth_emails_952353477": REMOVE 2015/05/13 16:27:33 validating: is [email protected] valid? false 2015/05/13 16:27:33 watching resumed for /tmp/test_auth_emails_952353477 2015/05/13 16:27:33 reloading after event: "/tmp/test_auth_emails_952353477": REMOVE 2015/05/13 16:27:33 failed opening authenticated-emails-file="/tmp/test_auth_emails_952353477", open /tmp/test_auth_emails_952353477: no such file or directory I believe that what happened was that the call to reload the file after the second "reloading after event" lost the race when the test shut down and the file was removed. This change introduces a `done` channel that ensures outstanding actions complete and the watcher exits before the test removes the file.
These test failures from bitly#93 inspired this change: https://travis-ci.org/bitly/google_auth_proxy/jobs/62474406 https://travis-ci.org/bitly/google_auth_proxy/jobs/62474407 Both tests exhibited this pattern: 2015/05/13 22:10:54 validating: is [email protected] valid? false 2015/05/13 22:10:54 watching interrupted on event: "/tmp/test_auth_emails_300880185": CHMOD 2015/05/13 22:10:54 watching resumed for /tmp/test_auth_emails_300880185 2015/05/13 22:10:54 reloading after event: "/tmp/test_auth_emails_300880185": CHMOD panic: test timed out after 1m0s [snip] goroutine 175 [chan send]: github.com/bitly/google_auth_proxy.(*ValidatorTest).TearDown(0xc2080bc330) /home/travis/gopath/src/github.com/bitly/google_auth_proxy/validator_test.go:27 +0x43 github.com/bitly/google_auth_proxy.TestValidatorOverwriteEmailListViaRenameAndReplace(0xc2080f2480) /home/travis/gopath/src/github.com/bitly/google_auth_proxy/validator_watcher_test.go:103 +0x3b9 [snip] goroutine 177 [chan send]: github.com/bitly/google_auth_proxy.func·017() /home/travis/gopath/src/github.com/bitly/google_auth_proxy/validator_test.go:34 +0x41 I realized that the spurious CHMOD events were causing calls to `func() { updated <- true }` (from validator_test.go:34), which caused the goroutine running the watcher to block. At the same time, ValidatorTest.TearDown was blocked by trying to send into the `done` channel. The solution was to create a flag that ensured only one value was ever sent into the update channel.
|
OK, now both of these commits combined seemed to do the trick. ;-) |
TestValidatorOverwriteEmailListViaRenameAndReplace was deadlocking on Windows because, on Windows, fsnotify.Watcher will continue to watch a renamed file using its new name. On other systems, it appears the watch on a file is removed after a rename. The fix is to explicitly remove the watch to ensure the watch is resumed under the original name.
|
Ah, Windows. Turns out it keeps watching a file even after a rename, unlike the Unix-based systems. With this third commit, it seems everybody's happy now. |
|
Ping... Kinda hoping to get rid of this flakiness I caused... Reputation to uphold, and all. :-) |
jehiah
added a commit
that referenced
this pull request
May 18, 2015
Provide graceful shutdown of file watcher in tests
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test failure from #92 inspired this change:
https://travis-ci.org/bitly/google_auth_proxy/jobs/62425336
2015/05/13 16:27:33 using authenticated emails file /tmp/test_auth_emails_952353477
2015/05/13 16:27:33 watching /tmp/test_auth_emails_952353477 for updates
2015/05/13 16:27:33 validating: is [email protected] valid? true
2015/05/13 16:27:33 watching interrupted on event: "/tmp/test_auth_emails_952353477": CHMOD
2015/05/13 16:27:33 watching resumed for /tmp/test_auth_emails_952353477
2015/05/13 16:27:33 reloading after event: "/tmp/test_auth_emails_952353477": CHMOD
2015/05/13 16:27:33 watching interrupted on event: "/tmp/test_auth_emails_952353477": REMOVE
2015/05/13 16:27:33 validating: is [email protected] valid? false
2015/05/13 16:27:33 watching resumed for /tmp/test_auth_emails_952353477
2015/05/13 16:27:33 reloading after event: "/tmp/test_auth_emails_952353477": REMOVE
2015/05/13 16:27:33 failed opening authenticated-emails-file="/tmp/test_auth_emails_952353477", open /tmp/test_auth_emails_952353477: no such file or directory
I believe that what happened was that the call to reload the file after the
second "reloading after event" lost the race when the test shut down and the
file was removed. This change introduces a
donechannel that ensuresoutstanding actions complete and the watcher exits before the test removes the
file.
When this goes in, I'll rebase #92 on it to trigger a new test run.
cc: @jehiah @balshor