git.iko.soy/typeable/oauth2-proxy
on master help sign in
Provide graceful shutdown of file watcher in tests

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 xyzzy@example.com 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 xyzzy@example.com 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.

commit6a0f119fc22080548a26e067c1ec49f0c5a5bcfb authorMike Bland <michael.bland@gsa.gov> dateWed 13 May 2015, 17:30 (11 years ago) parent254b26d
5 files changed+27−17
compare with master
unifiedsplit
validator.go+5−5 validator_test.go+14−5 validator_watcher_copy_test.go+1−2 validator_watcher_test.go+2−4 watcher.go+5−1
validator.go+5−5
@@ -15,13 +15,13 @@ type UserMap struct {
1515 m unsafe.Pointer
1616}
1717
18−func NewUserMap(usersFile string, onUpdate func()) *UserMap {
18+func NewUserMap(usersFile string, done <-chan bool, onUpdate func()) *UserMap {
1919 um := &UserMap{usersFile: usersFile}
2020 m := make(map[string]bool)
2121 atomic.StorePointer(&um.m, unsafe.Pointer(&m))
2222 if usersFile != "" {
2323 log.Printf("using authenticated emails file %s", usersFile)
24− started := WatchForUpdates(usersFile, func() {
24+ started := WatchForUpdates(usersFile, done, func() {
2525 um.LoadAuthenticatedEmailsFile()
2626 onUpdate()
2727 })
@@ -62,8 +62,8 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() {
6262}
6363
6464func newValidatorImpl(domains []string, usersFile string,
65− onUpdate func()) func(string) bool {
66− validUsers := NewUserMap(usersFile, onUpdate)
65+ done <-chan bool, onUpdate func()) func(string) bool {
66+ validUsers := NewUserMap(usersFile, done, onUpdate)
6767
6868 for i, domain := range domains {
6969 domains[i] = fmt.Sprintf("@%s", strings.ToLower(domain))
@@ -85,5 +85,5 @@ func newValidatorImpl(domains []string, usersFile string,
8585}
8686
8787func NewValidator(domains []string, usersFile string) func(string) bool {
88− return newValidatorImpl(domains, usersFile, func() {})
88+ return newValidatorImpl(domains, usersFile, nil, func() {})
8989}
validator_test.go+14−5
@@ -9,6 +9,7 @@ import (
99
1010type ValidatorTest struct {
1111 auth_email_file *os.File
12+ done chan bool
1213}
1314
1415func NewValidatorTest(t *testing.T) *ValidatorTest {
@@ -18,13 +19,21 @@ func NewValidatorTest(t *testing.T) *ValidatorTest {
1819 if err != nil {
1920 t.Fatal("failed to create temp file: " + err.Error())
2021 }
22+ vt.done = make(chan bool)
2123 return vt
2224}
2325
2426func (vt *ValidatorTest) TearDown() {
27+ vt.done <- true
2528 os.Remove(vt.auth_email_file.Name())
2629}
2730
31+func (vt *ValidatorTest) NewValidator(domains []string,
32+ updated chan<- bool) func(string) bool {
33+ return newValidatorImpl(domains, vt.auth_email_file.Name(),
34+ vt.done, func() { updated <- true })
35+}
36+
2837// This will close vt.auth_email_file.
2938func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) {
3039 defer vt.auth_email_file.Close()
@@ -41,7 +50,7 @@ func TestValidatorEmpty(t *testing.T) {
4150
4251 vt.WriteEmails(t, []string(nil))
4352 domains := []string(nil)
44− validator := NewValidator(domains, vt.auth_email_file.Name())
53+ validator := vt.NewValidator(domains, nil)
4554
4655 if validator("foo.bar@example.com") {
4756 t.Error("nothing should validate when the email and " +
@@ -55,7 +64,7 @@ func TestValidatorSingleEmail(t *testing.T) {
5564
5665 vt.WriteEmails(t, []string{"foo.bar@example.com"})
5766 domains := []string(nil)
58− validator := NewValidator(domains, vt.auth_email_file.Name())
67+ validator := vt.NewValidator(domains, nil)
5968
6069 if !validator("foo.bar@example.com") {
6170 t.Error("email should validate")
@@ -72,7 +81,7 @@ func TestValidatorSingleDomain(t *testing.T) {
7281
7382 vt.WriteEmails(t, []string(nil))
7483 domains := []string{"example.com"}
75− validator := NewValidator(domains, vt.auth_email_file.Name())
84+ validator := vt.NewValidator(domains, nil)
7685
7786 if !validator("foo.bar@example.com") {
7887 t.Error("email should validate")
@@ -91,7 +100,7 @@ func TestValidatorMultipleEmailsMultipleDomains(t *testing.T) {
91100 "plugh@example.com",
92101 })
93102 domains := []string{"example0.com", "example1.com"}
94− validator := NewValidator(domains, vt.auth_email_file.Name())
103+ validator := vt.NewValidator(domains, nil)
95104
96105 if !validator("foo.bar@example0.com") {
97106 t.Error("email from first domain should validate")
@@ -117,7 +126,7 @@ func TestValidatorComparisonsAreCaseInsensitive(t *testing.T) {
117126
118127 vt.WriteEmails(t, []string{"Foo.Bar@Example.Com"})
119128 domains := []string{"Frobozz.Com"}
120− validator := NewValidator(domains, vt.auth_email_file.Name())
129+ validator := vt.NewValidator(domains, nil)
121130
122131 if !validator("foo.bar@example.com") {
123132 t.Error("loaded email addresses are not lower-cased")
validator_watcher_copy_test.go+1−2
@@ -34,8 +34,7 @@ func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) {
3434 vt.WriteEmails(t, []string{"xyzzy@example.com"})
3535 domains := []string(nil)
3636 updated := make(chan bool)
37− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
38− func() { updated <- true })
37+ validator := vt.NewValidator(domains, updated)
3938
4039 if !validator("xyzzy@example.com") {
4140 t.Error("email in list should validate")
validator_watcher_test.go+2−4
@@ -51,8 +51,7 @@ func TestValidatorOverwriteEmailListDirectly(t *testing.T) {
5151 })
5252 domains := []string(nil)
5353 updated := make(chan bool)
54− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
55− func() { updated <- true })
54+ validator := vt.NewValidator(domains, updated)
5655
5756 if !validator("xyzzy@example.com") {
5857 t.Error("first email in list should validate")
@@ -89,8 +88,7 @@ func TestValidatorOverwriteEmailListViaRenameAndReplace(t *testing.T) {
8988 vt.WriteEmails(t, []string{"xyzzy@example.com"})
9089 domains := []string(nil)
9190 updated := make(chan bool)
92− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
93− func() { updated <- true })
91+ validator := vt.NewValidator(domains, updated)
9492
9593 if !validator("xyzzy@example.com") {
9694 t.Error("email in list should validate")
watcher.go+5−1
@@ -30,7 +30,7 @@ func WaitForReplacement(event fsnotify.Event, watcher *fsnotify.Watcher) {
3030 }
3131}
3232
33−func WatchForUpdates(filename string, action func()) bool {
33+func WatchForUpdates(filename string, done <-chan bool, action func()) bool {
3434 filename = filepath.Clean(filename)
3535 watcher, err := fsnotify.NewWatcher()
3636 if err != nil {
@@ -40,6 +40,10 @@ func WatchForUpdates(filename string, action func()) bool {
4040 defer watcher.Close()
4141 for {
4242 select {
43+ case _ = <-done:
44+ log.Printf("Shutting down watcher for: %s",
45+ filename)
46+ return
4347 case event := <-watcher.Events:
4448 // On Arch Linux, it appears Chmod events precede Remove events,
4549 // which causes a race between action() and the coming Remove event.