]> Cypherpunks repositories - gostls13.git/commitdiff
testing: fix -run behavior with fuzz tests
authorKatie Hockman <katie@golang.org>
Thu, 7 Oct 2021 20:26:36 +0000 (16:26 -0400)
committerKatie Hockman <katie@golang.org>
Tue, 12 Oct 2021 14:32:53 +0000 (14:32 +0000)
This change fixes some issues with -run, and
the subsequent command line output when running
in verbose mode. It replaces CorpusEntry.Name
with CorpusEntry.Path, and refactors the code
accordingly.

This change also adds a lot of additional tests
which check explicit command line output when
fuzz targets are run without fuzzing. This will
be important to avoid regressions.

Updates #48149

Change-Id: If34b1f51db646317b7b51c3c38ae53231d01f568
Reviewed-on: https://go-review.googlesource.com/c/go/+/354632
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
src/cmd/go/testdata/script/test_fuzz_run.txt [new file with mode: 0644]
src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt
src/internal/fuzz/fuzz.go
src/internal/fuzz/worker.go
src/testing/fuzz.go

index 79476ecb282ebb3e9fb00c6c59dc771d56bdc20a..628e003f419351313934be8ac478ef4a0b176a5e 100644 (file)
@@ -21,7 +21,7 @@ go run check_testdata.go FuzzWithBug
 # Now, the failing bytes should have been added to the seed corpus for
 # the target, and should fail when run without fuzzing.
 ! go test
-stdout 'testdata[/\\]fuzz[/\\]FuzzWithBug[/\\][a-f0-9]{64}'
+stdout 'FuzzWithBug/[a-f0-9]{64}'
 stdout 'this input caused a crash!'
 
 ! go test -run=FuzzWithNilPanic -fuzz=FuzzWithNilPanic -fuzztime=100x -fuzzminimizetime=1000x
