-
Notifications
You must be signed in to change notification settings - Fork 551
test/e2e: wrap subscription e2e test logic in Eventually statement #2392
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/e2e: wrap subscription e2e test logic in Eventually statement #2392
Conversation
@@ -467,9 +467,14 @@ var _ = Describe("Subscription", func() { | |||
require.NoError(GinkgoT(), err) | |||
|
|||
// Wait for the subscription to begin upgrading to csvB | |||
subscription, err = fetchSubscription(crc, testNamespace, subscriptionName, subscriptionStateUpgradePendingChecker) |
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.
I wonder if this should be handled at the fetchSubscription layer, and instead, create a new checker type.
/lgtm
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.
I think we probably want to start getting rid of a lot of the custom test framework if we can write test specs simply at the call sites.
it seems like maintaining custom test frameworks is one place where the cost of being DRY outweighs its value
2732fd6
to
ed1508e
Compare
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.
Looks good, one thought:
Signed-off-by: Daniel Sover <[email protected]>
ed1508e
to
3bde04e
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: exdx, njhale 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 |
/lgtm |
Signed-off-by: Daniel Sover [email protected]
Description of the change:
Fixes up the "with starting CSV" test by wrapping a flake-prone path with an Eventually()
Motivation for the change:
Flaky test
Reviewer Checklist
/doc