Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore fs.WalkDir errors in FindFiles #122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions script.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -219,20 +220,25 @@ func File(name string) *Pipe {
//
// test/1.txt
// test/2.txt
func FindFiles(path string) *Pipe {
func FindFiles(root string) *Pipe {
var fileNames []string
walkFn := func(path string, info os.FileInfo, err error) error {
walkFn := func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
fileNames = append(fileNames, path)
if !d.IsDir() {
fileNames = append(fileNames, filepath.Join(root, path))
}
return nil
}
if err := filepath.Walk(path, walkFn); err != nil {
return NewPipe().WithError(err)
}

// ignoring any errors here to more closely align
// behavior with `find`. for example, we wouldn't
// expect `find` to return an error without results
// if it encountered any permissions errors during
// its execution. see [github issue #99]
// (https://github.com/bitfield/script/issues/99)
_ = fs.WalkDir(os.DirFS(root), ".", walkFn)
return Slice(fileNames)
}

Expand Down
8 changes: 0 additions & 8 deletions script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1017,14 +1017,6 @@ func TestFindFiles_RecursesIntoSubdirectories(t *testing.T) {
}
}

func TestFindFiles_InNonexistentPathReturnsError(t *testing.T) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a key part of FindFiles behaviour that it doesn't return an error for missing paths, shouldn't we have a test for that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah great point, pushed up a test

t.Parallel()
p := script.FindFiles("nonexistent_path")
if p.Error() == nil {
t.Fatal("want error for nonexistent path")
}
}

func TestIfExists_ProducesErrorPlusNoOutputForNonexistentFile(t *testing.T) {
t.Parallel()
want := ""
Expand Down