testing/fstest: return structured errors in TestFS

TestFS now returns a structured error built with errors.Join to allow to
inspect errors using errors.Is and errors.As.

All errors are now wrapped using fmt.Errorf and %w.

Fixes #63675.

Change-Id: I8fc3363f8ae70085af4afdb84c16be9ca70d7731
Reviewed-on: https://go-review.googlesource.com/c/go/+/537015
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>

diff --git a/doc/next/6-stdlib/99-minor/testing/fstest/63675.md b/doc/next/6-stdlib/99-minor/testing/fstest/63675.md
new file mode 100644 (file)
index 0000000..1a3552d
--- /dev/null
+++ b/doc/next/6-stdlib/99-minor/testing/fstest/63675.md
@@ -0,0 +1,4 @@
+[`TestFS`](/pkg/testing/fstest#TestFS) now returns a structured
+error that can be unwrapped (via method `Unwrap() []error`). This allows
+inspecting errors using [`errors.Is`](/pkg/errors#Is) or
+[`errors.As`](/pkg/errors#As).

diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go
index 78b0b82640d0bcb909d3e1c8eff72d67d9fed309..0fd7d4806f72017a9c740b693196a3c503fd16ee 100644 (file)
--- a/src/testing/fstest/testfs.go
+++ b/src/testing/fstest/testfs.go
@@ -25,8 +25,8 @@ import (
 // Otherwise, fsys must contain at least the listed files; it can also contain others.
 // The contents of fsys must not change concurrently with TestFS.
 //
-// If TestFS finds any misbehaviors, it returns an error reporting all of them.
-// The error text spans multiple lines, one per detected misbehavior.
+// If TestFS finds any misbehaviors, it returns either the first error or a
+// list of errors. Use [errors.Is] or [errors.As] to inspect.
 //
 // Typical usage inside a test is:
 //
@@ -51,7 +51,7 @@ func TestFS(fsys fs.FS, expected ...string) error {
                                return err
                        }
                        if err := testFS(sub, subExpected...); err != nil {
-                               return fmt.Errorf("testing fs.Sub(fsys, %s): %v", dir, err)
+                               return fmt.Errorf("testing fs.Sub(fsys, %s): %w", dir, err)
                        }
                        break // one sub-test is enough
                }
@@ -89,32 +89,29 @@ func testFS(fsys fs.FS, expected ...string) error {
                        t.errorf("expected but not found: %s", name)
                }
        }
-       if len(t.errText) == 0 {
+       if len(t.errors) == 0 {
                return nil
        }
-       return errors.New("TestFS found errors:\n" + string(t.errText))
+       return fmt.Errorf("TestFS found errors:\n%w", errors.Join(t.errors...))
 }
 
 // An fsTester holds state for running the test.
 type fsTester struct {
-       fsys    fs.FS
-       errText []byte
-       dirs    []string
-       files   []string
+       fsys   fs.FS
+       errors []error
+       dirs   []string
+       files  []string
 }
 
-// errorf adds an error line to errText.
+// errorf adds an error to the list of errors.
 func (t *fsTester) errorf(format string, args ...any) {
-       if len(t.errText) > 0 {
-               t.errText = append(t.errText, '\n')
-       }
-       t.errText = append(t.errText, fmt.Sprintf(format, args...)...)
+       t.errors = append(t.errors, fmt.Errorf(format, args...))
 }
 
 func (t *fsTester) openDir(dir string) fs.ReadDirFile {
        f, err := t.fsys.Open(dir)
        if err != nil {
-               t.errorf("%s: Open: %v", dir, err)
+               t.errorf("%s: Open: %w", dir, err)
                return nil
        }
        d, ok := f.(fs.ReadDirFile)
@@ -138,7 +135,7 @@ func (t *fsTester) checkDir(dir string) {
        list, err := d.ReadDir(-1)
        if err != nil {
                d.Close()
-               t.errorf("%s: ReadDir(-1): %v", dir, err)
+               t.errorf("%s: ReadDir(-1): %w", dir, err)
                return
        }
 
@@ -176,7 +173,7 @@ func (t *fsTester) checkDir(dir string) {
        list2, err := d.ReadDir(-1)
        if len(list2) > 0 || err != nil {
                d.Close()
-               t.errorf("%s: ReadDir(-1) at EOF = %d entries, %v, wanted 0 entries, nil", dir, len(list2), err)
+               t.errorf("%s: ReadDir(-1) at EOF = %d entries, %w, wanted 0 entries, nil", dir, len(list2), err)
                return
        }
 
@@ -184,13 +181,13 @@ func (t *fsTester) checkDir(dir string) {
        list2, err = d.ReadDir(1)
        if len(list2) > 0 || err != io.EOF {
                d.Close()
-               t.errorf("%s: ReadDir(1) at EOF = %d entries, %v, wanted 0 entries, EOF", dir, len(list2), err)
+               t.errorf("%s: ReadDir(1) at EOF = %d entries, %w, wanted 0 entries, EOF", dir, len(list2), err)
                return
        }
 
        // Check that close does not report an error.
        if err := d.Close(); err != nil {
-               t.errorf("%s: Close: %v", dir, err)
+               t.errorf("%s: Close: %w", dir, err)
        }
 
        // Check that closing twice doesn't crash.
@@ -204,7 +201,7 @@ func (t *fsTester) checkDir(dir string) {
        defer d.Close()
        list2, err = d.ReadDir(-1)
        if err != nil {
-               t.errorf("%s: second Open+ReadDir(-1): %v", dir, err)
+               t.errorf("%s: second Open+ReadDir(-1): %w", dir, err)
                return
        }
        t.checkDirList(dir, "first Open+ReadDir(-1) vs second Open+ReadDir(-1)", list, list2)
@@ -230,7 +227,7 @@ func (t *fsTester) checkDir(dir string) {
                        break
                }
                if err != nil {
-                       t.errorf("%s: third Open: ReadDir(%d) after %d: %v", dir, n, len(list2), err)
+                       t.errorf("%s: third Open: ReadDir(%d) after %d: %w", dir, n, len(list2), err)
                        return
                }
                if n == 0 {
@@ -244,7 +241,7 @@ func (t *fsTester) checkDir(dir string) {
        if fsys, ok := t.fsys.(fs.ReadDirFS); ok {
                list2, err := fsys.ReadDir(dir)
                if err != nil {
-                       t.errorf("%s: fsys.ReadDir: %v", dir, err)
+                       t.errorf("%s: fsys.ReadDir: %w", dir, err)
                        return
                }
                t.checkDirList(dir, "first Open+ReadDir(-1) vs fsys.ReadDir", list, list2)
@@ -259,7 +256,7 @@ func (t *fsTester) checkDir(dir string) {
        // Check fs.ReadDir as well.
        list2, err = fs.ReadDir(t.fsys, dir)
        if err != nil {
-               t.errorf("%s: fs.ReadDir: %v", dir, err)
+               t.errorf("%s: fs.ReadDir: %w", dir, err)
                return
        }
        t.checkDirList(dir, "first Open+ReadDir(-1) vs fs.ReadDir", list, list2)
@@ -358,7 +355,7 @@ func (t *fsTester) checkGlob(dir string, list []fs.DirEntry) {
 
        names, err := t.fsys.(fs.GlobFS).Glob(glob)
        if err != nil {
-               t.errorf("%s: Glob(%#q): %v", dir, glob, err)
+               t.errorf("%s: Glob(%#q): %w", dir, glob, err)
                return
        }
        if reflect.DeepEqual(want, names) {
@@ -391,13 +388,13 @@ func (t *fsTester) checkGlob(dir string, list []fs.DirEntry) {
 func (t *fsTester) checkStat(path string, entry fs.DirEntry) {
        file, err := t.fsys.Open(path)
        if err != nil {
-               t.errorf("%s: Open: %v", path, err)
+               t.errorf("%s: Open: %w", path, err)
                return
        }
        info, err := file.Stat()
        file.Close()
        if err != nil {
-               t.errorf("%s: Stat: %v", path, err)
+               t.errorf("%s: Stat: %w", path, err)
                return
        }
        fentry := formatEntry(entry)
@@ -409,7 +406,7 @@ func (t *fsTester) checkStat(path string, entry fs.DirEntry) {
 
        einfo, err := entry.Info()
        if err != nil {
-               t.errorf("%s: entry.Info: %v", path, err)
+               t.errorf("%s: entry.Info: %w", path, err)
                return
        }
        finfo := formatInfo(info)
@@ -430,7 +427,7 @@ func (t *fsTester) checkStat(path string, entry fs.DirEntry) {
        // Stat should be the same as Open+Stat, even for symlinks.
        info2, err := fs.Stat(t.fsys, path)
        if err != nil {
-               t.errorf("%s: fs.Stat: %v", path, err)
+               t.errorf("%s: fs.Stat: %w", path, err)
                return
        }
        finfo2 := formatInfo(info2)
@@ -441,7 +438,7 @@ func (t *fsTester) checkStat(path string, entry fs.DirEntry) {
        if fsys, ok := t.fsys.(fs.StatFS); ok {
                info2, err := fsys.Stat(path)
                if err != nil {
-                       t.errorf("%s: fsys.Stat: %v", path, err)
+                       t.errorf("%s: fsys.Stat: %w", path, err)
                        return
                }
                finfo2 := formatInfo(info2)
@@ -508,19 +505,19 @@ func (t *fsTester) checkFile(file string) {
        // Read entire file.
        f, err := t.fsys.Open(file)
        if err != nil {
-               t.errorf("%s: Open: %v", file, err)
+               t.errorf("%s: Open: %w", file, err)
                return
        }
 
        data, err := io.ReadAll(f)
        if err != nil {
                f.Close()
-               t.errorf("%s: Open+ReadAll: %v", file, err)
+               t.errorf("%s: Open+ReadAll: %w", file, err)
                return
        }
 
        if err := f.Close(); err != nil {
-               t.errorf("%s: Close: %v", file, err)
+               t.errorf("%s: Close: %w", file, err)
        }
 
        // Check that closing twice doesn't crash.
@@ -531,7 +528,7 @@ func (t *fsTester) checkFile(file string) {
        if fsys, ok := t.fsys.(fs.ReadFileFS); ok {
                data2, err := fsys.ReadFile(file)
                if err != nil {
-                       t.errorf("%s: fsys.ReadFile: %v", file, err)
+                       t.errorf("%s: fsys.ReadFile: %w", file, err)
                        return
                }
                t.checkFileRead(file, "ReadAll vs fsys.ReadFile", data, data2)
@@ -543,7 +540,7 @@ func (t *fsTester) checkFile(file string) {
                }
                data2, err = fsys.ReadFile(file)
                if err != nil {
-                       t.errorf("%s: second call to fsys.ReadFile: %v", file, err)
+                       t.errorf("%s: second call to fsys.ReadFile: %w", file, err)
                        return
                }
                t.checkFileRead(file, "Readall vs second fsys.ReadFile", data, data2)
@@ -555,7 +552,7 @@ func (t *fsTester) checkFile(file string) {
        // Check that fs.ReadFile works with t.fsys.
        data2, err := fs.ReadFile(t.fsys, file)
        if err != nil {
-               t.errorf("%s: fs.ReadFile: %v", file, err)
+               t.errorf("%s: fs.ReadFile: %w", file, err)
                return
        }
        t.checkFileRead(file, "ReadAll vs fs.ReadFile", data, data2)
@@ -563,7 +560,7 @@ func (t *fsTester) checkFile(file string) {
        // Use iotest.TestReader to check small reads, Seek, ReadAt.
        f, err = t.fsys.Open(file)
        if err != nil {
-               t.errorf("%s: second Open: %v", file, err)
+               t.errorf("%s: second Open: %w", file, err)
                return
        }
        defer f.Close()

diff --git a/src/testing/fstest/testfs_test.go b/src/testing/fstest/testfs_test.go
index a48c597ff43e320f3cca7d83aec1c310a408f37e..b9f10c613a7c4749ffd3cc610005097f26f4aa56 100644 (file)
--- a/src/testing/fstest/testfs_test.go
+++ b/src/testing/fstest/testfs_test.go
@@ -5,6 +5,7 @@
 package fstest
 
 import (
+       "errors"
        "internal/testenv"
        "io/fs"
        "os"
@@ -76,3 +77,40 @@ func TestShuffledFS(t *testing.T) {
                t.Error(err)
        }
 }
+
+// failPermFS is a filesystem that always fails with fs.ErrPermission.
+type failPermFS struct{}
+
+func (f failPermFS) Open(name string) (fs.File, error) {
+       if !fs.ValidPath(name) {
+               return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
+       }
+       return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrPermission}
+}
+
+func TestTestFSWrappedErrors(t *testing.T) {
+       err := TestFS(failPermFS{})
+       if err == nil {
+               t.Fatal("error expected")
+       }
+       t.Logf("Error (expecting wrapped fs.ErrPermission):\n%v", err)
+
+       if !errors.Is(err, fs.ErrPermission) {
+               t.Errorf("error should be a wrapped ErrPermission: %#v", err)
+       }
+
+       // TestFS is expected to return a list of errors.
+       // Enforce that the list can be extracted for browsing.
+       var errs interface{ Unwrap() []error }
+       if !errors.As(err, &errs) {
+               t.Errorf("caller should be able to extract the errors as a list: %#v", err)
+       } else {
+               for _, err := range errs.Unwrap() {
+                       // ErrPermission is expected
+                       // but any other error must be reported.
+                       if !errors.Is(err, fs.ErrPermission) {
+                               t.Errorf("unexpected error: %v", err)
+                       }
+               }
+       }
+}