[dev.fuzz] testing: panic if certain testing.F functions are called in Fuzz func

Change-Id: I8ee513b2b157e6033d4bc9607d0e65f42bd6801f
Reviewed-on: https://go-review.googlesource.com/c/go/+/259657
Trust: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
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.txt b/src/cmd/go/testdata/script/test_fuzz.txt
index ab93f5cc2efd21d1f19a0dde4c92bfd324efae5c..d9aa718987ee8e4c94198b8e9f9534f84ae0cfd5 100644 (file)
--- a/src/cmd/go/testdata/script/test_fuzz.txt
+++ b/src/cmd/go/testdata/script/test_fuzz.txt
@@ -36,6 +36,27 @@ go test skipped_fuzz_test.go
 stdout ok
 ! stdout FAIL
 
+# Test that f.Fatal within f.Fuzz panics
+! go test fatal_fuzz_fn_fuzz_test.go
+! stdout ^ok
+! stdout 'fatal here'
+stdout FAIL
+stdout illegal
+
+# Test that f.Error within f.Fuzz panics
+! go test error_fuzz_fn_fuzz_test.go
+! stdout ^ok
+! stdout 'error here'
+stdout FAIL
+stdout illegal
+
+# Test that f.Skip within f.Fuzz panics
+! go test skip_fuzz_fn_fuzz_test.go
+! stdout ^ok
+! stdout 'skip here'
+stdout FAIL
+stdout illegal
+
 # Test that multiple calls to f.Fuzz causes a non-zero exit status.
 ! go test multiple_fuzz_calls_fuzz_test.go
 ! stdout ^ok
@@ -104,6 +125,42 @@ func Fuzz(f *testing.F) {
     f.Skip()
 }
 
+-- fatal_fuzz_fn_fuzz_test.go --
+package fatal_fuzz_fn_fuzz
+
+import "testing"
+
+func Fuzz(f *testing.F) {
+    f.Add([]byte("aa"))
+    f.Fuzz(func(t *testing.T, b []byte) {
+        f.Fatal("fatal here")
+    })
+}
+
+-- error_fuzz_fn_fuzz_test.go --
+package error_fuzz_fn_fuzz
+
+import "testing"
+
+func Fuzz(f *testing.F) {
+    f.Add([]byte("aa"))
+    f.Fuzz(func(t *testing.T, b []byte) {
+        f.Error("error here")
+    })
+}
+
+-- skip_fuzz_fn_fuzz_test.go --
+package skip_fuzz_fn_fuzz
+
+import "testing"
+
+func Fuzz(f *testing.F) {
+    f.Add([]byte("aa"))
+    f.Fuzz(func(t *testing.T, b []byte) {
+        f.Skip("skip here")
+    })
+}
+
 -- multiple_fuzz_calls_fuzz_test.go --
 package multiple_fuzz_calls_fuzz
 

diff --git a/src/testing/fuzz.go b/src/testing/fuzz.go
index 6f985c7c38140265d0bf81e87ceecb2e1a0945c3..01895e8d7d9d95cff5d2b308abb3d8296c0613ab 100644 (file)
--- a/src/testing/fuzz.go
+++ b/src/testing/fuzz.go
@@ -87,11 +87,16 @@ func (f *F) Fuzz(ff interface{}) {
                        }
                        if err != nil {
                                t.Fail()
-                               t.output = []byte(fmt.Sprintf("    panic: %s\n", err))
+                               t.output = []byte(fmt.Sprintf("    %s", err))
                        }
                        f.setRan()
+                       f.inFuzzFn = false
                        t.signal <- true // signal that the test has finished
                }()
+               // TODO(katiehockman, jayconrod): consider replacing inFuzzFn with
+               // general purpose flag that checks whether specific methods can be
+               // called.
+               f.inFuzzFn = true
                fn(t, b)
                t.finished = true
        }

