Skip to content
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.

Sign up for GitHub

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

🌱improve Ginkgo/Gomega test style #4426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −22
Open

🌱improve Ginkgo/Gomega test style #4426

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 19 additions & 22 deletions test/e2e/v4/plugin_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,54 +254,53 @@ func Run(kbc *utils.TestContext, hasWebhook, isToUseInstaller, isToUseHelmChart,

if hasWebhook {
By("validating that cert-manager has provisioned the certificate Secret")
EventuallyWithOffset(1, func() error {
_, err := kbc.Kubectl.Get(
EventuallyWithOffset(1, func(g Gomega) {
output, err := kbc.Kubectl.Get(
true,
"secrets", "webhook-server-cert")
return err
g.Expect(err).ToNot(HaveOccurred(), "webhook-server-cert should exist in the namespace")
g.Expect(output).To(ContainSubstring("webhook-server-cert"))
}, time.Minute, time.Second).Should(Succeed())

By("validating that the mutating|validating webhooks have the CA injected")
verifyCAInjection := func() error {
verifyCAInjection := func(g Gomega) {
mwhOutput, err := kbc.Kubectl.Get(
false,
"mutatingwebhookconfigurations.admissionregistration.k8s.io",
fmt.Sprintf("e2e-%s-mutating-webhook-configuration", kbc.TestSuffix),
"-o", "go-template={{ range .webhooks }}{{ .clientConfig.caBundle }}{{ end }}")
ExpectWithOffset(2, err).NotTo(HaveOccurred())
g.ExpectWithOffset(2, err).NotTo(HaveOccurred())
// check that ca should be long enough, because there may be a place holder "\n"
ExpectWithOffset(2, len(mwhOutput)).To(BeNumerically(">", 10))
g.ExpectWithOffset(2, len(mwhOutput)).To(BeNumerically(">", 10))
Copy link
Member

@camilamacedo86 camilamacedo86 Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to follow the same approach as the test scaffolds: https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v4/test (as closer as possible is the ideal, same standards of checks)
If you look there, you will see that it is quite similar to what we do here. You can indeed compare the same checks for metrics etc.

For example, we should no longer use ExpectWithOffset; instead, we should use the functions and simplify it.

Look at #4135

And check the PRs done by @mogsie related to the tests: https://github.com/kubernetes-sigs/kubebuilder/pulls?q=is%3Apr+author%3Amogsie+is%3Aclosed

I think that will help out understand better what we are trying to do,
BTW, thank you a lot for checking this one


vwhOutput, err := kbc.Kubectl.Get(
false,
"validatingwebhookconfigurations.admissionregistration.k8s.io",
fmt.Sprintf("e2e-%s-validating-webhook-configuration", kbc.TestSuffix),
"-o", "go-template={{ range .webhooks }}{{ .clientConfig.caBundle }}{{ end }}")
ExpectWithOffset(2, err).NotTo(HaveOccurred())
g.ExpectWithOffset(2, err).NotTo(HaveOccurred())
// check that ca should be long enough, because there may be a place holder "\n"
ExpectWithOffset(2, len(vwhOutput)).To(BeNumerically(">", 10))

return nil
g.ExpectWithOffset(2, len(vwhOutput)).To(BeNumerically(">", 10))
}

EventuallyWithOffset(1, verifyCAInjection, time.Minute, time.Second).Should(Succeed())

By("validating that the CA injection is applied for CRD conversion")
crdKind := "ConversionTest"
verifyCAInjection = func() error {
verifyCAInjection = func(g Gomega) {
crdOutput, err := kbc.Kubectl.Get(
false,
"customresourcedefinition.apiextensions.k8s.io",
"-o", fmt.Sprintf(
"jsonpath={.items[?(@.spec.names.kind=='%s')].spec.conversion.webhook.clientConfig.caBundle}",
crdKind),
)
ExpectWithOffset(1, err).NotTo(HaveOccurred(),
g.ExpectWithOffset(1, err).NotTo(HaveOccurred(),
"failed to get CRD conversion webhook configuration")

// Check if the CA bundle is populated (length > 10 to avoid placeholder values)
ExpectWithOffset(1, len(crdOutput)).To(BeNumerically(">", 10),
g.ExpectWithOffset(1, len(crdOutput)).To(BeNumerically(">", 10),
"CA bundle should be injected into the CRD")
return nil
}
EventuallyWithOffset(1, verifyCAInjection, time.Minute, time.Second).Should(Succeed(),
"CA injection validation failed")
Expand All @@ -324,9 +323,8 @@ func Run(kbc *utils.TestContext, hasWebhook, isToUseInstaller, isToUseHelmChart,
_, err = f.WriteString(" foo: bar")
Expect(err).To(Not(HaveOccurred()))

EventuallyWithOffset(1, func() error {
_, err = kbc.Kubectl.Apply(true, "-f", sampleFile)
return err
EventuallyWithOffset(1, func(g Gomega) {
g.Expect(kbc.Kubectl.Apply(true, "-f", sampleFile)).Error().ToNot(HaveOccurred())
}, time.Minute, time.Second).Should(Succeed())

if hasMetrics {
Expand Down Expand Up @@ -359,14 +357,13 @@ func Run(kbc *utils.TestContext, hasWebhook, isToUseInstaller, isToUseHelmChart,
By("creating a namespace")
namespace := "test-webhooks"
_, err := kbc.Kubectl.Command("create", "namespace", namespace)
Expect(err).NotTo(HaveOccurred(), "namespace should be created successfully")
Expect(err).To(Not(HaveOccurred()), "namespace should be created successfully")

By("applying the CR in the created namespace")
EventuallyWithOffset(1, func() error {
EventuallyWithOffset(1, func(g Gomega) {
_, err := kbc.Kubectl.Apply(false, "-n", namespace, "-f", sampleFile)
return err
}, 2*time.Minute, time.Second).ShouldNot(HaveOccurred(),
"apply in test-webhooks ns should not fail")
g.Expect(err).NotTo(HaveOccurred())
}, 2*time.Minute, time.Second).Should(Succeed())

By("validating that mutating webhooks are working fine outside of the manager's namespace")
cnt, err := kbc.Kubectl.Get(
Expand Down
Loading