Skip to content

Commit

Permalink
all: make tests pass on macOS, do assorted cleanup
Browse files Browse the repository at this point in the history
Add go.sum to placate cmd/go.

Check error from os.RemoveAll in test.

Make NewNotifyTest and friends handle cleanup.
This way the callers don't need to manage it.
It is also more robust in the face of test failure,
and thus less likely to strand testdata test files.

Use t.TempDir instead of ioutil.TempDir.

Use os.SameFile to test for path equality.
On macOS, the temp dir is located in /var, which can also
have path /private/var. Make the tests resilient to
changes in the exact path name.
With this fix, all tests now pass locally on macOS.

Be more careful about cleaning up test trees.
Fatalf calls runtime.Goexit, which doesn't run deferred funcs.
Remove the defer and just call RemoveAll directly.
This should help prevent stranded testdata dirs on test failure.

Remove ioutil.
It is deprecated.
  • Loading branch information
josharian committed Dec 13, 2022
1 parent 11c1ede commit 130b672
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 38 deletions.
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
golang.org/x/sys v0.0.0-20180926160741-c2ed4eda69e7 h1:bit1t3mgdR35yN0cX0G8orgLtOuyL9Wqxa1mccLB0ig=
golang.org/x/sys v0.0.0-20180926160741-c2ed4eda69e7/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
6 changes: 3 additions & 3 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package notify

