🌱 add more e2e tests #121
🌱 add more e2e tests #121
Conversation
| var _ = Describe("Create providers with minimal specified configuration", func() { | ||
| It("should succefully create a CoreProvider", func() { | ||
| var _ = Describe("Create, upgrade, downgrade and delete providers with minimal specified configuration", func() { | ||
| It("should successfully create a CoreProvider", func() { |
There was a problem hiding this comment.
Do we want to add a delete test for CoreProvider as well?
There was a problem hiding this comment.
Yes, I wrote a test for that (line 396). The thing is that in order to install other providers, we need Core Provider to be Ready. It means we can't delete it immediately and have to postpone this action.
There was a problem hiding this comment.
Ah right, was thinking the same initially, thanks for clarification
There was a problem hiding this comment.
@Fedosin feel free to resolve the conversation
| Expect(k8sclient.Delete(ctx, coreProvider)).To(Succeed()) | ||
|
|
||
| By("Waiting for the core provider deployment to be deleted") | ||
| Eventually(func() bool { | ||
| deployment := &appsv1.Deployment{} | ||
| key := client.ObjectKey{Namespace: operatorNamespace, Name: coreProviderDeploymentName} | ||
| isReady, err := waitForObjectToBeDeleted(k8sclient, ctx, key, deployment) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return isReady | ||
| }, timeout).Should(Equal(true)) |
There was a problem hiding this comment.
perhaps it is better if we will move this code to the end of core provider tests at the beginning of the file (see this commment) and drop the dedicated "It" test case to shorten the code.
There was a problem hiding this comment.
Unfortunately we can't delete it earlier, because other providers will fail a preflight check that tests that Core Provider is Ready: https://github.com/kubernetes-sigs/cluster-api-operator/blob/main/internal/controller/preflight_checks.go#L130-L131
This PR adds several e2e tests to verify deletion, upgrading and downgrading providers.
|
@furkatgofurov7 thanks for your review! I updated the tests as you suggested. |
@Fedosin thanks, looks good to me. The only thing might worth mentioning is, we could refactor the code around |
|
@furkatgofurov7 ack, agree with you! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demicev 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 |
What this PR does / why we need it:
This PR adds several e2e tests to verify deletion, upgrading and downgrading providers.