suite: Experimental - Fuzz test support #1142

asaha123 · 2022-01-07T02:33:53Z

Summary

Add support for fuzz tests as introduced in Go 1.18 (still beta) and documented here. Some additional implementation details are available in this blog post.

Changes

TBD

Motivation

Support fuzz tests in testify

Example usage

Example test:

package foo

import (
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/suite"
)

type ExampleTestSuite struct {
	suite.Suite
	VariableThatShouldStartAtFive int
}

func (suite *ExampleTestSuite) SetupTest() {
	suite.VariableThatShouldStartAtFive = 5
}

func (suite *ExampleTestSuite) TestExample() {
	assert.Equal(suite.T(), 5, suite.VariableThatShouldStartAtFive)
	suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

type ExampleFuzzSuite struct {
	suite.Suite
}

func (suite *ExampleFuzzSuite) FuzzAFunc() {
	f := suite.F()
	f.Add("foo")
	f.Fuzz(func(t *testing.T, aStr string) {
		// this fails the test run when run in fuzz mode
		//t.Fail()

		// this doesn't fail the test
		suite.EqualError(nil, "an error")
	})
}

func TestExampleTestSuite(t *testing.T) {
	suite.Run(t, new(ExampleTestSuite))
}

func FuzzSuite(f *testing.F) {
	suite.Run(f, new(ExampleFuzzSuite))
}

With this modification to testify, the above test run in fuzz mode looks as follows:

$ go1.18beta1 test -fuzz=FuzzSuite -fuzztime=1s
fuzz: elapsed: 0s, gathering baseline coverage: 0/300 completed
fuzz: elapsed: 0s, gathering baseline coverage: 300/300 completed, now fuzzing with 8 workers
fuzz: elapsed: 1s, execs: 18028 (16323/sec), new interesting: 0 (total: 298)
PASS
ok  	test	1.308s

I expected the test to fail since i have suite.EqualError(nil, "an error').

In "unit test" mode, the test fails:

% go1.18beta1 test                             
--- FAIL: FuzzAFunc (0.00s)
    f_test.go:36: 
        	Error Trace:	f_test.go:36
        	            				value.go:556
        	            				value.go:339
        	            				fuzz.go:332
        	Error:      	An error is expected but got nil.
        	Test:       	FuzzAFunc
    f_test.go:36: 
        	Error Trace:	f_test.go:36
        	            				value.go:556
        	            				value.go:339
        	            				fuzz.go:332
        	Error:      	An error is expected but got nil.
        	Test:       	FuzzAFunc
--- FAIL: FuzzSuite (0.00s)
FAIL
exit status 1
FAIL	test	0.165s

This runs the fuzz tests in fuzz test mode, but doesn't fail when any of the suite assertion methods are used.

boyan-soubachov

Thanks for the contribution! 🙂

I've put down some questions to better understand the decisions you've made.

boyan-soubachov · 2022-02-10T10:21:45Z

suite/suite.go

+	suite.Assertions = assert.New(f)
+	suite.require = require.New(f)


I think this might be problematic. If we called suite.SetT(t) then called suite.SetF(), would that not lose the "T" we set in SetT(t)?

If so, it seems like that would be the wrong behaviour? Should we not find a way to add an "F" or "T" if the relevant Assertions and requires have already been instantiated?

Good point. The current change assumes that you will not have a situation like that. From my current very limited understanding of both testify and go fuzz tests, for having a suite of fuzz tests, your suite's entrypoint "test function" will accept a testing.F type parameter. The fuzz target has a testing.T context, but that's not relevant i think to testify as that gets invoked by the go test runner.

boyan-soubachov · 2022-02-10T10:26:26Z

suite/suite.go

+		if suite.T() != nil {
+			tT = suite.T()
+		}
+		if suite.F() != nil {


Why are we checking for the existence of suite.F and suite.T when previously we weren't?

Is it possible that the existence of both of those should be guaranteed before the code reaches this point (i.e. when suite is first instantiated)?

At this stage, the current suite under test could either be a fuzz test suite (either in normal/fuzz mode) or a non-fuzz test suite. Hence, depending on which mode we are in, initialize suite.require appropriately. Hence, the current panic() where i assert that at least one of them is set.

Again, this assumes that during a single run, for a suite, it can only have testing.T or testing.F set.

boyan-soubachov · 2022-02-10T10:28:34Z

suite/suite.go

+		if tT == nil {
+			panic("Neither T nor F was non-nil")
+		}


I'm not sure why we want to test for this? Is it not possible for a suite to have both non-nil T and F?

Again, same assumption as above.

boyan-soubachov · 2022-02-10T10:29:19Z

suite/suite.go

 	}
 	return suite.Assertions
 }

-func failOnPanic(t *testing.T) {
+func failOnPanic(t testing.TB) {


What's the logic for expanding the interface to include benchmarks and removing the pointer here?

When we run a fuzz test we have a testing.F injected into the test binary instead of testing.T.

The reason for replacing *testing.T by testing.TB is so that failOnPanic() can be called with a testing.F value as well (testing.TB interface is satisfied by T, F and B source)

boyan-soubachov · 2022-02-10T10:29:47Z

suite/suite.go

@@ -80,10 +112,17 @@ func (suite *Suite) Run(name string, subtest func()) bool {

 // Run takes a testing suite and runs all of the tests attached
 // to it.
-func Run(t *testing.T, suite TestingSuite) {
+func Run(t testing.TB, suite TestingSuite) {


Same question as line 92

Hopefully the above also explains this.

boyan-soubachov · 2022-02-10T10:31:51Z

suite/suite.go

@@ -193,12 +232,13 @@ func runTests(t testing.TB, tests []testing.InternalTest) {

 	r, ok := t.(runner)
 	if !ok { // backwards compatibility with Go 1.6 and below


We don't officially support Go 1.6 anymore. Can we remove this check altogether or at least remove the old backwards compatibility change and update this to reflect 1.18 and fuzzing?

Sounds like a good idea. Should I create a separate change to remove this and rebase on top it?

asaha123 · 2022-03-08T02:28:35Z

suite/suite.go

@@ -193,12 +232,13 @@ func runTests(t testing.TB, tests []testing.InternalTest) {

 	r, ok := t.(runner)
 	if !ok { // backwards compatibility with Go 1.6 and below
+		// t.F doesn't have Run() so Fuzz tests also
+		// take this branch
 		if !testing.RunTests(allTestsFilter, tests) {
 			t.Fail()


What i am observing is this which explains the behavior I am seeing (which may have been clear to you folks, but i am seeing why now):

When the fuzz test is run in "unit test" mode, the function call RunTests() returns with a false value and hence the test fails (considering the sample test in the PR)

When the fuzz test is run in fuzz mode, this function call, RunTests() returns true (considering the sample test in the PR)

When I uncomment the t.Fail() in the test, the test fails in fuzz mode, however, the return value of RunTests() is still true.

Okay, I think i have found the issue. The problem seems to be that when we call the suite.EqualError() method, it reports any failures using the testing context's (testing.T value) methods. However, the problem is since we are running a Fuzz test, we only have a testing.F injected via go test. The testing.T value is created and injected into the fuzz target as part of the test run. So, we need to make this testing.T created as part of the test run available to testify.

I am going to next experiment with updating the test discovery and running logic using https://pkg.go.dev/testing@go1.18beta2#InternalFuzzTarget for the fuzz tests. i think that may be required and certainly feels correct.

I don't think that's going to help. That is an internal type which refers to the "internal" fuzz target function.

Okay, I think i have found the issue. The problem seems to be that when we call the suite.EqualError() method, it reports any failures using the testing context's (testing.T value) methods. However, the problem is since we are running a Fuzz test, we only have a testing.F injected via go test. The testing.T value is created and injected into the fuzz target as part of the test run. So, we need to make this testing.T created as part of the test run available to testify.

Hence, when i update my test as follows to make a call to the SetT() method, it fails as expected in fuzz mode as well:

type ExampleFuzzSuite struct { suite.Suite } func (suite *ExampleFuzzSuite) FuzzAFunc() { f := suite.F() f.Add("foo") f.Fuzz(func(t *testing.T, aStr string) { fmt.Println("I am in Fuzz target") // this fails the test run when run in fuzz mode //t.Fail() // this doesn't fail the test when run in fuzz mode if i comment // out the next line: https://github.com/stretchr/testify/pull/1142/files#r821269235 suite.SetT(t) suite.EqualError(nil, "an error") }) } func TestExampleTestSuite(t *testing.T) { suite.Run(t, new(ExampleTestSuite)) } func FuzzSuite(f *testing.F) { suite.Run(f, new(ExampleFuzzSuite)) }

asaha123 · 2022-03-08T05:07:33Z

Thanks for the contribution! slightly_smiling_face

I've put down some questions to better understand the decisions you've made.

Thanks for the review so far. I have replied to the comments.

I suspect we may have run into an issue with how we handle assertions and report failures using testing.T value currently and it's workings in the context of fuzz tests: https://github.com/stretchr/testify/pull/1142/files#r821324429

mcauto · 2022-12-13T05:17:00Z

How's it going?

Go supports fuzzing in its standard toolchain beginning in Go 1.18. Native Go fuzz tests are supported by OSS-Fuzz.

https://go.dev/security/fuzz/

asaha123 added 2 commits December 23, 2021 11:50

Support for discovering fuzz tests

51a8160

This runs the fuzz tests in fuzz test mode, but doesn't fail when any of the suite assertion methods are used.

update

596ee11

boyan-soubachov requested changes Feb 10, 2022

View reviewed changes

asaha123 commented Mar 8, 2022

View reviewed changes

dolmen changed the title ~~Experimental - Fuzz test support~~ suite: Experimental - Fuzz test support Oct 30, 2023

dolmen added the pkg-suite Change related to package testify/suite label Oct 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

suite: Experimental - Fuzz test support #1142

suite: Experimental - Fuzz test support #1142

asaha123 commented Jan 7, 2022 •

edited

Loading

boyan-soubachov left a comment

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022 •

edited

Loading

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022 •

edited

Loading

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022 •

edited

Loading

boyan-soubachov Feb 10, 2022

asaha123 Mar 7, 2022

asaha123 Mar 8, 2022 •

edited

Loading

asaha123 Mar 8, 2022 •

edited

Loading

asaha123 Mar 8, 2022 •

edited

Loading

asaha123 Mar 8, 2022

asaha123 commented Mar 8, 2022

mcauto commented Dec 13, 2022

		suite.Assertions = assert.New(f)
		suite.require = require.New(f)

		@@ -193,12 +232,13 @@ func runTests(t testing.TB, tests []testing.InternalTest) {

		r, ok := t.(runner)
		if !ok { // backwards compatibility with Go 1.6 and below

suite: Experimental - Fuzz test support #1142

Are you sure you want to change the base?

suite: Experimental - Fuzz test support #1142

Conversation

asaha123 commented Jan 7, 2022 • edited Loading

Summary

Changes

Motivation

Example usage

boyan-soubachov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asaha123 Mar 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asaha123 Mar 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asaha123 Mar 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asaha123 Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

asaha123 Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

asaha123 Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asaha123 commented Mar 8, 2022

mcauto commented Dec 13, 2022

asaha123 commented Jan 7, 2022 •

edited

Loading

asaha123 Mar 7, 2022 •

edited

Loading

asaha123 Mar 7, 2022 •

edited

Loading

asaha123 Mar 7, 2022 •

edited

Loading

asaha123 Mar 8, 2022 •

edited

Loading

asaha123 Mar 8, 2022 •

edited

Loading

asaha123 Mar 8, 2022 •

edited

Loading