import (
"errors"
"io/ioutil"
"io/fs"
"os"
"path/filepath"
)
Expand Down Expand Up @@ -78,12 +78,12 @@ Traverse:
}
// TODO(rjeczalik): tolerate open failures - add failed names to
// AddDirError and notify users which names are not added to the tree.
fi, err := ioutil.ReadDir(nd.Name)
fi, err := os.ReadDir(nd.Name)
if err != nil {
return err
}
for _, fi := range fi {
if fi.Mode()&(os.ModeSymlink|os.ModeDir) == os.ModeDir {
if fi.Type()&(fs.ModeSymlink|fs.ModeDir) == fs.ModeDir {
name := filepath.Join(nd.Name, fi.Name())
stack = append(stack, nd.addchild(name, name[len(nd.Name)+1:]))
}
Expand Down
2 changes: 0 additions & 2 deletions notify_inotify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import "testing"

func TestNotifySystemAndGlobalMix(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(2)

Expand All @@ -30,7 +29,6 @@ func TestNotifySystemAndGlobalMix(t *testing.T) {

func TestUnknownEvent(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(1)

Expand Down
3 changes: 0 additions & 3 deletions notify_readdcw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import "testing"

func TestNotifySystemSpecificEvent(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(1)

Expand All @@ -33,7 +32,6 @@ func TestNotifySystemSpecificEvent(t *testing.T) {

func TestUnknownEvent(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(1)

Expand All @@ -42,7 +40,6 @@ func TestUnknownEvent(t *testing.T) {

func TestNotifySystemAndGlobalMix(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(3)

Expand Down
39 changes: 23 additions & 16 deletions notify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
package notify

import (
"io/ioutil"
"errors"
"os"
"path/filepath"
"testing"
Expand All @@ -17,7 +17,6 @@ import (

func TestNotifyExample(t *testing.T) {
n := NewNotifyTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(3)

Expand Down Expand Up @@ -110,11 +109,7 @@ func TestStop(t *testing.T) {
}

func TestRenameInRoot(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "notify_test-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmpDir)
tmpDir := t.TempDir()

c := make(chan EventInfo, 100)
first := filepath.Join(tmpDir, "foo")
Expand All @@ -138,7 +133,7 @@ func TestRenameInRoot(t *testing.T) {
for {
select {
case ev := <-c:
if ev.Path() == file {
if samefile(t, ev.Path(), file) {
return
}
t.Log(ev.Path())
Expand All @@ -149,11 +144,7 @@ func TestRenameInRoot(t *testing.T) {
}

func TestRecreated(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "notify_test-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmpDir)
tmpDir := t.TempDir()

dir := filepath.Join(tmpDir, "folder")
file := filepath.Join(dir, "file")
Expand All @@ -165,20 +156,20 @@ func TestRecreated(t *testing.T) {

recreateFolder := func() {
// Give the sync some time to process events
_ = os.RemoveAll(dir)
mustT(t, os.RemoveAll(dir))
mustT(t, os.Mkdir(dir, 0777))
time.Sleep(100 * time.Millisecond)

// Create a file
mustT(t, ioutil.WriteFile(file, []byte("abc"), 0666))
mustT(t, os.WriteFile(file, []byte("abc"), 0666))
}
timeout := time.After(5 * time.Second)
checkCreated := func() {
for {
select {
case ev := <-eventChan:
t.Log(ev.Path(), ev.Event())
if ev.Path() == file && ev.Event() == Create {
if samefile(t, ev.Path(), file) && ev.Event() == Create {
return
}
case <-timeout:
Expand Down Expand Up @@ -216,3 +207,19 @@ func mustT(t testing.TB, err error) {
t.Fatal(err)
}
}

func samefile(t *testing.T, p1, p2 string) bool {
// The tests sometimes delete files shortly after creating them.
// That's expected; ignore stat failures.
fi1, err := os.Stat(p1)
if errors.Is(err, os.ErrNotExist) {
return false
}
mustT(t, err)
fi2, err := os.Stat(p2)
if errors.Is(err, os.ErrNotExist) {
return false
}
mustT(t, err)
return os.SameFile(fi1, fi2)
}
15 changes: 9 additions & 6 deletions testing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package notify
import (
"bufio"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -105,7 +104,7 @@ func tmptree(root, list string) (string, error) {
}
defer f.Close()
if root == "" {
if root, err = ioutil.TempDir(vfs()); err != nil {
if root, err = os.MkdirTemp(vfs()); err != nil {
return "", err
}
}
Expand Down Expand Up @@ -713,18 +712,21 @@ func NewNotifyTest(t *testing.T, tree string) *N {
} else {
n.tree = newNonrecursiveTree(n.w.watcher(), n.w.c(), nil)
}
t.Cleanup(n.Close)
return n
}

func NewRecursiveTreeTest(t *testing.T, tree string) *N {
n := newTreeN(t, tree)
n.tree = newRecursiveTree(n.spy, n.c)
t.Cleanup(n.Close)
return n
}

func NewNonrecursiveTreeTest(t *testing.T, tree string) *N {
n := newTreeN(t, tree)
n.tree = newNonrecursiveTree(n.spy, n.c, nil)
t.Cleanup(n.Close)
return n
}

Expand All @@ -750,6 +752,7 @@ func NewNonrecursiveTreeTestC(t *testing.T, tree string) (*N, chan EventInfo) {
tr := newNonrecursiveTree(n.spy, n.c, recinternal)
tr.rec = rec
n.tree = tr
t.Cleanup(n.Close)
return n, recuser
}

Expand All @@ -764,12 +767,12 @@ func (n *N) W() *W {
return n.w
}

func (n *N) Close() error {
defer os.RemoveAll(n.w.root)
if err := n.tree.Close(); err != nil {
func (n *N) Close() {
err := n.tree.Close()
os.RemoveAll(n.w.root)
if err != nil {
n.w.Fatalf("(notifier).Close()=%v", err)
}
return nil
}

func (n *N) Watch(path string, c chan<- EventInfo, events ...Event) {
Expand Down
2 changes: 0 additions & 2 deletions tree_nonrecursive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

func TestNonrecursiveTree(t *testing.T) {
n := NewNonrecursiveTreeTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(5)

Expand Down Expand Up @@ -430,7 +429,6 @@ func TestNonrecursiveTree(t *testing.T) {

func TestNonrecursiveTreeInternal(t *testing.T) {
n, c := NewNonrecursiveTreeTestC(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(5)

Expand Down
3 changes: 0 additions & 3 deletions tree_recursive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import "testing"

func TestRecursiveTree(t *testing.T) {
n := NewRecursiveTreeTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(5)

Expand Down Expand Up @@ -409,7 +408,6 @@ func TestRecursiveTree(t *testing.T) {

func TestRecursiveTreeWatchInactiveMerge(t *testing.T) {
n := NewRecursiveTreeTest(t, "testdata/vfs.txt")
defer n.Close()

ch := NewChans(1)

Expand Down Expand Up @@ -480,7 +478,6 @@ func TestRecursiveTreeWatchInactiveMerge(t *testing.T) {

func TestRecursiveTree_Windows(t *testing.T) {
n := NewRecursiveTreeTest(t, "testdata/vfs.txt")
defer n.Close()

const ChangeFileName = Event(0x1)

Expand Down
5 changes: 2 additions & 3 deletions util_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
package notify

import (
"io/ioutil"
"os"
"path/filepath"
"testing"
)

func tmpfile(s string) (string, error) {
f, err := ioutil.TempFile(filepath.Split(s))
f, err := os.CreateTemp(filepath.Split(s))
if err != nil {
return "", err
}
Expand Down Expand Up @@ -96,7 +95,7 @@ func TestCanonicalCircular(t *testing.T) {

// issue #83
func TestCanonical_RelativeSymlink(t *testing.T) {
dir, err := ioutil.TempDir(wd, "")
dir, err := os.MkdirTemp(wd, "")
if err != nil {
t.Fatalf("TempDir()=%v", err)
}
Expand Down

0 comments on commit 130b672

Please sign in to comment.