tests: replace panic with t.Fatal, upgrade to latest base image for vmtests and fix enforcer tests
#2685
Conversation
Panicking in tests is bad as it fails to tell the test handler that the test failed. Instead we should use t.Fatal(). This commit changes instances of panic in tests to t.Fatal(). Signed-off-by: Kevin Sheldrake <[email protected]>
kevsecurity
added
the
release-note/misc
There was a problem hiding this comment.
Okay good so the CI is failing as expected now. I still don't understand why "panic" made it not fail on main and v1.1, but fail on v1.0. I have a patch to add the netcat.openbsd cilium/little-vm-helper-images#582 on base image which is a reasonable fix that we can integrate into that PR when it's ready.
Is it okay if I push to this PR?
Push away; I made it for you! |
|
Ok thanks. |
For the base image to have nc.openbsd, see cilium/little-vm-helper-images@2af406c. Signed-off-by: Mahe Tardy <[email protected]>
Because of root-images update in previous commit, the enforcer security tests started failing, after investigation we realized that the difference was that the old image mounted /tmp on the disk fs and the new one with tmpfs. Then the direct-write-tester.c program was failing using O_DIRECT (because apparently it fails on tmpfs). It failed silently on recent linux versions however because O_DIRECT on tmpfs might just be a noop. /var/tmp should be backed by disk so it should be a fix for our issue. Signed-off-by: Mahe Tardy <[email protected]>
O_DIRECT is not posix so we need to add _GNU_SOURCE Co-authored-by: Jiri Olsa <[email protected]> Signed-off-by: Mahe Tardy <[email protected]>
mtardy
force-pushed
the
pr/kevsecurity/change-panic-to-fatal-in-tests
branch
from
c7c7e52 to
f7f227e
Compare
Use the output returned by Fatal instead of passing stuff to printf Signed-off-by: Mahe Tardy <[email protected]>
mtardy
changed the title
panic with t.Fatal, upgrade to latest base image for vmtests and fix enforcer tests
Related to #2686.
Panicking in tests is bad as it fails to tell the test handler that the test failed. Instead we should use t.Fatal(). This commit changes instances of panic in tests to t.Fatal().