kola/harness.go: fix kola panics #2710
Conversation
|
@saqibali-2k: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest-required |
Wow. Nice work finding the needle in the haystack. This makes my head hurt a little. Are there other problems that could have been caused by us not doing this appropriately? ^^ Need to update the link to point at the issue. |
| @@ -1276,17 +1276,17 @@ func makeNonExclusiveTest(bucket int, tests []*register.Test, flight platform.Fl | |||
| t := t | |||
| run := func(h *harness.H) { | |||
| testResults.add(h) | |||
There was a problem hiding this comment.
Can we comment here in the code why this newTC is needed?
Recently we have seen non-exclusive tests panicking due to runtime.Goexit executing, but somehow harness.finished being false. This problem was occurring because we pass tcluster to collectLogsExternalTest instead of newTC. tcluster has a reference to the harness of the wrapper test, thus harness.finished is incorrectly set to true for the wrapper test's harness instead of the non-exclusive subtest's harness. closes: coreos#2701
saqibali-2k
force-pushed
the
pr/kola-panics
branch
from
fd7c59a
to
c1e0cb0
Compare
|
This passed CI in a previous iteration. The new iteration just added comments so no code changes. |
Recently we have seen non-exclusive tests panicking due to
runtime.Goexit executing, but somehow harness.finished being false.
This problem was occurring because we pass tcluster to collectLogsExternalTest
instead of newTC. tcluster has a reference to the harness of the wrapper test,
thus harness.finished is incorrectly set to true for the wrapper test's harness
instead of the non-exclusive subtest's harness.
closes: #2701