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.

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

🐛 (go/v4): Refactor e2e-tests for readable logs #4159

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ var _ = Describe("Manager", Ordered, func() {

podOutput, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Failed to retrieve controller-manager pod information")
podNames := utils.GetNonEmptyLines(string(podOutput))
podNames := utils.GetNonEmptyLines(podOutput)
g.Expect(podNames).To(HaveLen(1), "expected 1 controller pod running")
controllerPodName = podNames[0]
g.Expect(controllerPodName).To(ContainSubstring("controller-manager"))
Expand All @@ -110,7 +110,7 @@ var _ = Describe("Manager", Ordered, func() {
"pods", controllerPodName, "-o", "jsonpath={.status.phase}",
"-n", namespace,
)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Running"), "Incorrect controller-manager pod status")
g.Expect(utils.Run(cmd)).To(Equal("Running"), "Incorrect controller-manager pod status")
}
// Repeatedly check if the controller-manager pod is running until it succeeds or times out.
Eventually(verifyControllerUp).Should(Succeed())
Expand Down Expand Up @@ -166,7 +166,7 @@ var _ = Describe("Manager", Ordered, func() {
cmd := exec.Command("kubectl", "get", "pods", "curl-metrics",
"-o", "jsonpath={.status.phase}",
"-n", namespace)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Succeeded"), "curl pod in wrong status")
g.Expect(utils.Run(cmd)).To(Equal("Succeeded"), "curl pod in wrong status")
}
Eventually(verifyCurlUp, 5*time.Minute).Should(Succeed())

Expand Down Expand Up @@ -278,9 +278,9 @@ func getMetricsOutput() string {
cmd := exec.Command("kubectl", "logs", "curl-metrics", "-n", namespace)
metricsOutput, err := utils.Run(cmd)
Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod")
metricsOutputStr := string(metricsOutput)
Expect(metricsOutputStr).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutputStr

Expect(metricsOutput).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutput
}

// tokenRequest is a simplified representation of the Kubernetes TokenRequest API response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func warnError(err error) {
}

// Run executes the provided command within this context
func Run(cmd *exec.Cmd) ([]byte, error) {
func Run(cmd *exec.Cmd) (string, error) {
dir, _ := GetProjectDir()
cmd.Dir = dir

Expand All @@ -54,10 +54,10 @@ func Run(cmd *exec.Cmd) ([]byte, error) {
_, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command)
output, err := cmd.CombinedOutput()
if err != nil {
return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
return "", fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
Copy link
Member

Choose a reason for hiding this comment

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

Then, we are no longer return the output when we we should here.
We need return the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output is in the error message, though. Generally the return values are zero-valued, or the error is nil. This is what Gomega checks, and so I replaced it with "" to adhere to the go idioms.

}

return output, nil
return string(output), nil
}

// InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics.
Expand Down Expand Up @@ -92,7 +92,7 @@ func IsPrometheusCRDsInstalled() bool {
if err != nil {
return false
}
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range prometheusCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down Expand Up @@ -153,7 +153,7 @@ func IsCertManagerCRDsInstalled() bool {
}

// Check if any of the Cert Manager CRDs are present
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range certManagerCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ var _ = Describe("Manager", Ordered, func() {

podOutput, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Failed to retrieve controller-manager pod information")
podNames := utils.GetNonEmptyLines(string(podOutput))
podNames := utils.GetNonEmptyLines(podOutput)
g.Expect(podNames).To(HaveLen(1), "expected 1 controller pod running")
controllerPodName = podNames[0]
g.Expect(controllerPodName).To(ContainSubstring("controller-manager"))
Expand All @@ -110,7 +110,7 @@ var _ = Describe("Manager", Ordered, func() {
"pods", controllerPodName, "-o", "jsonpath={.status.phase}",
"-n", namespace,
)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Running"), "Incorrect controller-manager pod status")
g.Expect(utils.Run(cmd)).To(Equal("Running"), "Incorrect controller-manager pod status")
}
// Repeatedly check if the controller-manager pod is running until it succeeds or times out.
Eventually(verifyControllerUp).Should(Succeed())
Expand Down Expand Up @@ -166,7 +166,7 @@ var _ = Describe("Manager", Ordered, func() {
cmd := exec.Command("kubectl", "get", "pods", "curl-metrics",
"-o", "jsonpath={.status.phase}",
"-n", namespace)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Succeeded"), "curl pod in wrong status")
g.Expect(utils.Run(cmd)).To(Equal("Succeeded"), "curl pod in wrong status")
}
Eventually(verifyCurlUp, 5*time.Minute).Should(Succeed())

Expand Down Expand Up @@ -240,9 +240,9 @@ func getMetricsOutput() string {
cmd := exec.Command("kubectl", "logs", "curl-metrics", "-n", namespace)
metricsOutput, err := utils.Run(cmd)
Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod")
metricsOutputStr := string(metricsOutput)
Expect(metricsOutputStr).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutputStr

Expect(metricsOutput).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutput
}

// tokenRequest is a simplified representation of the Kubernetes TokenRequest API response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func warnError(err error) {
}

// Run executes the provided command within this context
func Run(cmd *exec.Cmd) ([]byte, error) {
func Run(cmd *exec.Cmd) (string, error) {
dir, _ := GetProjectDir()
cmd.Dir = dir

Expand All @@ -54,10 +54,10 @@ func Run(cmd *exec.Cmd) ([]byte, error) {
_, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command)
output, err := cmd.CombinedOutput()
if err != nil {
return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
return "", fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
Copy link
Member

Choose a reason for hiding this comment

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

we should return the output always.

}

return output, nil
return string(output), nil
}

// InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics.
Expand Down Expand Up @@ -92,7 +92,7 @@ func IsPrometheusCRDsInstalled() bool {
if err != nil {
return false
}
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range prometheusCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down Expand Up @@ -153,7 +153,7 @@ func IsCertManagerCRDsInstalled() bool {
}

// Check if any of the Cert Manager CRDs are present
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range certManagerCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ var _ = Describe("Manager", Ordered, func() {

podOutput, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Failed to retrieve controller-manager pod information")
podNames := utils.GetNonEmptyLines(string(podOutput))
podNames := utils.GetNonEmptyLines(podOutput)
g.Expect(podNames).To(HaveLen(1), "expected 1 controller pod running")
controllerPodName = podNames[0]
g.Expect(controllerPodName).To(ContainSubstring("controller-manager"))
Expand All @@ -110,7 +110,7 @@ var _ = Describe("Manager", Ordered, func() {
"pods", controllerPodName, "-o", "jsonpath={.status.phase}",
"-n", namespace,
)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Running"), "Incorrect controller-manager pod status")
g.Expect(utils.Run(cmd)).To(Equal("Running"), "Incorrect controller-manager pod status")
Copy link
Member

Choose a reason for hiding this comment

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

Just for curiosity,
what is the diff between BeEquivalentTo and Equal do you know?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Equal checks for equality while BeEquivalentTo does some type conversion, like between string and []byte. Unfortunately, it still prints out the byte array representation. Equals doesn't allow a byte array to be compared to a string.

}
// Repeatedly check if the controller-manager pod is running until it succeeds or times out.
Eventually(verifyControllerUp).Should(Succeed())
Expand Down Expand Up @@ -166,7 +166,7 @@ var _ = Describe("Manager", Ordered, func() {
cmd := exec.Command("kubectl", "get", "pods", "curl-metrics",
"-o", "jsonpath={.status.phase}",
"-n", namespace)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Succeeded"), "curl pod in wrong status")
g.Expect(utils.Run(cmd)).To(Equal("Succeeded"), "curl pod in wrong status")
}
Eventually(verifyCurlUp, 5*time.Minute).Should(Succeed())

Expand Down Expand Up @@ -278,9 +278,9 @@ func getMetricsOutput() string {
cmd := exec.Command("kubectl", "logs", "curl-metrics", "-n", namespace)
metricsOutput, err := utils.Run(cmd)
Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod")
metricsOutputStr := string(metricsOutput)
Expect(metricsOutputStr).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutputStr

Expect(metricsOutput).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutput
}

// tokenRequest is a simplified representation of the Kubernetes TokenRequest API response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func warnError(err error) {
}

// Run executes the provided command within this context
func Run(cmd *exec.Cmd) ([]byte, error) {
func Run(cmd *exec.Cmd) (string, error) {
dir, _ := GetProjectDir()
cmd.Dir = dir

Expand All @@ -54,10 +54,10 @@ func Run(cmd *exec.Cmd) ([]byte, error) {
_, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command)
output, err := cmd.CombinedOutput()
if err != nil {
return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
return "", fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
}

return output, nil
return string(output), nil
}

// InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics.
Expand Down Expand Up @@ -92,7 +92,7 @@ func IsPrometheusCRDsInstalled() bool {
if err != nil {
return false
}
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range prometheusCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down Expand Up @@ -153,7 +153,7 @@ func IsCertManagerCRDsInstalled() bool {
}

// Check if any of the Cert Manager CRDs are present
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range certManagerCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ var _ = Describe("Manager", Ordered, func() {

podOutput, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Failed to retrieve controller-manager pod information")
podNames := utils.GetNonEmptyLines(string(podOutput))
podNames := utils.GetNonEmptyLines(podOutput)
g.Expect(podNames).To(HaveLen(1), "expected 1 controller pod running")
controllerPodName = podNames[0]
g.Expect(controllerPodName).To(ContainSubstring("controller-manager"))
Expand All @@ -243,7 +243,7 @@ var _ = Describe("Manager", Ordered, func() {
"pods", controllerPodName, "-o", "jsonpath={.status.phase}",
"-n", namespace,
)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Running"), "Incorrect controller-manager pod status")
g.Expect(utils.Run(cmd)).To(Equal("Running"), "Incorrect controller-manager pod status")
}
// Repeatedly check if the controller-manager pod is running until it succeeds or times out.
Eventually(verifyControllerUp).Should(Succeed())
Expand Down Expand Up @@ -299,7 +299,7 @@ var _ = Describe("Manager", Ordered, func() {
cmd := exec.Command("kubectl", "get", "pods", "curl-metrics",
"-o", "jsonpath={.status.phase}",
"-n", namespace)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Succeeded"), "curl pod in wrong status")
g.Expect(utils.Run(cmd)).To(Equal("Succeeded"), "curl pod in wrong status")
}
Eventually(verifyCurlUp, 5 * time.Minute).Should(Succeed())

Expand Down Expand Up @@ -373,9 +373,9 @@ func getMetricsOutput() string {
cmd := exec.Command("kubectl", "logs", "curl-metrics", "-n", namespace)
metricsOutput, err := utils.Run(cmd)
Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod")
metricsOutputStr := string(metricsOutput)
Expect(metricsOutputStr).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutputStr

Expect(metricsOutput).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutput
}

// tokenRequest is a simplified representation of the Kubernetes TokenRequest API response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func warnError(err error) {
}

// Run executes the provided command within this context
func Run(cmd *exec.Cmd) ([]byte, error) {
func Run(cmd *exec.Cmd) (string, error) {
dir, _ := GetProjectDir()
cmd.Dir = dir

Expand All @@ -79,10 +79,10 @@ func Run(cmd *exec.Cmd) ([]byte, error) {
_, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command)
output, err := cmd.CombinedOutput()
if err != nil {
return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
return "", fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
}

return output, nil
return string(output), nil
}

// InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics.
Expand Down Expand Up @@ -117,7 +117,7 @@ func IsPrometheusCRDsInstalled() bool {
if err != nil {
return false
}
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range prometheusCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down Expand Up @@ -178,7 +178,7 @@ func IsCertManagerCRDsInstalled() bool {
}

// Check if any of the Cert Manager CRDs are present
crdList := GetNonEmptyLines(string(output))
crdList := GetNonEmptyLines(output)
for _, crd := range certManagerCRDs {
for _, line := range crdList {
if strings.Contains(line, crd) {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/utils/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Kubectl struct {
func (k *Kubectl) Command(cmdOptions ...string) (string, error) {
cmd := exec.Command("kubectl", cmdOptions...)
output, err := k.Run(cmd)
return string(output), err
return output, err
}

// WithInput is a general func to run kubectl commands with input
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/utils/test_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,18 +301,18 @@ type CmdContext struct {
}

// Run executes the provided command within this context
func (cc *CmdContext) Run(cmd *exec.Cmd) ([]byte, error) {
func (cc *CmdContext) Run(cmd *exec.Cmd) (string, error) {
cmd.Dir = cc.Dir
cmd.Env = append(os.Environ(), cc.Env...)
cmd.Stdin = cc.Stdin
command := strings.Join(cmd.Args, " ")
_, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command)
output, err := cmd.CombinedOutput()
if err != nil {
return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
return "", fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output))
}

return output, nil
return string(output), nil
}

// AllowProjectBeMultiGroup will update the PROJECT file with the information to allow we scaffold
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/v4/plugin_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var _ = Describe("kubebuilder", func() {
func Run(kbc *utils.TestContext, hasWebhook, isToUseInstaller, hasMetrics bool, hasNetworkPolicies bool) {
var controllerPodName string
var err error
var output []byte
var output string

By("creating manager namespace")
err = kbc.CreateManagerNamespace()
Expand Down
12 changes: 6 additions & 6 deletions testdata/project-v4-multigroup-with-plugins/test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ var _ = Describe("Manager", Ordered, func() {

podOutput, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Failed to retrieve controller-manager pod information")
podNames := utils.GetNonEmptyLines(string(podOutput))
podNames := utils.GetNonEmptyLines(podOutput)
g.Expect(podNames).To(HaveLen(1), "expected 1 controller pod running")
controllerPodName = podNames[0]
g.Expect(controllerPodName).To(ContainSubstring("controller-manager"))
Expand All @@ -110,7 +110,7 @@ var _ = Describe("Manager", Ordered, func() {
"pods", controllerPodName, "-o", "jsonpath={.status.phase}",
"-n", namespace,
)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Running"), "Incorrect controller-manager pod status")
g.Expect(utils.Run(cmd)).To(Equal("Running"), "Incorrect controller-manager pod status")
}
// Repeatedly check if the controller-manager pod is running until it succeeds or times out.
Eventually(verifyControllerUp).Should(Succeed())
Expand Down Expand Up @@ -166,7 +166,7 @@ var _ = Describe("Manager", Ordered, func() {
cmd := exec.Command("kubectl", "get", "pods", "curl-metrics",
"-o", "jsonpath={.status.phase}",
"-n", namespace)
g.Expect(utils.Run(cmd)).To(BeEquivalentTo("Succeeded"), "curl pod in wrong status")
g.Expect(utils.Run(cmd)).To(Equal("Succeeded"), "curl pod in wrong status")
}
Eventually(verifyCurlUp, 5*time.Minute).Should(Succeed())

Expand Down Expand Up @@ -278,9 +278,9 @@ func getMetricsOutput() string {
cmd := exec.Command("kubectl", "logs", "curl-metrics", "-n", namespace)
metricsOutput, err := utils.Run(cmd)
Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod")
metricsOutputStr := string(metricsOutput)
Expect(metricsOutputStr).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutputStr

Expect(metricsOutput).To(ContainSubstring("< HTTP/1.1 200 OK"))
return metricsOutput
}

// tokenRequest is a simplified representation of the Kubernetes TokenRequest API response,
Expand Down
Loading