damccorm commented on code in PR #17587:
URL: https://github.com/apache/beam/pull/17587#discussion_r868204673
##########
sdks/go/pkg/beam/core/graph/coder/stringutf8_test.go:
##########
@@ -120,3 +120,26 @@ func TestEncodeDecodeStringUTF8LP(t *testing.T) {
})
}
}
+
+func FuzzEncodeDecodeStringUTF8LP(f *testing.F) {
Review Comment:
Is there a reason to keep the above test that is doing the same thing as
this one? Same question for the tests in other files
##########
sdks/go/pkg/beam/core/graph/coder/bytes_test.go:
##########
@@ -59,3 +59,23 @@ func TestEncodeDecodeBytes(t *testing.T) {
})
}
}
+
+func FuzzEncodeDecodeBytes(f *testing.F) {
Review Comment:
Looking at https://go.dev/doc/fuzz/, it looks like these tests won't be run
automatically since we're not passing in the fuzz flag, right? We'll just run
the [seed corpus](https://go.dev/doc/fuzz/#glos-seed-corpus)?
This is neat and probably helps us a bit, but investing heavily in tests
that don't get run continuously has a pretty limited ceiling since it can't be
used to prove correctness of future changes without taking non-obvious manual
steps. We should definitely keep our eyes on [this issue to enable continuous
fuzzing](https://github.com/golang/go/issues/50192) - until we get there, I'd
probably vote we don't make too big of a push towards adding a bunch of fuzz
tests. Alternately, we could consider adding our fuzz tests to the set of
validations that gets done before a release.
Again - this PR is great and provides a valuable map towards adding fuzz
tests, I just want to make sure we're calling out the downsides before going
too far down that road.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