diff --git a/src/cmd/go/testdata/script/test_fuzz_run.txt b/src/cmd/go/testdata/script/test_fuzz_run.txt
new file mode 100644 (file)
index 0000000..e546d99
--- /dev/null
@@ -0,0 +1,145 @@
+# TODO(jayconrod): support shared memory on more platforms.
+[!darwin] [!linux] [!windows] skip
+
+[short] skip
+env GOCACHE=$WORK/cache
+
+# Tests which verify the behavior and command line output when
+# running a fuzz target as a unit test.
+
+# Tests without -run.
+
+! go test
+stdout FAIL
+stdout 'error here'
+
+! go test -v
+stdout FAIL
+stdout 'error here'
+stdout '=== RUN   FuzzFoo/thisfails'
+stdout '--- FAIL: FuzzFoo/thisfails'
+stdout '=== RUN   FuzzFoo/thispasses'
+stdout '--- PASS: FuzzFoo/thispasses'
+
+# Tests where -run matches all seed corpora.
+
+! go test -run FuzzFoo/this
+stdout FAIL
+stdout 'error here'
+! stdout 'no tests to run'
+
+! go test -run /this
+stdout FAIL
+stdout 'error here'
+! stdout 'no tests to run'
+
+! go test -v -run FuzzFoo/this
+stdout FAIL
+stdout 'error here'
+stdout '=== RUN   FuzzFoo/thisfails'
+stdout '--- FAIL: FuzzFoo/thisfails'
+stdout '=== RUN   FuzzFoo/thispasses'
+stdout '--- PASS: FuzzFoo/thispasses'
+! stdout 'no tests to run'
+
+! go test -v -run /this
+stdout FAIL
+stdout 'error here'
+stdout '=== RUN   FuzzFoo/thisfails'
+stdout '--- FAIL: FuzzFoo/thisfails'
+stdout '=== RUN   FuzzFoo/thispasses'
+stdout '--- PASS: FuzzFoo/thispasses'
+! stdout 'no tests to run'
+
+# Tests where -run only matches one seed corpus which passes.
+
+go test -run FuzzFoo/thispasses
+stdout ok
+! stdout 'no tests to run'
+
+go test -run /thispasses
+stdout ok
+! stdout 'no tests to run'
+
+# Same tests in verbose mode
+go test -v -run FuzzFoo/thispasses
+stdout '=== RUN   FuzzFoo/thispasses'
+stdout '--- PASS: FuzzFoo/thispasses'
+! stdout '=== RUN   FuzzFoo/thisfails'
+! stdout 'no tests to run'
+
+go test -v -run /thispasses
+stdout '=== RUN   FuzzFoo/thispasses'
+stdout '--- PASS: FuzzFoo/thispasses'
+! stdout '=== RUN   FuzzFoo/thisfails'
+! stdout 'no tests to run'
+
+# Tests where -run only matches one seed corpus which fails.
+
+! go test -run FuzzFoo/thisfails
+stdout FAIL
+stdout 'error here'
+! stdout 'no tests to run'
+
+! go test -run /thisfails
+stdout FAIL
+stdout 'error here'
+! stdout 'no tests to run'
+
+! go test -v -run FuzzFoo/thisfails
+stdout 'error here'
+stdout '=== RUN   FuzzFoo/thisfails'
+stdout '--- FAIL: FuzzFoo/thisfails'
+! stdout '=== RUN   FuzzFoo/thispasses'
+! stdout 'no tests to run'
+
+! go test -v -run /thisfails
+stdout 'error here'
+stdout '=== RUN   FuzzFoo/thisfails'
+stdout '--- FAIL: FuzzFoo/thisfails'
+! stdout '=== RUN   FuzzFoo/thispasses'
+! stdout 'no tests to run'
+
+# Tests where -run doesn't match any seed corpora.
+
+go test -run FuzzFoo/nomatch
+stdout ok
+
+go test -run /nomatch
+stdout ok
+
+go test -v -run FuzzFoo/nomatch
+stdout '=== RUN   FuzzFoo'
+stdout '--- PASS: FuzzFoo'
+stdout ok
+! stdout 'no tests to run'
+
+go test -v -run /nomatch
+stdout '=== RUN   FuzzFoo'
+stdout '--- PASS: FuzzFoo'
+stdout ok
+! stdout 'no tests to run'
+
+-- go.mod --
+module example.com/x
+
+go 1.16
+-- x_test.go --
+package x
+
+import "testing"
+
+func FuzzFoo(f *testing.F) {
+    f.Add("this is fine")
+    f.Fuzz(func(t *testing.T, s string) {
+        if s == "fails" {
+            t.Error("error here")
+        }
+    })
+}
+-- testdata/fuzz/FuzzFoo/thisfails --
+go test fuzz v1
+string("fails")
+-- testdata/fuzz/FuzzFoo/thispasses --
+go test fuzz v1
+string("passes")
\ No newline at end of file
index a66d0554b69a8c8c8d1577baa4ae2cc646d5f108..5d04d8c0223ec424a91b40392b73ef86acc7b2ec 100644 (file)
@@ -26,6 +26,7 @@ stdout ok
 ! go test -fuzz=FuzzWithTestdata -run=FuzzWithTestdata -fuzztime=1x
 ! stdout ^ok
 ! stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithTestdata[/\\]'
+stdout 'found a crash while testing seed corpus entry: FuzzWithTestdata/1'
 stdout FAIL
 
 # Test that fuzzing a target with no seed corpus or cache finds a crash, prints
index f6600529114225dd775581cdbe4cf92905a10958..a8bbd60b1cc7e3ee33d1fd4317758445439b7ec4 100644 (file)
@@ -141,14 +141,14 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                if c.crashMinimizing == nil || crashWritten {
                        return
                }
-               fileName, werr := writeToCorpus(c.crashMinimizing.entry.Data, opts.CorpusDir)
+               werr := writeToCorpus(&c.crashMinimizing.entry, opts.CorpusDir)
                if werr != nil {
                        err = fmt.Errorf("%w\n%v", err, werr)
                        return
                }
                if err == nil {
                        err = &crashError{
-                               name: filepath.Base(fileName),
+                               path: c.crashMinimizing.entry.Path,
                                err:  errors.New(c.crashMinimizing.crasherMsg),
                        }
                }
