testing: fix -run behavior with fuzz tests
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>
diff --git a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt b/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
index 79476ec..628e003 100644
--- a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
+++ b/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
@@ -21,7 +21,7 @@
# 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
index 0000000..e546d99
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_fuzz_run.txt
@@ -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
diff --git a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt
index a66d055..5d04d8c 100644
--- a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt
+++ b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt
@@ -26,6 +26,7 @@
! 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
diff --git a/src/internal/fuzz/fuzz.go b/src/internal/fuzz/fuzz.go
index f660052..a8bbd60 100644
--- a/src/internal/fuzz/fuzz.go
+++ b/src/internal/fuzz/fuzz.go
@@ -141,14 +141,14 @@
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 @@
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 @@
} 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 @@
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 @@
// 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 @@
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 @@
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 @@
// 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 @@
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 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 @@
return ce.Data, nil
}
- return os.ReadFile(ce.Name)
+ return os.ReadFile(ce.Path)
}
type fuzzInput struct {
@@ -616,7 +616,7 @@
}
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 @@
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 @@
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 @@
// 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 @@
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{} {
diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index 1429dec..0c428ed 100644
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -1029,7 +1029,7 @@
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 @@
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,
}
diff --git a/src/testing/fuzz.go b/src/testing/fuzz.go
index 60e2603..40b77c1 100644
--- a/src/testing/fuzz.go
+++ b/src/testing/fuzz.go
@@ -80,7 +80,7 @@
// import internal/fuzz from testing.
type corpusEntry = struct {
Parent string
- Name string
+ Path string
Data []byte
Values []interface{}
Generation int
@@ -259,7 +259,7 @@
}
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 @@
// 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 @@
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 @@
// 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 @@
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.
