NO-JIRA: fixed e2e teardown failing the outer test instead of subtest #3891
Conversation
|
@muraee: This pull request explicitly references no jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
area/testing
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: muraee The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/test e2e-aws |
|
/retest-required |
|
/test e2e-kubevirt-aws-ovn |
|
/retest |
|
/lgtm |
| // as it is called during the cleanup phase where t.Run() is not supported. | ||
| func (h *hypershiftTest) teardownHostedCluster(ctx context.Context, hc *hyperv1.HostedCluster, opts *core.CreateOptions, artifactDir string) { | ||
| // NOTE: teardownHostedCluster shouldn't start any subtests with t.Run() when cleanupPhase=True, this is not a supported operation and will fail immediately | ||
| func teardownHostedCluster(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluster, client crclient.Client, opts *core.CreateOptions, artifactDir string, cleanupPhase bool) { | ||
| // TODO (Mulham): dumpCluster() uses testName to construc dumpDir, since we removed sub tests from this function |
There was a problem hiding this comment.
Is this still relevant and could we do it as part of this pr or is it bigger piece of work?
There was a problem hiding this comment.
hmm, not sure if we still need this.
Basically we get 1 dump currently before we destroy the cluster and save it to <artifactDir>/<testName>. but in the rare cases, where the destroy fails, another dump is generated, but currently without this, its saved in the same path overriding the first one.
There was a problem hiding this comment.
I am not sure whether we need all those different dumps to debug any issue.
There was a problem hiding this comment.
I think it's nice to have if we ever need it but not a priority. We can create a Jira for this and stick it in the backlog, wdyt?
|
@muraee: all tests passed! 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. |
What this PR does / why we need it:
Previously, an error during
Teardownsubtest would cause the main outer test to fail directly and the subtest to succeed, which makes it difficult to identify the source of the error.Which issue(s) this PR fixes (optional, use
fixes #<issue_number>(, fixes #<issue_number>, ...)format, where issue_number might be a GitHub issue, or a Jira story:Fixes #
Checklist