Merge pull request #3 from ploxiln/watcher_test_deflake

tests: watcher test deflake

validator.go

@@ -59,8 +59,7 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() {
 	atomic.StorePointer(&um.m, unsafe.Pointer(&updated))
 }
 
-func newValidatorImpl(domains []string, usersFile string,
-	done <-chan bool, onUpdate func()) func(string) bool {
+func newValidatorImpl(domains []string, usersFile string, done <-chan bool, onUpdate func()) func(string) bool {
 	validUsers := NewUserMap(usersFile, done, onUpdate)
 
 	var allowAll bool

validator_test.go

@@ -20,7 +20,8 @@ func NewValidatorTest(t *testing.T) *ValidatorTest {
 	if err != nil {
 		t.Fatal("failed to create temp file: " + err.Error())
 	}
-	vt.done = make(chan bool, 1)
+	t.Logf("%s using temp file %s", t.Name(), vt.auth_email_file.Name())
+	vt.done = make(chan bool)
 	return vt
 }
 
@@ -29,20 +30,17 @@ func (vt *ValidatorTest) TearDown() {
 	os.Remove(vt.auth_email_file.Name())
 }
 
-func (vt *ValidatorTest) NewValidator(domains []string,
-	updated chan<- bool) func(string) bool {
-	return newValidatorImpl(domains, vt.auth_email_file.Name(),
-		vt.done, func() {
-			if vt.update_seen == false {
-				updated <- true
-				vt.update_seen = true
-			}
-		})
+func (vt *ValidatorTest) NewValidator(domains []string, updated chan<- bool) func(string) bool {
+	return newValidatorImpl(domains, vt.auth_email_file.Name(), vt.done, func() {
+		if vt.update_seen == false {
+			updated <- true
+			vt.update_seen = true
+		}
+	})
 }
 
 // This will close vt.auth_email_file.
 func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) {
-	defer vt.auth_email_file.Close()
 	vt.auth_email_file.WriteString(strings.Join(emails, "\n"))
 	if err := vt.auth_email_file.Close(); err != nil {
 		t.Fatal("failed to close temp file " +

validator_watcher_copy_test.go

@@ -1,4 +1,4 @@
-// +build go1.3,!plan9,!solaris,!windows
+// +build !plan9,!solaris,!windows
 
 // Turns out you can't copy over an existing file on Windows.
 
@@ -32,7 +32,7 @@ func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) {
 
 	vt.WriteEmails(t, []string{"xyzzy@example.com"})
 	domains := []string(nil)
-	updated := make(chan bool)
+	updated := make(chan bool, 1)
 	validator := vt.NewValidator(domains, updated)
 
 	if !validator("xyzzy@example.com") {

validator_watcher_test.go

@@ -1,4 +1,4 @@
-// +build go1.3,!plan9,!solaris
+// +build !plan9,!solaris
 
 package main
 
@@ -49,7 +49,7 @@ func TestValidatorOverwriteEmailListDirectly(t *testing.T) {
 		"plugh@example.com",
 	})
 	domains := []string(nil)
-	updated := make(chan bool)
+	updated := make(chan bool, 1)
 	validator := vt.NewValidator(domains, updated)
 
 	if !validator("xyzzy@example.com") {

watcher.go

@@ -1,4 +1,4 @@
-// +build go1.3,!plan9,!solaris
+// +build !plan9,!solaris
 
 package main
 
@@ -11,23 +11,18 @@ import (
 	"github.com/fsnotify/fsnotify"
 )
 
-func WaitForReplacement(filename string, op fsnotify.Op,
-	watcher *fsnotify.Watcher) {
-	const sleep_interval = 50 * time.Millisecond
+func WaitForReplacement(filename string, watcher *fsnotify.Watcher) {
+	for i := 0; i < 20; i++ {
+		time.Sleep(100 * time.Millisecond)
 
-	// Avoid a race when fsnofity.Remove is preceded by fsnotify.Chmod.
-	if op&fsnotify.Chmod != 0 {
-		time.Sleep(sleep_interval)
-	}
-	for {
 		if _, err := os.Stat(filename); err == nil {
 			if err := watcher.Add(filename); err == nil {
 				log.Printf("watching resumed for %s", filename)
 				return
 			}
 		}
-		time.Sleep(sleep_interval)
 	}
+	log.Printf("failed to resume watching for %s", filename)
 }
 
 func WatchForUpdates(filename string, done <-chan bool, action func()) {
@@ -37,22 +32,22 @@ func WatchForUpdates(filename string, done <-chan bool, action func()) {
 		log.Fatal("failed to create watcher for ", filename, ": ", err)
 	}
 	go func() {
-		defer watcher.Close()
 		for {
 			select {
 			case _ = <-done:
 				log.Printf("Shutting down watcher for: %s", filename)
-				break
+				watcher.Close()
+				return
 			case event := <-watcher.Events:
 				// On Arch Linux, it appears Chmod events precede Remove events,
 				// which causes a race between action() and the coming Remove event.
-				// If the Remove wins, the action() (which calls
-				// UserMap.LoadAuthenticatedEmailsFile()) crashes when the file
-				// can't be opened.
-				if event.Op&(fsnotify.Remove|fsnotify.Rename|fsnotify.Chmod) != 0 {
+				if event.Op == fsnotify.Chmod {
+					continue
+				}
+				if event.Op&(fsnotify.Remove|fsnotify.Rename) != 0 {
 					log.Printf("watching interrupted on event: %s", event)
 					watcher.Remove(filename)
-					WaitForReplacement(filename, event.Op, watcher)
+					WaitForReplacement(filename, watcher)
 				}
 				log.Printf("reloading after event: %s", event)
 				action()

watcher_unsupported.go

@@ -1,4 +1,4 @@
-// +build !go1.3 plan9 solaris
+// +build plan9 solaris
 
 package main