test: Only call Fail() once for all error logs #11184
Conversation
We've recently expanded the fail cases for when different logs are seen in the cilium-agent logs. However at the same time we also introduced a behaviour where for every single instance of the failure in the logs, we will call Fail(). In local CI runs with `--holdEnvironment` this can lead to a loop where the test is suspended for debugging, the developer performs ^Z and `fg`, then the same failure prints again, potentially many times over before the test can finally proceed / clean up. Tidy it up by detecting any failure of this sort once, then failing out. Note that the actual failure logs should already be visible earlier in the test output so this should not hide the failures from the developer. Signed-off-by: Joe Stringer <joe@cilium.io>
joestringer
added
area/CI
|
test-me-please EDIT: CI:
Looks like bad luck with quay.io outage. Will re-try. |
|
test-me-please |
There was a problem hiding this comment.
Thanks for fixing this! One less item on my todo list :-)
Note that the actual failure logs should already be visible earlier in
the test output so this should not hide the failures from the developer.
Are you referring to the Top 5 errors/warnings? Apart from that top 5 (and only if they are frequent enough), I don't think bad log messages are printed on test failures. You have to dig through the logs to find them, no?
At least locally I found that they were printed twice in the output. It's possible that one goes to stdout and the other goes to the ginkgoprinter. If the representation is not obvious in the jenkins side output then we may need to just make sure we direct that output to the correct writer (might be a matter of just adding an extra log printing statement to the previous fail location). |
We've recently expanded the fail cases for when different logs are seen
in the cilium-agent logs. However at the same time we also introduced
a behaviour where for every single instance of the failure in the logs,
we will call Fail(). In local CI runs with
--holdEnvironmentthis canlead to a loop where the test is suspended for debugging, the developer
performs ^Z and
fg, then the same failure prints again, potentiallymany times over before the test can finally proceed / clean up.
Tidy it up by detecting any failure of this sort once, then failing out.
Note that the actual failure logs should already be visible earlier in
the test output so this should not hide the failures from the developer.