From: Martin Tournoij Date: Fri, 5 Aug 2022 15:03:18 +0000 (+0200) Subject: Use "os.Rename()" in tests instead of "mv" X-Git-Tag: v1.7.2~74 X-Git-Url: https://go.fuhry.dev/?a=commitdiff_plain;h=ee33a655c06adcd00538cb9fb3d9fc5d8a727f42;p=fsnotify.git 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 08164fd..c858a6f 100644 --- 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 fff6074..7f833ce 100644 --- 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 6b8b338..f188fd8 100644 --- 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