OCM-11127 | ci: fix id:73753,55883,75927 #2458
Conversation
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aaraj7, yingzhanredhat The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2458 +/- ##
=======================================
Coverage 29.35% 29.35%
=======================================
Files 161 161
Lines 22364 22364
=======================================
Hits 6566 6566
Misses 15253 15253
Partials 545 545 ☔ View full report in Codecov by Sentry. |
| @@ -3530,6 +3530,7 @@ var _ = Describe("Sts cluster creation supplemental testing", | |||
| testOperatorRolePrefix := common.GenerateRandomName("opp75927", 2) | |||
| flags, err := profilehandler.GenerateClusterCreateFlags(customProfile, rosaClient) | |||
| Expect(err).ToNot(HaveOccurred()) | |||
| rolePrefix = flags[16] | |||
There was a problem hiding this comment.
Is this arbitrary ? SHouldn't you use GetFlagValue ?
There was a problem hiding this comment.
The next test case uses same method, so I became just consistent, If you will tell I will change, wdyt?
There was a problem hiding this comment.
I would rather use some discovery than using hardcoded index which may change quite easily
There was a problem hiding this comment.
Okay will change it Thanks
| @@ -392,8 +392,6 @@ var _ = Describe("Cluster Upgrade testing", | |||
| By("Check upgrade state") | |||
| err = WaitForUpgradeToState(upgradeService, clusterID, con.Scheduled, 4) | |||
| Expect(err).To(BeNil()) | |||
| err = WaitForUpgradeToState(upgradeService, clusterID, con.Started, 70) | |||
There was a problem hiding this comment.
We don't wait for upgrade started anymore ?
There was a problem hiding this comment.
If we will wait, the upgrade started will never be cancelled/deleted for next test cases.
There was a problem hiding this comment.
In the test case I see Check the upgrade is finished successfully
So we should add a step to wait for the upgrade
There was a problem hiding this comment.
Okay will try to mould the test case same
openshift-merge-robot
added
the
needs-rebase
aaraj7
changed the title
|
New changes are detected. LGTM label has been removed. |
openshift-merge-robot
removed
the
needs-rebase
|
@aaraj7: 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-sigs/prow repository. I understand the commands that are listed here. |
No description provided.