New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Fix artifact collection for bad log failures #16489
Conversation
When a test fails because a bad log message is found [1], the per-test artifacts are not collected and exposed in the Jenkins UI. This issue is visible at [2] at the time of writing this. We only collect per-test artifacts for failing tests, which we detect with ginkgo.CurrentGinkgoTestDescription().Failed. Unfortunately, this variable is not updated when the test fails in a JustAfterEach block. That's the case for the bad log messages, which are almost always checked from a JustAfterEach block. This commit fixes that issue by using the dedicated function TestFailed instead of ginkgo.CurrentGinkgoTestDescription().Failed. At the time of writing this, the result can be seen at [3]. This issue was discovered before in da25f94 ("k8sT: Clean up services in JustAfterEach"), but the fix was not extended to the existing code at the time. 1 - https://github.com/cilium/cilium/blob/v1.10.0/test/helpers/cons.go#L291 2 - https://jenkins.cilium.io/job/Cilium-PR-Runtime-4.9/4917/testReport/junit/(root)/Suite-runtime/RuntimeFQDNPolicies_toFQDNs_populates_toCIDRSet__data_from_proxy__L3_dependent_L7_HTTP_with_toFQDN_updates_proxy_policy/ 3 - https://jenkins.cilium.io/job/Cilium-PR-K8s-1.20-kernel-4.19/637/testReport/junit/Suite-k8s-1/20/K8sServicesTest_Checks_service_across_nodes_Supports_IPv4_fragments/ Signed-off-by: Paul Chaignon <paul@cilium.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaaaah so that's why! Thanks a lot for finding this :D
I've already validated at https://jenkins.cilium.io/job/Cilium-PR-K8s-1.20-kernel-4.19/637/testReport/junit/Suite-k8s-1/20/K8sServicesTest_Checks_service_across_nodes_Supports_IPv4_fragments/ that this fix works, so no need to run the full test suite. Marking as ready to merge. |
Marking for backports as it can significantly help flake debugging. |
When a test fails because a bad log message is found, the per-test artifacts are not collected and exposed in the Jenkins UI.
We only collect per-test artifacts for failing tests, which we detect with
ginkgo.CurrentGinkgoTestDescription().Failed
. Unfortunately, this variable is not updated when the test fails in aJustAfterEach
block. That's the case for the bad log messages, which are almost always checked from aJustAfterEach
block.This pull request fixes that issue by using the dedicated function
TestFailed()
instead ofginkgo.CurrentGinkgoTestDescription().Failed
.This issue was discovered by @joestringer before in da25f94 ("k8sT: Clean up services in JustAfterEach"), but the fix was not extended to the existing code at the time.
Before:
After: