google / gofuzz

Fuzz testing for go.
Apache License 2.0
1.5k stars 119 forks source link

Allow Continue.Fuzz to accept a reflect.Value #51

Closed dnephin closed 4 years ago

dnephin commented 4 years ago

This is a small optimization, so I completely understand if you would rather keep the existing interface and close this PR without merging. I made the change before I realized I could use reflect.Value.Addr() as the argument. I thought I would open this PR anyway, in case there is interest in the small optimization. I imagine in some larger test suites it might save a little time by avoiding the need to reflect again.

Continue.Fuzz (and FuzzNoCustom) immediately convert the interface{} into a reflect.Value. In some cases the caller may already have a reflect.Value, so accepting that type makes it easier to write custom fuzz functions.

One use case for this is ignoring a field on a struct. The SkipFieldsWithPattern option is a good choice when the field name should always be ignored. In other cases a field name may exist on multiple structs, and using a regex pattern would result in the field being skipped in all cases. To ignore a field by name on only a single struct a custom fuzz function can use reflect to iterate over the fields, and call c.Fuzz() on all fields except for the ignored field.

dnephin commented 4 years ago

Ah, the new test is flaky because fuzz can set it to the zero value. If there is interest in this change I will see about fixing that. I guess there are a couple other tests that have this same problem.

lavalamp commented 4 years ago

I think this is a reasonable change and I'll take it if you fix the test :)

dnephin commented 4 years ago

Awesome! I changed the test to use the existing tryFuzz helper. I only had to make a small change to it to accept a Continue as well as a Fuzzer, and added the t.Helper() call.

lavalamp commented 4 years ago

LGTM, thanks!