diff --git a/src/testing/testing.go b/src/testing/testing.go
index 4fd628c0ff20afeddf7230eb3c36935d838f0182..7cf3323d51a6d1bf90dbaf19543b01244b43f557 100644 (file)
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -399,6 +399,7 @@ type common struct {
 
        chatty     *chattyPrinter // A copy of chattyPrinter, if the chatty flag is set.
        bench      bool           // Whether the current test is a benchmark.
+       inFuzzFn   bool           // Whether the test is executing a Fuzz function
        finished   bool           // Test function has completed.
        hasSub     int32          // Written atomically.
        raceErrors int            // Number of races detected during test.
@@ -681,6 +682,9 @@ func (c *common) setRan() {
 
 // Fail marks the function as having failed but continues execution.
 func (c *common) Fail() {
+       if c.inFuzzFn {
+               panic("testing: f.Fail was called inside the f.Fuzz function")
+       }
        if c.parent != nil {
                c.parent.Fail()
        }
@@ -710,6 +714,9 @@ func (c *common) Failed() bool {
 // created during the test. Calling FailNow does not stop
 // those other goroutines.
 func (c *common) FailNow() {
+       if c.inFuzzFn {
+               panic("testing: f.FailNow was called inside the f.Fuzz function")
+       }
        c.Fail()
 
        // Calling runtime.Goexit will exit the goroutine, which
@@ -787,36 +794,54 @@ func (c *common) Logf(format string, args ...interface{}) { c.log(fmt.Sprintf(fo
 
 // Error is equivalent to Log followed by Fail.
 func (c *common) Error(args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Error was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintln(args...))
        c.Fail()
 }
 
 // Errorf is equivalent to Logf followed by Fail.
 func (c *common) Errorf(format string, args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Errorf was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintf(format, args...))
        c.Fail()
 }
 
 // Fatal is equivalent to Log followed by FailNow.
 func (c *common) Fatal(args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Fatal was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintln(args...))
        c.FailNow()
 }
 
 // Fatalf is equivalent to Logf followed by FailNow.
 func (c *common) Fatalf(format string, args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Fatalf was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintf(format, args...))
        c.FailNow()
 }
 
 // Skip is equivalent to Log followed by SkipNow.
 func (c *common) Skip(args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Skip was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintln(args...))
        c.SkipNow()
 }
 
 // Skipf is equivalent to Logf followed by SkipNow.
 func (c *common) Skipf(format string, args ...interface{}) {
+       if c.inFuzzFn {
+               panic("testing: f.Skipf was called inside the f.Fuzz function")
+       }
        c.log(fmt.Sprintf(format, args...))
        c.SkipNow()
 }
@@ -830,6 +855,9 @@ func (c *common) Skipf(format string, args ...interface{}) {
 // other goroutines created during the test. Calling SkipNow does not stop
 // those other goroutines.
 func (c *common) SkipNow() {
+       if c.inFuzzFn {
+               panic("testing: f.SkipNow was called inside the f.Fuzz function")
+       }
        c.skip()
        c.finished = true
        runtime.Goexit()
@@ -852,6 +880,9 @@ func (c *common) Skipped() bool {
 // When printing file and line information, that function will be skipped.
 // Helper may be called simultaneously from multiple goroutines.
 func (c *common) Helper() {
+       if c.inFuzzFn {
+               panic("testing: f.Helper was called inside the f.Fuzz function")
+       }
        c.mu.Lock()
        defer c.mu.Unlock()
        if c.helpers == nil {
@@ -864,6 +895,9 @@ func (c *common) Helper() {
 // subtests complete. Cleanup functions will be called in last added,
 // first called order.
 func (c *common) Cleanup(f func()) {
+       if c.inFuzzFn {
+               panic("testing: f.Cleanup was called inside the f.Fuzz function")
+       }
        var pc [maxStackLen]uintptr
        // Skip two extra frames to account for this function and runtime.Callers itself.
        n := runtime.Callers(2, pc[:])
@@ -902,6 +936,9 @@ var tempDirReplacer struct {
 // Each subsequent call to t.TempDir returns a unique directory;
 // if the directory creation fails, TempDir terminates the test by calling Fatal.
 func (c *common) TempDir() string {
+       if c.inFuzzFn {
+               panic("testing: f.TempDir was called inside the f.Fuzz function")
+       }
        // Use a single parent directory for all the temporary directories
        // created by a test, each numbered sequentially.
        c.tempDirMu.Lock()