@@ -230,7 +230,8 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
 
                        if result.crasherMsg != "" {
                                if c.warmupRun() && result.entry.IsSeed {
-                                       fmt.Fprintf(c.opts.Log, "found a crash while testing seed corpus entry: %q\n", result.entry.Parent)
+                                       target := filepath.Base(c.opts.CorpusDir)
+                                       fmt.Fprintf(c.opts.Log, "found a crash while testing seed corpus entry: %s/%s\n", target, testName(result.entry.Parent))
                                        stop(errors.New(result.crasherMsg))
                                        break
                                }
@@ -249,11 +250,11 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                                } else if !crashWritten {
                                        // Found a crasher that's either minimized or not minimizable.
                                        // Write to corpus and stop.
-                                       fileName, err := writeToCorpus(result.entry.Data, opts.CorpusDir)
+                                       err := writeToCorpus(&result.entry, opts.CorpusDir)
                                        if err == nil {
                                                crashWritten = true
                                                err = &crashError{
-                                                       name: filepath.Base(fileName),
+                                                       path: result.entry.Path,
                                                        err:  errors.New(result.crasherMsg),
                                                }
                                        }
@@ -262,7 +263,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                                                        c.opts.Log,
                                                        "DEBUG new crasher, elapsed: %s, id: %s, parent: %s, gen: %d, size: %d, exec time: %s\n",
                                                        c.elapsed(),
-                                                       fileName,
+                                                       result.entry.Path,
                                                        result.entry.Parent,
                                                        result.entry.Generation,
                                                        len(result.entry.Data),
@@ -315,12 +316,11 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                                                // Update the coordinator's coverage mask and save the value.
                                                inputSize := len(result.entry.Data)
                                                if opts.CacheDir != "" {
-                                                       filename, err := writeToCorpus(result.entry.Data, opts.CacheDir)
+                                                       err := writeToCorpus(&result.entry, opts.CacheDir)
                                                        if err != nil {
                                                                stop(err)
                                                        }
                                                        result.entry.Data = nil
-                                                       result.entry.Name = filename
                                                }
                                                c.updateCoverage(keepCoverage)
                                                c.corpus.entries = append(c.corpus.entries, result.entry)
@@ -331,7 +331,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                                                                c.opts.Log,
                                                                "DEBUG new interesting input, elapsed: %s, id: %s, parent: %s, gen: %d, new bits: %d, total bits: %d, size: %d, exec time: %s\n",
                                                                c.elapsed(),
-                                                               result.entry.Name,
+                                                               result.entry.Path,
                                                                result.entry.Parent,
                                                                result.entry.Generation,
                                                                countBits(keepCoverage),
@@ -347,7 +347,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
                                                        c.opts.Log,
                                                        "DEBUG worker reported interesting input that doesn't expand coverage, elapsed: %s, id: %s, parent: %s, canMinimize: %t\n",
                                                        c.elapsed(),
-                                                       result.entry.Name,
+                                                       result.entry.Path,
                                                        result.entry.Parent,
                                                        result.canMinimize,
                                                )
@@ -397,7 +397,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err
 // of the file where the input causing the crasher was saved. The testing
 // framework uses this to report a command to re-run that specific input.
 type crashError struct {
-       name string
+       path string
        err  error
 }
 
@@ -409,8 +409,8 @@ func (e *crashError) Unwrap() error {
        return e.err
 }
 
