Extend fuzz tests #196
Extend fuzz tests #196
Conversation
Covering more methods for CellUnion and add a rudimentary fuzz test for polygon.
s2/encode_fuzz_test.go
Outdated
| if got := p.Area(); got < 0 { | ||
| t.Errorf("Area() = %v, want >= 0. Polygon: %v", got, p) | ||
| } | ||
| // TODO: Test more methods on Polygon. |
There was a problem hiding this comment.
What's the rationale for
- one file with all fuzz tests (or all decoding related fuzz tests)?
- having monolithic
FuzzFootests rather than testing properties independently?
?
http://go/unit-testing-practices?polyglot=go#behavior-testing-examples
There was a problem hiding this comment.
-
That's mostly me being oldschool. In the past, I added fuzz tests to a project that supported earlier go versions that did not yet support fuzz testing, so the fuzz tests needed to be in a seperate file, where a minimum go version was enforced. In this project, we might want to move the fuzz tests to the respective type test, e.g. polygon_test.go. Do you have a preference?
-
Not sure what you mean. The primary thing under test is
Decode: Will it always create a valid X? The properties we check on the decoded object feel secondary, hence I feel checking them in FuzzDecodeX is approrpiate.
There was a problem hiding this comment.
- Seems better to keep all polygon tests together rather than all fuzz tests together.
- You're not testing a single property. You're testing forall bytes decode doesn't crash && forall polygons area non-negative && more in the future. This seems partially due to the limited set of types supported by fuzzing. Maybe it's also computationally efficient depending on how hard it is for the fuzzer to find valid polygons.
There was a problem hiding this comment.
- Done
- I think this is correct. With every pass, the fuzzer tries to find new interesting examples that execute more code paths. Having a large variety of code paths executed inside the fuzz test is preferable.
Covering more methods for CellUnion and add a rudimentary fuzz test for polygon.