🐛 (go/v4): Refactor e2e-tests for clearer error handling and readable logs

[camilamacedo86]

Refactors all test cases that use g.Expect(utils.Run(cmd)) to improve both the readability of logs and the clarity of the code.

Changes:

• Replaced occurrences of g.Expect(utils.Run(cmd)) with:

output, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(string(output)).To(<condition>)

OR

_, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred())

Motivation

• Human-readable logs: Output is now converted to a string, providing more understandable logs by displaying actual kubectl command results instead of raw byte arrays.
• Improved code clarity: The previous g.Expect(utils.Run(cmd)) usage did not make it clear that the function returns both output and error. By explicitly handling the output and err variables, the code is more transparent and easier to maintain.

Example of problematic scenario

Otherwise, we are unable to check the output when errors are faced. For example

Before the changes:
Example: https://github.com/kubernetes-sigs/kubebuilder/actions/runs/10807585375/job/29978696165

  [FAILED] Timed out after 120.001s.
  The function passed to Eventually failed at /home/runner/work/kubebuilder/kubebuilder/testdata/project-v4-multigroup/test/e2e/e2e_test.go:145 with:
  Metrics endpoint is not ready
  Expected
      <[]uint8 | len:151, cap:1024>: [78, 65, 77, 69, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 69, 78, 68, 80, 79, 73, 78, 84, 83, 32, 32, 32, 65, 71, 69, 10, 112, 114, 111, 106, 101, 99, 116, 45, 118, 52, 45, 109, 117, 108, 116, 105, 103, 114, 111, 117, 112, 45, 99, 111, 110, 116, 114, 111, 108, 108, 101, 114, 45, 109, 97, 110, 97, 103, 101, 114, 45, 109, 101, 116, 114, 105, 99, 115, 45, 115, 101, 114, 118, 105, 99, 101, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 50, 109, 51, 115, 10]
  to contain substring
      <string>: 8443

And then, after the changes:
Example: https://github.com/kubernetes-sigs/kubebuilder/actions/runs/10808334463/job/29981083176?pr=4154

  [FAILED] Timed out after 120.001s.
  The function passed to Eventually failed at /home/runner/work/kubebuilder/kubebuilder/testdata/project-v4-multigroup/test/e2e/e2e_test.go:145 with:
  Metrics endpoint is not ready
  Expected
      <string>: NAME                                                       ENDPOINTS   AGE
      project-v4-multigroup-controller-manager-metrics-service               2m3s

  to contain substring
      <string>: 8443
  In [It] at: /home/runner/work/kubebuilder/kubebuilder/testdata/project-v4-multigroup/test/e2e/e2e_test.go:147 @ 09/11/

[camilamacedo86]

c/c @mogsie

[camilamacedo86]

/test pull-kubebuilder-e2e-k8s-1-31-0

[mogsie]

I added a commit 05da7bc that has a different approach to fixing this. It replaces the []byte with a string in the utils.Run() signature.

[camilamacedo86]

I added a commit 05da7bc that has a different approach to fixing this. It replaces the []byte with a string in the utils.Run() signature.

Hi @mogsie

I do not think that we should have g.Expect(utils.Run(cmd)) at all.
That does not make clear when we are reading the code what is the return.
So, I think we need to move with this one and then we can discuss the changed of the PR.

[mogsie]

I understand. It may be more a matter of style; maybe we should discuss that (style) over in #4135 before I continue making suggestions to changes to these assertions 😊

[camilamacedo86]

Hi @mogsie

I understand. It may be more a matter of style; maybe we should discuss that (style) over in #4135 before I continue making suggestions to changes to these assertions 😊I understand. It may be more a matter of style; maybe we should discuss that (style) over in #4135 before I continue making suggestions to changes to these assertions 😊

All contributions that you are doing a terrific great !!!
IHMO we just need this need as we spoke about. So that we make clear the return and either we solve the problem faced where the bytes are outputted. Please, feel free to continue doing your amazing work !!!

Are you ok with we get this one merged?

[camilamacedo86]

Hi @mogsie

Thank you for your review !
All done .

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mogsie

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:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment