Fix nil pointer dereferences in assertion functions and add fuzz tests#126
Conversation
…d fuzz tests Add nil-safety checks to assertion functions that dereference reflect.TypeOf() results without verifying they are non-nil: - collection.go: isList() panicked when list was nil - compare.go: Positive()/Negative() panicked when value was nil - order.go: isStrictlyOrdered() panicked when collection was nil - type.go: Implements()/NotImplements() panicked when interfaceObject was nil Add property-based and fuzz tests in internal/testintegration/assertions/ using pgregory.net/rapid to exercise assertion functions with arbitrary values (including nil) and verify they never panic. Signed-off-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
fredbi
left a comment
There was a problem hiding this comment.
Linter is complaining
Error: /home/runner/work/testify/testify/internal/assertions/order.go:325:22: error-format: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, false, fmt.Errorf("object is not an ordered collection")
Signed-off-by: GitHub Copilot <noreply@github.com>
Fixed — replaced |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #126 +/- ##
==========================================
- Coverage 91.69% 91.67% -0.02%
==========================================
Files 97 97
Lines 12601 12620 +19
==========================================
+ Hits 11555 11570 +15
- Misses 825 827 +2
- Partials 221 223 +2 ☔ View full report in Codecov by Harness. |
fredbi
left a comment
There was a problem hiding this comment.
Integration test is failing.
this job is failing on data races in the rapid/property tests under internal/testintegration/v2/assertions, not on assertion mismatches.
From the logs:
testing.go:1712: race detected during execution of test
Multiple tests fail with that same race signal:
TestNilSafetyUnary
TestNilSafetyBinary
TestNilSafetyCollections
TestNilSafetyComparison
TestNilSafetyType
TestNilSafetyExportedValues
The “fail file is no longer valid” lines are secondary (rapid replay artifact invalidation), not the primary breakage.
What to fix
The tests likely call assertion helpers that mutate shared/global state (or shared buffers/formatters) while rapid runs many generated cases. Under -race, this trips immediately.
Target file
Workflow: .github/workflows/go-test.yml (from job metadata)
Failing tests: internal/testintegration/v2/assertions/assertions_test.go (per stack lines in logs)
…riable Signed-off-by: GitHub Copilot <noreply@github.com>
The race was caused by a package-level |
Several assertion functions panic with a nil pointer dereference when passed
nilvalues, due to uncheckedreflect.TypeOf()calls (which returnsnilfor untypednil).Nil-safety fixes
isList()—reflect.TypeOf(list).Kind()panics on nil listPositive()/Negative()—reflect.Zero(reflect.TypeOf(e))panics on nil valueisStrictlyOrdered()— samereflect.TypeOf(object).Kind()patternImplements()/NotImplements()—reflect.TypeOf(interfaceObject).Elem()panics on nilAll fixed by adding a nil check before dereferencing, returning a clear failure message instead of panicking:
Fuzz tests
Added
internal/testintegration/assertions/with property-based tests (pgregory.net/rapid) and a Go native fuzz target exercising assertion functions with arbitrary values including nil, nil pointers, typed nils, and diverse container types. These tests discovered all four bugs above.