From d4f074b32f685ce308ea6cae9c2a61aec77bed5f Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Wed, 22 Aug 2018 15:33:06 -0400 Subject: [PATCH] watch: simplify the fileEvent interface to only contain paths (#144) --- pkg/watch/notify.go | 6 +- pkg/watch/notify_test.go | 114 ++++++++---------------------------- pkg/watch/watcher_darwin.go | 43 +++----------- pkg/watch/watcher_linux.go | 12 ++-- 4 files changed, 42 insertions(+), 133 deletions(-) diff --git a/pkg/watch/notify.go b/pkg/watch/notify.go index efc024139..e40b04dc8 100644 --- a/pkg/watch/notify.go +++ b/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 } diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index 28a9bddc0..9f023e4e5 100644 --- a/pkg/watch/notify_test.go +++ b/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: diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index 624d5ffd6..aa50d78fc 100644 --- a/pkg/watch/watcher_darwin.go +++ b/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{} diff --git a/pkg/watch/watcher_linux.go b/pkg/watch/watcher_linux.go index e3fc09910..16beef1b4 100644 --- a/pkg/watch/watcher_linux.go +++ b/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{}