-func (e *crashError) CrashName() string {
-       return e.name
+func (e *crashError) CrashPath() string {
+       return e.path
 }
 
 type corpus struct {
@@ -426,14 +426,14 @@ type corpus struct {
 type CorpusEntry = struct {
        Parent string
 
-       // Name is the name of the corpus file, if the entry was loaded from the
-       // seed corpus. It can be used with -run. For entries added with f.Add and
-       // entries generated by the mutator, Name is empty and Data is populated.
-       Name string
+       // Path is the path of the corpus file, if the entry was loaded from disk.
+       // For other entries, including seed values provided by f.Add, Path is the
+       // name of the test, e.g. seed#0 or its hash.
+       Path string
 
-       // Data is the raw input data. Data should only be populated for initial
-       // seed values added with f.Add. For on-disk corpus files, Data will
-       // be nil.
+       // Data is the raw input data. Data should only be populated for seed
+       // values. For on-disk corpus files, Data will be nil, as it will be loaded
+       // from disk using Path.
        Data []byte
 
        // Values is the unmarshaled values from a corpus file.
@@ -452,7 +452,7 @@ func CorpusEntryData(ce CorpusEntry) ([]byte, error) {
                return ce.Data, nil
        }
 
-       return os.ReadFile(ce.Name)
+       return os.ReadFile(ce.Path)
 }
 
 type fuzzInput struct {
@@ -616,7 +616,7 @@ type coordinator struct {
 }
 
 func newCoordinator(opts CoordinateFuzzingOpts) (*coordinator, error) {
-       // Make sure all of the seed corpus given by f.Add has marshalled data.
+       // Make sure all of the seed corpus has marshalled data.
        for i := range opts.Seed {
                if opts.Seed[i].Data == nil && opts.Seed[i].Values != nil {
                        opts.Seed[i].Data = marshalCorpusFile(opts.Seed[i].Values...)
@@ -673,7 +673,7 @@ func newCoordinator(opts CoordinateFuzzingOpts) (*coordinator, error) {
                data := marshalCorpusFile(vals...)
                h := sha256.Sum256(data)
                name := fmt.Sprintf("%x", h[:4])
-               c.corpus.entries = append(c.corpus.entries, CorpusEntry{Name: name, Data: data})
+               c.corpus.entries = append(c.corpus.entries, CorpusEntry{Path: name, Data: data})
        }
 
        return c, nil
@@ -956,7 +956,7 @@ func ReadCorpus(dir string, types []reflect.Type) ([]CorpusEntry, error) {
                        errs = append(errs, fmt.Errorf("%q: %v", filename, err))
                        continue
                }
-               corpus = append(corpus, CorpusEntry{Name: filename, Values: vals})
+               corpus = append(corpus, CorpusEntry{Path: filename, Values: vals})
        }
        if len(errs) > 0 {
                return corpus, &MalformedCorpusError{errs: errs}
@@ -979,7 +979,7 @@ func readCorpusData(data []byte, types []reflect.Type) ([]interface{}, error) {
 // provided.
 func CheckCorpus(vals []interface{}, types []reflect.Type) error {
        if len(vals) != len(types) {
-               return fmt.Errorf("wrong number of values in corpus entry: %d, want %d", len(vals), len(types))
+               return fmt.Errorf("wrong number of values in corpus entry %v: want %v", vals, types)
        }
        for i := range types {
                if reflect.TypeOf(vals[i]) != types[i] {
@@ -989,21 +989,25 @@ func CheckCorpus(vals []interface{}, types []reflect.Type) error {
        return nil
 }
 
-// writeToCorpus atomically writes the given bytes to a new file in testdata.
-// If the directory does not exist, it will create one. If the file already
-// exists, writeToCorpus will not rewrite it. writeToCorpus returns the
-// file's name, or an error if it failed.
-func writeToCorpus(b []byte, dir string) (name string, err error) {
-       sum := fmt.Sprintf("%x", sha256.Sum256(b))
-       name = filepath.Join(dir, sum)
+// writeToCorpus atomically writes the given bytes to a new file in testdata. If
+// the directory does not exist, it will create one. If the file already exists,
+// writeToCorpus will not rewrite it. writeToCorpus sets entry.Path to the new
+// file that was just written or an error if it failed.
+func writeToCorpus(entry *CorpusEntry, dir string) (err error) {
+       sum := fmt.Sprintf("%x", sha256.Sum256(entry.Data))
+       entry.Path = filepath.Join(dir, sum)
        if err := os.MkdirAll(dir, 0777); err != nil {
-               return "", err
+               return err
        }
-       if err := ioutil.WriteFile(name, b, 0666); err != nil {
-               os.Remove(name) // remove partially written file
-               return "", err
+       if err := ioutil.WriteFile(entry.Path, entry.Data, 0666); err != nil {
+               os.Remove(entry.Path) // remove partially written file
+               return err
        }
-       return name, nil
+       return nil
+}
+
+func testName(path string) string {
+       return filepath.Base(path)
 }
 
 func zeroValue(t reflect.Type) interface{} {
index 1429decba83929a4aaecd068a19bc6e32637669a..0c428ed832d3bacacbce68a389ace2e158512429 100644 (file)
@@ -1029,7 +1029,7 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args
                entryOut.Values, err = unmarshalCorpusFile(entryOut.Data)
                h := sha256.Sum256(entryOut.Data)
                name := fmt.Sprintf("%x", h[:4])
-               entryOut.Name = name
+               entryOut.Path = name
                entryOut.Parent = entryIn.Parent
                entryOut.Generation = entryIn.Generation
                if err != nil {
@@ -1092,8 +1092,8 @@ func (wc *workerClient) fuzz(ctx context.Context, entryIn CorpusEntry, args fuzz
                h := sha256.Sum256(dataOut)
                name := fmt.Sprintf("%x", h[:4])
                entryOut = CorpusEntry{
-                       Name:       name,
-                       Parent:     entryIn.Name,
+                       Parent:     entryIn.Path,
+                       Path:       name,
                        Data:       dataOut,
                        Generation: entryIn.Generation + 1,
                }
index 60e2603da993e7837b3cff335bbc859b370376fc..40b77c133136b46a85101cd5dc89f3791390910a 100644 (file)
@@ -80,7 +80,7 @@ var _ TB = (*F)(nil)
 // import internal/fuzz from testing.
 type corpusEntry = struct {
        Parent     string
-       Name       string
+       Path       string
        Data       []byte
        Values     []interface{}
        Generation int
@@ -259,7 +259,7 @@ func (f *F) Add(args ...interface{}) {
                }
                values = append(values, args[i])
        }
-       f.corpus = append(f.corpus, corpusEntry{Values: values, IsSeed: true, Name: fmt.Sprintf("seed#%d", len(f.corpus))})
+       f.corpus = append(f.corpus, corpusEntry{Values: values, IsSeed: true, Path: fmt.Sprintf("seed#%d", len(f.corpus))})
 }
 
 // supportedTypes represents all of the supported types which can be fuzzed.
@@ -369,16 +369,16 @@ func (f *F) Fuzz(ff interface{}) {
        // fn is called in its own goroutine.
        run := func(e corpusEntry) error {
                if e.Values == nil {
-                       // Every code path should have already unmarshaled Data into Values.
-                       // It's our fault if it didn't.
-                       panic(fmt.Sprintf("corpus file %q was not unmarshaled", e.Name))
+                       // The corpusEntry must have non-nil Values in order to run the
+                       // test. If Values is nil, it is a bug in our code.
+                       panic(fmt.Sprintf("corpus file %q was not unmarshaled", e.Path))
                }
                if shouldFailFast() {
                        return nil
                }
-               testName := f.common.name
-               if e.Name != "" {
-                       testName = fmt.Sprintf("%s/%s", testName, e.Name)
+               testName := f.name
+               if e.Path != "" {
+                       testName = fmt.Sprintf("%s/%s", testName, filepath.Base(e.Path))
                }
 
                // Record the stack trace at the point of this call so that if the subtest
@@ -448,9 +448,10 @@ func (f *F) Fuzz(ff interface{}) {
                        f.Fail()
                        fmt.Fprintf(f.w, "%v\n", err)
                        if crashErr, ok := err.(fuzzCrashError); ok {
-                               crashName := crashErr.CrashName()
-                               fmt.Fprintf(f.w, "Crash written to %s\n", filepath.Join(corpusDir, f.name, crashName))
-                               fmt.Fprintf(f.w, "To re-run:\ngo test %s -run=%s/%s\n", f.fuzzContext.deps.ImportPath(), f.name, crashName)
+                               crashPath := crashErr.CrashPath()
+                               fmt.Fprintf(f.w, "Crash written to %s\n", crashPath)
+                               testName := filepath.Base(crashPath)
+                               fmt.Fprintf(f.w, "To re-run:\ngo test %s -run=%s/%s\n", f.fuzzContext.deps.ImportPath(), f.name, testName)
                        }
                }
                // TODO(jayconrod,katiehockman): Aggregate statistics across workers
@@ -470,7 +471,10 @@ func (f *F) Fuzz(ff interface{}) {
                // Fuzzing is not enabled, or will be done later. Only run the seed
                // corpus now.
                for _, e := range f.corpus {
-                       run(e)
+                       name := fmt.Sprintf("%s/%s", f.name, filepath.Base(e.Path))
+                       if _, ok, _ := f.testContext.match.fullName(nil, name); ok {
+                               run(e)
+                       }
                }
        }
 }
@@ -516,11 +520,11 @@ type fuzzCrashError interface {
        error
        Unwrap() error
 
-       // CrashName returns the name of the subtest that corresponds to the saved
-       // crash input file in the seed corpus. The test can be re-run with
-       // go test $pkg -run=$target/$name where $pkg is the package's import path,
-       // $target is the fuzz target name, and $name is the string returned here.
-       CrashName() string
+       // CrashPath returns the path of the subtest that corresponds to the saved
+       // crash input file in the seed corpus. The test can be re-run with go test
+       // -run=$target/$name $target is the fuzz target name, and $name is the
+       // filepath.Base of the string returned here.
+       CrashPath() string
 }
 
 // fuzzContext holds fields common to all fuzz targets.