Use "os.Rename()" in tests instead of "mv"

This was added in 00f2dfd (2011): "BSD Test: Need to use outside program
to trigger some events". All tests pass fine with just os.Rename(), so
just use that.

Might add another real-world test in the future for common tools like
mv, cp, vi, etc. because they don't always behave the same as library
calls, but calling mv in *only* the mv() helper is rather odd.

This also renames a few things in the kqueue helper for clarity, and
uses map[string]struct{} if we just use them for lookups to an
allocation (I originally tried to fix #488, but gave up on that for now,
hence the kqueue changes).

diff --git a/helpers_test.go b/helpers_test.go
index 08164fd09461d7ef040a82e20a410e2037ecae40..c858a6ff6562bbf755be83ec85710d755ac84bb4 100644 (file)
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -4,7 +4,6 @@ import (
        "fmt"
        "io/fs"
        "os"
-       "os/exec"
        "path/filepath"
        "runtime"
        "sort"
@@ -184,13 +183,7 @@ func mv(t *testing.T, src string, dst ...string) {
                t.Fatalf("mv: dst must have at least one element: %s", dst)
        }
 
-       var err error
-       switch runtime.GOOS {
-       case "windows", "plan9":
-               err = os.Rename(src, filepath.Join(dst...))
-       default:
-               err = exec.Command("mv", src, filepath.Join(dst...)).Run()
-       }
+       err := os.Rename(src, filepath.Join(dst...))
        if err != nil {
                t.Fatalf("mv(%q, %q): %s", src, filepath.Join(dst...), err)
        }
@@ -441,3 +434,8 @@ func cmpEvents(t *testing.T, tmp string, have, want Events) {
 func indent(s fmt.Stringer) string {
        return "\t" + strings.ReplaceAll(s.String(), "\n", "\n\t")
 }
+
+func isCI() bool {
+       _, ok := os.LookupEnv("CI")
+       return ok
+}

diff --git a/integration_test.go b/integration_test.go
index fff60748613b7e6d43d65fcea9657742fe2cc238..7f833cec7bd6ea010dd4573d1e031918422bfa47 100644 (file)
--- a/integration_test.go
+++ b/integration_test.go
@@ -113,8 +113,8 @@ func TestWatchRename(t *testing.T) {
                `},
 
                {"rename to unwatched directory", func(t *testing.T, w *Watcher, tmp string) {
-                       if runtime.GOOS == "netbsd" {
-                               t.Skip("NetBSD behaviour is not fully correct") // TODO: investigate and fix.
+                       if runtime.GOOS == "netbsd" && isCI() {
+                               t.Skip("fails in CI; see #488")
                        }
 
                        unwatched := t.TempDir()
@@ -128,10 +128,10 @@ func TestWatchRename(t *testing.T) {
                        cat(t, "data", renamed) // Modify the file outside of the watched dir
                        touch(t, file)          // Recreate the file that was moved
                }, `
-                       create /file
-                       write  /file
-                       rename /file
-                       create /file
+                       create /file # cat data >file
+                       write  /file # ^
+                       rename /file # mv file ../renamed
+                       create /file # touch file
 
                        # Windows has REMOVE /file, rather than CREATE /file
                        windows:

diff --git a/kqueue.go b/kqueue.go
index 6b8b3389e5cb64f6f9dffa3f658f8a69f3d28c41..f188fd86ed34d57f1aa2c0de4205bdc9a87530b7 100644 (file)
--- a/kqueue.go
+++ b/kqueue.go
@@ -27,14 +27,14 @@ type Watcher struct {
 
        kq int // File descriptor (as returned by the kqueue() syscall).
 
-       mu              sync.Mutex                  // Protects access to watcher data
-       watches         map[string]int              // Map of watched file descriptors (key: path).
-       watchesByDir    map[string]map[int]struct{} // Map of watched file descriptors indexed by the parent directory (key: dirname(path)).
-       externalWatches map[string]bool             // Map of watches added by user of the library.
-       dirFlags        map[string]uint32           // Map of watched directories to fflags used in kqueue.
-       paths           map[int]pathInfo            // Map file descriptors to path names for processing kqueue events.
-       fileExists      map[string]bool             // Keep track of if we know this file exists (to stop duplicate create events).
-       isClosed        bool                        // Set to true when Close() is first called
+       mu           sync.Mutex                  // Protects access to watcher data
+       watches      map[string]int              // Watched file descriptors (key: path).
+       watchesByDir map[string]map[int]struct{} // Watched file descriptors indexed by the parent directory (key: dirname(path)).
+       userWatches  map[string]struct{}         // Watches added with Watcher.Add()
+       dirFlags     map[string]uint32           // Watched directories to fflags used in kqueue.
+       paths        map[int]pathInfo            // File descriptors to path names for processing kqueue events.
+       fileExists   map[string]struct{}         // Keep track of if we know this file exists (to stop duplicate create events).
+       isClosed     bool                        // Set to true when Close() is first called
 }
 
 type pathInfo struct {
@@ -50,16 +50,16 @@ func NewWatcher() (*Watcher, error) {
        }
 
        w := &Watcher{
-               kq:              kq,
-               watches:         make(map[string]int),
-               watchesByDir:    make(map[string]map[int]struct{}),
-               dirFlags:        make(map[string]uint32),
-               paths:           make(map[int]pathInfo),
-               fileExists:      make(map[string]bool),
-               externalWatches: make(map[string]bool),
-               Events:          make(chan Event),
-               Errors:          make(chan error),
-               done:            make(chan struct{}),
+               kq:           kq,
+               watches:      make(map[string]int),
+               watchesByDir: make(map[string]map[int]struct{}),
+               dirFlags:     make(map[string]uint32),
+               paths:        make(map[int]pathInfo),
+               fileExists:   make(map[string]struct{}),
+               userWatches:  make(map[string]struct{}),
+               Events:       make(chan Event),
+               Errors:       make(chan error),
+               done:         make(chan struct{}),
        }
 
        go w.readEvents()
@@ -96,7 +96,7 @@ func (w *Watcher) Close() error {
 // Add starts watching the named file or directory (non-recursively).
 func (w *Watcher) Add(name string) error {
        w.mu.Lock()
-       w.externalWatches[name] = true
+       w.userWatches[name] = struct{}{}
        w.mu.Unlock()
        _, err := w.addWatch(name, noteAllEvents)
        return err
@@ -140,7 +140,7 @@ func (w *Watcher) Remove(name string) error {
                w.mu.Lock()
                for fd := range w.watchesByDir[name] {
                        path := w.paths[fd]
-                       if !w.externalWatches[path.name] {
+                       if _, ok := w.userWatches[path.name]; !ok {
                                pathsToRemove = append(pathsToRemove, path.name)
                        }
                }
@@ -325,21 +325,23 @@ loop:
 
                // Flush the events we received to the Events channel
                for len(kevents) > 0 {
-                       kevent := &kevents[0]
-                       watchfd := int(kevent.Ident)
-                       mask := uint32(kevent.Fflags)
+                       var (
+                               kevent  = &kevents[0]
+                               watchfd = int(kevent.Ident)
+                               mask    = uint32(kevent.Fflags)
+                       )
                        w.mu.Lock()
                        path := w.paths[watchfd]
                        w.mu.Unlock()
+
                        event := newEvent(path.name, mask)
 
                        if path.isDir && !event.Has(Remove) {
-                               // Double check to make sure the directory exists. This can happen when
-                               // we do a rm -fr on a recursively watched folders and we receive a
-                               // modification event first but the folder has been deleted and later
-                               // receive the delete event
+                               // Double check to make sure the directory exists. This can
+                               // happen when we do a rm -fr on a recursively watched folders
+                               // and we receive a modification event first but the folder has
+                               // been deleted and later receive the delete event.
                                if _, err := os.Lstat(event.Name); os.IsNotExist(err) {
-                                       // mark is as delete event
                                        event.Op |= Remove
                                }
                        }
@@ -423,10 +425,6 @@ func newEvent(name string, mask uint32) Event {
        return e
 }
 
-func newCreateEvent(name string) Event {
-       return Event{Name: name, Op: Create}
-}
-
 // watchDirectoryFiles to mimic inotify when adding a watch on a directory
 func (w *Watcher) watchDirectoryFiles(dirPath string) error {
        // Get all files
@@ -443,7 +441,7 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error {
                }
 
                w.mu.Lock()
-               w.fileExists[filePath] = true
+               w.fileExists[filePath] = struct{}{}
                w.mu.Unlock()
        }
 
@@ -483,7 +481,7 @@ func (w *Watcher) sendFileCreatedEventIfNew(filePath string, fileInfo os.FileInf
        if !doesExist {
                // Send create event
                select {
-               case w.Events <- newCreateEvent(filePath):
+               case w.Events <- Event{Name: filePath, Op: Create}:
                case <-w.done:
                        return
                }
@@ -496,7 +494,7 @@ func (w *Watcher) sendFileCreatedEventIfNew(filePath string, fileInfo os.FileInf
        }
 
        w.mu.Lock()
-       w.fileExists[filePath] = true
+       w.fileExists[filePath] = struct{}{}
        w.mu.Unlock()
 
        return nil