-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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) { | ||
|
@@ -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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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")) | ||
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for curiosity, There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()) | ||
|
@@ -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()) | ||
|
||
|
@@ -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, | ||
|
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.
Then, we are no longer return the output when we we should here.
We need return the output.
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.
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.