cilium / tetragon

eBPF-based Security Observability and Runtime Enforcement
https://tetragon.io
Apache License 2.0
3.66k stars 369 forks source link

Inappropriate uses of assert.NoError? #3125

Open knrc opened 6 days ago

knrc commented 6 days ago

While completing a simple PR as part of the KubeCon contribfest I hit an issue where one test was causing a panic.

I tracked this down to the use of assert.NoError(t, err) for checking errors, but instead of being used as a guard for the subsequent test code it continued as if the error did not occur and dereferenced a nil. It looks as if this may occur often within the project tests.

sayboras commented 6 days ago

Just wanna share the experience in cilium/cilium. I think the value of assert.NoError is to gather all the outputs/failures at once, so that developer can fix in one shot. However, it requires a careful thinking to make sure there is no side effect or misleading or even panic, hence in cilium/cilium most of assert. is changed to require. recently in main branch.