-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 add basic integration tests for ClusterClass webhook #5803
🌱 add basic integration tests for ClusterClass webhook #5803
Conversation
|
Question: In order to add the index check to the ClusterClass webhooks client.Get (clusters) #5719 We'd need to port many complex table tests to use envtest in this package. Is that the direction I should go? |
I'm not sure I get the full context. But I would assume it's good enough to cover the case where we also have another cluster which is not using the same ClusterClass. I think everything else doesn't have to duplicated. |
@sbueringer I think there's three cases that currently require testenv and I'll probably add in a couple more. The plan is to add the indexer and the testenv dependent test cases to this directory (In another PR) - does that sound right to you? |
Sounds good to me. |
98f3f20
to
08a5051
Compare
/check-cla |
08a5051
to
f004801
Compare
Thx! lgtm pending squash |
f57e591
to
786170a
Compare
/lgtm |
Build() | ||
|
||
// Create the ClusterClass in the API server. | ||
g.Expect(env.Create(ctx, clusterClass)).To(Succeed()) |
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.
Should this be
g.Expect(env.Create(ctx, clusterClass)).To(Succeed()) | |
g.Expect(env.CreateAndWait(ctx, clusterClass)).To(Succeed()) |
so we reduce the chance of flakiness (same below when we have objects depending on each others)
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.
Good idea - I've also changed the Deletes to Cleanups / CleanupAndWait where appropriate.
786170a
to
3c0598f
Compare
Signed-off-by: killianmuldoon <[email protected]>
3c0598f
to
0ed6f1c
Compare
yay! more tests! |
Thx! |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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 |
Signed-off-by: killianmuldoon [email protected]
This PR adds basic integration tests for the ClusterClass webhook assessing whether Create, Delete and Update validations work as expected with a real API server.