Skip to content

Commit

Permalink
filewatcher: only recurse when directory has a /... suffix
Browse files Browse the repository at this point in the history
Previously filewatcher.Watch would recurse into all directories.

Also fix a bug where max depth of the directory walk was including the root directory path in the depth.

Adds tests for a couple functions
  • Loading branch information
dnephin committed Oct 24, 2020
1 parent 1c78e40 commit d0804e6
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 21 deletions.
53 changes: 32 additions & 21 deletions internal/filewatcher/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,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)
Expand All @@ -68,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
Expand All @@ -108,25 +119,25 @@ 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
}
}
}
Expand Down
81 changes: 81 additions & 0 deletions internal/filewatcher/watch_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
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) {
Expand Down Expand Up @@ -76,3 +80,80 @@ func TestHandler_HandleEvent(t *testing.T) {
})
}
}

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)
}

0 comments on commit d0804e6

Please sign in to comment.