watch: simplify the fileEvent interface to only contain paths (#144)

pkg/watch/notify.go

@@ -1,10 +1,12 @@
 package watch
 
-import "github.com/windmilleng/fsnotify"
+type FileEvent struct {
+	Path string
+}
 
 type Notify interface {
 	Close() error
 	Add(name string) error
-	Events() chan fsnotify.Event
+	Events() chan FileEvent
 	Errors() chan error
 }

pkg/watch/notify_test.go

@@ -9,8 +9,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-
-	"github.com/windmilleng/fsnotify"
 )
 
 // Each implementation of the notify interface should have the same basic
@@ -19,7 +17,6 @@ import (
 func TestNoEvents(t *testing.T) {
 	f := newNotifyFixture(t)
 	defer f.tearDown()
-	f.fsync()
 	f.assertEvents()
 }
 
@@ -44,7 +41,7 @@ func TestEventOrdering(t *testing.T) {
 	f.fsync()
 	f.events = nil
 
-	var expected []fsnotify.Event
+	var expected []string
 	for i, dir := range dirs {
 		base := fmt.Sprintf("%d.txt", i)
 		p := filepath.Join(dir, base)
@@ -52,33 +49,10 @@ func TestEventOrdering(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		expected = append(expected, create(filepath.Join(dir, base)))
+		expected = append(expected, filepath.Join(dir, base))
 	}
 
-	f.fsync()
-
-	f.filterJustCreateEvents()
 	f.assertEvents(expected...)
-
-	// Check to make sure that the files appeared in the right order.
-	createEvents := make([]fsnotify.Event, 0, count)
-	for _, e := range f.events {
-		if e.Op == fsnotify.Create {
-			createEvents = append(createEvents, e)
-		}
-	}
-
-	if len(createEvents) != count {
-		t.Fatalf("Expected %d create events. Actual: %+v", count, createEvents)
-	}
-
-	for i, event := range createEvents {
-		base := fmt.Sprintf("%d.txt", i)
-		p := filepath.Join(dirs[i], base)
-		if event.Name != p {
-			t.Fatalf("Expected event %q at %d. Actual: %+v", base, i, createEvents)
-		}
-	}
 }
 
 func TestWatchesAreRecursive(t *testing.T) {
@@ -112,10 +86,7 @@ func TestWatchesAreRecursive(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// we should get notified
-	f.fsync()
-
-	f.assertEvents(create(changeFilePath))
+	f.assertEvents(changeFilePath)
 }
 
 func TestNewDirectoriesAreRecursivelyWatched(t *testing.T) {
@@ -146,10 +117,7 @@ func TestNewDirectoriesAreRecursivelyWatched(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	// we should get notified
-	f.fsync()
-	// assert events
-	f.assertEvents(create(subPath), create(changeFilePath))
+	f.assertEvents(subPath, changeFilePath)
 }
 
 func TestWatchNonExistentPath(t *testing.T) {
@@ -172,12 +140,7 @@ func TestWatchNonExistentPath(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	f.fsync()
-	if runtime.GOOS == "darwin" {
-		f.assertEvents(create(path))
-	} else {
-		f.assertEvents(create(path), write(path))
-	}
+	f.assertEvents(path)
 }
 
 func TestRemove(t *testing.T) {
@@ -209,9 +172,7 @@ func TestRemove(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	f.fsync()
-
-	f.assertEvents(remove(path))
+	f.assertEvents(path)
 }
 
 func TestRemoveAndAddBack(t *testing.T) {
@@ -242,9 +203,8 @@ func TestRemoveAndAddBack(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	f.fsync()
 
-	f.assertEvents(remove(path))
+	f.assertEvents(path)
 	f.events = nil
 
 	err = ioutil.WriteFile(path, d1, 0644)
@@ -252,7 +212,7 @@ func TestRemoveAndAddBack(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	f.assertEvents(create(path))
+	f.assertEvents(path)
 }
 
 func TestSingleFile(t *testing.T) {
@@ -288,9 +248,7 @@ func TestSingleFile(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	f.fsync()
-
-	f.assertEvents(create(path))
+	f.assertEvents(path)
 }
 
 type notifyFixture struct {
@@ -298,7 +256,7 @@ type notifyFixture struct {
 	root    *TempDir
 	watched *TempDir
 	notify  Notify
-	events  []fsnotify.Event
+	events  []FileEvent
 }
 
 func newNotifyFixture(t *testing.T) *notifyFixture {
@@ -330,52 +288,21 @@ func newNotifyFixture(t *testing.T) *notifyFixture {
 	}
 }
 
-func (f *notifyFixture) filterJustCreateEvents() {
-	var r []fsnotify.Event
+func (f *notifyFixture) assertEvents(expected ...string) {
+	f.fsync()
 
-	for _, ev := range f.events {
-		if ev.Op != fsnotify.Create {
-			continue
-		}
-		r = append(r, ev)
-	}
-
-	f.events = r
-}
-
-func (f *notifyFixture) assertEvents(expected ...fsnotify.Event) {
 	if len(f.events) != len(expected) {
 		f.t.Fatalf("Got %d events (expected %d): %v %v", len(f.events), len(expected), f.events, expected)
 	}
 
 	for i, actual := range f.events {
-		if actual != expected[i] {
-			f.t.Fatalf("Got event %v (expected %v)", actual, expected[i])
+		e := FileEvent{expected[i]}
+		if actual != e {
+			f.t.Fatalf("Got event %v (expected %v)", actual, e)
 		}
 	}
 }
 
-func create(f string) fsnotify.Event {
-	return fsnotify.Event{
-		Name: f,
-		Op:   fsnotify.Create,
-	}
-}
-
-func write(f string) fsnotify.Event {
-	return fsnotify.Event{
-		Name: f,
-		Op:   fsnotify.Write,
-	}
-}
-
-func remove(f string) fsnotify.Event {
-	return fsnotify.Event{
-		Name: f,
-		Op:   fsnotify.Remove,
-	}
-}
-
 func (f *notifyFixture) fsync() {
 	syncPathBase := fmt.Sprintf("sync-%d.txt", time.Now().UnixNano())
 	syncPath := filepath.Join(f.watched.Path(), syncPathBase)
@@ -394,12 +321,19 @@ F:
 			f.t.Fatal(err)
 
 		case event := <-f.notify.Events():
-			if strings.Contains(event.Name, syncPath) {
+			if strings.Contains(event.Path, syncPath) {
 				break F
 			}
-			if strings.Contains(event.Name, anySyncPath) {
+			if strings.Contains(event.Path, anySyncPath) {
 				continue
 			}
+
+			// Don't bother tracking duplicate changes to the same path
+			// for testing.
+			if len(f.events) > 0 && f.events[len(f.events)-1].Path == event.Path {
+				continue
+			}
+
 			f.events = append(f.events, event)
 
 		case <-timeout:

pkg/watch/watcher_darwin.go

@@ -6,12 +6,11 @@ import (
 	"time"
 
 	"github.com/windmilleng/fsevents"
-	"github.com/windmilleng/fsnotify"
 )
 
 type darwinNotify struct {
 	stream *fsevents.EventStream
-	events chan fsnotify.Event
+	events chan FileEvent
 	errors chan error
 	stop   chan struct{}
 
@@ -35,7 +34,6 @@ func (d *darwinNotify) isTrackingPath(path string) bool {
 }
 
 func (d *darwinNotify) loop() {
-	lastCreate := ""
 	ignoredSpuriousEvent := false
 	for {
 		select {
@@ -48,31 +46,18 @@ func (d *darwinNotify) loop() {
 
 			for _, e := range events {
 				e.Path = filepath.Join("/", e.Path)
-				op := eventFlagsToOp(e.Flags)
-
-				// Sometimes we get duplicate CREATE events.
-				//
-				// This is exercised by TestEventOrdering, which creates lots of files
-				// and generates duplicate CREATE events for some of them.
-				if op == fsnotify.Create {
-					if lastCreate == e.Path {
-						continue
-					}
-					lastCreate = e.Path
-				}
 
 				// ignore the first event that says the watched directory
 				// has been created. these are fired spuriously on initiation.
-				if op == fsnotify.Create {
+				if e.Flags&fsevents.ItemCreated == fsevents.ItemCreated {
 					if d.isTrackingPath(e.Path) && !ignoredSpuriousEvent {
 						ignoredSpuriousEvent = true
 						continue
 					}
 				}
 
-				d.events <- fsnotify.Event{
-					Name: e.Path,
-					Op:   op,
+				d.events <- FileEvent{
+					Path: e.Path,
 				}
 			}
 		}
@@ -116,7 +101,7 @@ func (d *darwinNotify) Close() error {
 	return nil
 }
 
-func (d *darwinNotify) Events() chan fsnotify.Event {
+func (d *darwinNotify) Events() chan FileEvent {
 	return d.events
 }
 
@@ -131,7 +116,7 @@ func NewWatcher() (Notify, error) {
 			Flags:   fsevents.FileEvents,
 		},
 		sm:     &sync.Mutex{},
-		events: make(chan fsnotify.Event),
+		events: make(chan FileEvent),
 		errors: make(chan error),
 		stop:   make(chan struct{}),
 	}
@@ -139,18 +124,4 @@ func NewWatcher() (Notify, error) {
 	return dw, nil
 }
 
-func eventFlagsToOp(flags fsevents.EventFlags) fsnotify.Op {
-	if flags&fsevents.ItemRemoved != 0 {
-		return fsnotify.Remove
-	}
-	if flags&fsevents.ItemRenamed != 0 {
-		return fsnotify.Rename
-	}
-	if flags&fsevents.ItemChangeOwner != 0 {
-		return fsnotify.Chmod
-	}
-	if flags&fsevents.ItemCreated != 0 {
-		return fsnotify.Create
-	}
-	return fsnotify.Write
-}
+var _ Notify = &darwinNotify{}

pkg/watch/watcher_linux.go

@@ -20,7 +20,7 @@ const inotifyMin = 8192
 type linuxNotify struct {
 	watcher       *fsnotify.Watcher
 	events        chan fsnotify.Event
-	wrappedEvents chan fsnotify.Event
+	wrappedEvents chan FileEvent
 	errors        chan error
 	watchList     map[string]bool
 }
@@ -69,7 +69,7 @@ func (d *linuxNotify) Close() error {
 	return d.watcher.Close()
 }
 
-func (d *linuxNotify) Events() chan fsnotify.Event {
+func (d *linuxNotify) Events() chan FileEvent {
 	return d.wrappedEvents
 }
 
@@ -107,12 +107,12 @@ func (d *linuxNotify) loop() {
 
 func (d *linuxNotify) sendEventIfWatched(e fsnotify.Event) {
 	if _, ok := d.watchList[e.Name]; ok {
-		d.wrappedEvents <- e
+		d.wrappedEvents <- FileEvent{e.Name}
 	} else {
 		// TODO(dmiller): maybe use a prefix tree here?
 		for path := range d.watchList {
 			if pathIsChildOf(e.Name, path) {
-				d.wrappedEvents <- e
+				d.wrappedEvents <- FileEvent{e.Name}
 				break
 			}
 		}
@@ -125,7 +125,7 @@ func NewWatcher() (*linuxNotify, error) {
 		return nil, err
 	}
 
-	wrappedEvents := make(chan fsnotify.Event)
+	wrappedEvents := make(chan FileEvent)
 
 	wmw := &linuxNotify{
 		watcher:       fsw,
@@ -171,3 +171,5 @@ func checkInotifyLimits() error {
 
 	return nil
 }
+
+var _ Notify = &linuxNotify{}