Repository: gotestyourself/gotestsum
Pull request #157 (2020-10-24 03:42:37)

filewatcher: fix a bug with maxDepth and comparing event.Op
also, some tests


Patch:
diff --git a/go.mod b/go.mod
index 60fe2a4..9e01474 100644
--- a/go.mod
+++ b/go.mod
@@ -5,7 +5,7 @@ replace github.com/spf13/pflag => github.com/dnephin/pflag v0.0.0-20200521001137
 require (
 	github.com/fatih/color v1.9.0
 	github.com/fsnotify/fsnotify v1.4.9
-	github.com/google/go-cmp v0.3.0
+	github.com/google/go-cmp v0.4.0
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/jonboulle/clockwork v0.1.0
 	github.com/mattn/go-colorable v0.1.6 // indirect
@@ -15,7 +15,7 @@ require (
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 	golang.org/x/sys v0.0.0-20201009025420-dfb3f7c4e634
 	golang.org/x/tools v0.0.0-20190624222133-a101b041ded4
-	gotest.tools/v3 v3.0.2
+	gotest.tools/v3 v3.0.3
 )
 
 go 1.11
diff --git a/go.sum b/go.sum
index b3c0d38..8c7759d 100644
--- a/go.sum
+++ b/go.sum
@@ -4,8 +4,8 @@ github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
-github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
-github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
+github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/jonboulle/clockwork v0.1.0 h1:VKV+ZcuP6l3yW9doeqz6ziZGgcynBVQO+obU0+0hcPo=
@@ -42,5 +42,7 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20190624222133-a101b041ded4 h1:1mMox4TgefDwqluYCv677yNXwlfTkija4owZve/jr78=
 golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
-gotest.tools/v3 v3.0.2 h1:kG1BFyqVHuQoVQiR1bWGnfz/fmHvvuiSPIV7rvl360E=
-gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
+gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
diff --git a/internal/filewatcher/watch.go b/internal/filewatcher/watch.go
index b8733e1..b90792d 100644
--- a/internal/filewatcher/watch.go
+++ b/internal/filewatcher/watch.go
@@ -38,6 +38,9 @@ func Watch(dirs []string, run func(pkg string) error) error {
 		case <-timer.C:
 			return fmt.Errorf("exceeded idle timeout while watching files")
 		case event := <-watcher.Events:
+			if !timer.Stop() {
+				<-timer.C
+			}
 			log.Debugf("handling event %v", event)
 
 			if handleDirCreated(watcher, event) {
@@ -54,9 +57,28 @@ func Watch(dirs []string, run func(pkg string) error) error {
 	}
 }
 
-func findAllDirs(dirs []string, depth int) []string {
-	var output []string
+func findAllDirs(dirs []string, maxDepth int) []string {
+	if len(dirs) == 0 {
+		dirs = []string{"./..."}
+	}
+
+	var output []string // nolint: prealloc
+	for _, dir := range dirs {
+		const recur = "/..."
+		if strings.HasSuffix(dir, recur) {
+			dir = strings.TrimSuffix(dir, recur)
+			output = append(output, findSubDirs(dir, maxDepth)...)
+			continue
+		}
+		output = append(output, dir)
+	}
+	return output
+}
 
+func findSubDirs(rootDir string, maxDepth int) []string {
+	var output []string
+	// add root dir depth so that maxDepth is relative to the root dir
+	maxDepth += pathDepth(rootDir)
 	walker := func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			log.Warnf("failed to watch %v: %v", path, err)
@@ -65,32 +87,24 @@ func findAllDirs(dirs []string, depth int) []string {
 		if !info.IsDir() {
 			return nil
 		}
-		if isMaxDepth(path, depth) || exclude(path) {
+		if pathDepth(path) > maxDepth || exclude(path) {
 			log.Debugf("Ignoring %v because of max depth or exclude list", path)
 			return filepath.SkipDir
 		}
-		if noGoFiles(path) {
+		if !hasGoFiles(path) {
 			log.Debugf("Ignoring %v because it has no .go files", path)
 			return nil
 		}
 		output = append(output, path)
 		return nil
 	}
-
-	if len(dirs) == 0 {
-		dirs = []string{"."}
-	}
-
-	for _, dir := range dirs {
-		dir = strings.TrimSuffix(dir, "/...")
-		// nolint: errcheck // error is handled by walker func
-		filepath.Walk(dir, walker)
-	}
+	// nolint: errcheck // error is handled by walker func
+	filepath.Walk(rootDir, walker)
 	return output
 }
 
-func isMaxDepth(path string, depth int) bool {
-	return strings.Count(filepath.Clean(path), string(filepath.Separator)) >= depth
+func pathDepth(path string) int {
+	return strings.Count(filepath.Clean(path), string(filepath.Separator))
 }
 
 // return true if path is vendor, testdata, or starts with a dot
@@ -105,31 +119,31 @@ func exclude(path string) bool {
 	return false
 }
 
-func noGoFiles(path string) bool {
+func hasGoFiles(path string) bool {
 	fh, err := os.Open(path)
 	if err != nil {
-		return true
+		return false
 	}
 
 	for {
 		names, err := fh.Readdirnames(20)
 		switch {
 		case err == io.EOF:
-			return true
+			return false
 		case err != nil:
 			log.Warnf("failed to read directory %v: %v", path, err)
-			return true
+			return false
 		}
 
 		for _, name := range names {
 			if strings.HasSuffix(name, ".go") {
-				return false
+				return true
 			}
 		}
 	}
 }
 
-func handleDirCreated(watcher *fsnotify.Watcher, event fsnotify.Event) bool {
+func handleDirCreated(watcher *fsnotify.Watcher, event fsnotify.Event) (handled bool) {
 	if event.Op&fsnotify.Create != fsnotify.Create {
 		return false
 	}
@@ -158,7 +172,7 @@ type handler struct {
 const floodThreshold = 250 * time.Millisecond
 
 func (h *handler) handleEvent(event fsnotify.Event) error {
-	if event.Op&fsnotify.Write|fsnotify.Create == 0 {
+	if event.Op&(fsnotify.Write|fsnotify.Create) == 0 {
 		return nil
 	}
 


Test patch:
diff --git a/internal/filewatcher/watch_test.go b/internal/filewatcher/watch_test.go
new file mode 100644
index 0000000..657db54
--- /dev/null
+++ b/internal/filewatcher/watch_test.go
@@ -0,0 +1,159 @@
+package filewatcher
+
+import (
+	"fmt"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/fsnotify/fsnotify"
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/env"
+	"gotest.tools/v3/fs"
+)
+
+func TestHandler_HandleEvent(t *testing.T) {
+	type testCase struct {
+		name        string
+		last        time.Time
+		expectedRun bool
+		event       fsnotify.Event
+	}
+
+	fn := func(t *testing.T, tc testCase) {
+		var ran bool
+		run := func(pkg string) error {
+			ran = true
+			return nil
+		}
+
+		h := handler{last: tc.last, fn: run}
+		err := h.handleEvent(tc.event)
+		assert.NilError(t, err)
+		assert.Equal(t, ran, tc.expectedRun)
+		if tc.expectedRun {
+			assert.Assert(t, !h.last.IsZero())
+		}
+	}
+
+	var testCases = []testCase{
+		{
+			name:  "Op is rename",
+			event: fsnotify.Event{Op: fsnotify.Rename, Name: "file_test.go"},
+		},
+		{
+			name:  "Op is remove",
+			event: fsnotify.Event{Op: fsnotify.Remove, Name: "file_test.go"},
+		},
+		{
+			name:  "Op is chmod",
+			event: fsnotify.Event{Op: fsnotify.Chmod, Name: "file_test.go"},
+		},
+		{
+			name:        "Op is write+chmod",
+			event:       fsnotify.Event{Op: fsnotify.Write | fsnotify.Chmod, Name: "file_test.go"},
+			expectedRun: true,
+		},
+		{
+			name:        "Op is write",
+			event:       fsnotify.Event{Op: fsnotify.Write, Name: "file_test.go"},
+			expectedRun: true,
+		},
+		{
+			name:        "Op is create",
+			event:       fsnotify.Event{Op: fsnotify.Create, Name: "file_test.go"},
+			expectedRun: true,
+		},
+		{
+			name:  "file is not a go file",
+			event: fsnotify.Event{Op: fsnotify.Write, Name: "readme.md"},
+		},
+		{
+			name:  "under flood threshold",
+			event: fsnotify.Event{Op: fsnotify.Create, Name: "file_test.go"},
+			last:  time.Now(),
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			fn(t, tc)
+		})
+	}
+}
+
+func TestHasGoFiles(t *testing.T) {
+	t.Run("none", func(t *testing.T) {
+		tmpDir := fs.NewDir(t, t.Name(), fs.WithFile("readme.md", ""))
+		defer tmpDir.Remove()
+		assert.Assert(t, !hasGoFiles(tmpDir.Path()))
+	})
+	t.Run("empty", func(t *testing.T) {
+		tmpDir := fs.NewDir(t, t.Name())
+		defer tmpDir.Remove()
+		assert.Assert(t, !hasGoFiles(tmpDir.Path()))
+	})
+	t.Run("some go files", func(t *testing.T) {
+		tmpDir := fs.NewDir(t, t.Name(), fs.WithFile("main.go", ""))
+		defer tmpDir.Remove()
+		assert.Assert(t, hasGoFiles(tmpDir.Path()))
+	})
+	t.Run("many go files", func(t *testing.T) {
+		tmpDir := fs.NewDir(t, t.Name())
+		for i := 0; i < 47; i++ {
+			fs.Apply(t, tmpDir, fs.WithFile(fmt.Sprintf("file%d.go", i), ""))
+		}
+		defer tmpDir.Remove()
+		assert.Assert(t, hasGoFiles(tmpDir.Path()))
+	})
+}
+
+func TestFindAllDirs(t *testing.T) {
+	goFile := fs.WithFile("file.go", "")
+	dirOne := fs.NewDir(t, t.Name(),
+		goFile,
+		fs.WithFile("not-a-dir", ""),
+		fs.WithDir("no-go-files"),
+		fs.WithDir(".starts-with-dot", goFile))
+	defer dirOne.Remove()
+	var path string
+	for i := 1; i <= 10; i++ {
+		path = filepath.Join(path, fmt.Sprintf("%d", i))
+		var ops []fs.PathOp
+		if i != 4 && i != 5 {
+			ops = []fs.PathOp{goFile}
+		}
+		fs.Apply(t, dirOne, fs.WithDir(path, ops...))
+	}
+
+	dirTwo := fs.NewDir(t, t.Name(),
+		goFile,
+		// subdir should be ignored, dirTwo is used without /... suffix
+		fs.WithDir("subdir", goFile))
+	defer dirTwo.Remove()
+
+	dirs := findAllDirs([]string{dirOne.Path() + "/...", dirTwo.Path()}, maxDepth)
+	expected := []string{
+		dirOne.Path(),
+		dirOne.Join("1"),
+		dirOne.Join("1/2"),
+		dirOne.Join("1/2/3"),
+		dirOne.Join("1/2/3/4/5/6"),
+		dirOne.Join("1/2/3/4/5/6/7"),
+		dirTwo.Path(),
+	}
+	assert.DeepEqual(t, dirs, expected)
+}
+
+func TestFindAllDirs_DefaultPath(t *testing.T) {
+	goFile := fs.WithFile("file.go", "")
+	dirOne := fs.NewDir(t, t.Name(),
+		goFile,
+		fs.WithDir("a", goFile),
+		fs.WithDir("b", goFile))
+	defer dirOne.Remove()
+
+	defer env.ChangeWorkingDir(t, dirOne.Path())()
+	dirs := findAllDirs([]string{}, maxDepth)
+	expected := []string{".", "a", "b"}
+	assert.DeepEqual(t, dirs, expected)
+}