Unit testing conventions

[perdasilva]

A recent discussion in #142 raised a couple of questions around what our unit testing conventions ought to be:

• should we still use ginkgo for unit tests or stick with gotest?
• should we default to tabular tests, singular tests, or are we happy with either?

For the sake of consistency, I'm putting this out there to our community so we can establish some best practices and conventions. IMHO:

• I think we should rollback on ginkgo for unit testing. Adds additional files (*_suite.go), it's not as nicely integrated with my IDE (or I haven't found a way to have as nice an experience as I do with gotests).
• I personally think there is a trade-off between tabular and singular tests, and I'm of the camp that thinks that for unit tests readability > conciseness (and I personally have a hard time reading tabular tests as there is a physical distance between the input and the logic). Having said that, I'm happy to default to what out community decides. Ultimately, consistency is a better goal in my mind.

[fgiloux]

my 2 cents:

• I feel that Ginkgo is often more in my way than being helpful
• I see value (also for readability) in both tabular and singular tests and I would just let people decide on a case by case basis what works best.
• I am with you that readability may be more important than other concerns for unit tests. In that regard consistency should serve the goal of making easy to grasp one unit test after the other. If forcing consistency makes some of the unit tests convoluted I would happily deviate from it.

[perdasilva]

Excellent points! Thank you ^^

[m1kola]

• I'm not a fan of having Ginkgo for unit tests. As Per mentioned - Ginkgo is not as nicely integrated with tools as standard Go tests. It is possible to get the same results, but requires extra efforts (for example to run an individual spec you need an extra extension for vscode).
• Overall I think Ginkgo and Gomega are overloaded and provide too many ways to do the same thing. Consider this example from the docs:

  It("adds a few books and checks the count", func(ctx SpecContext) {
      intialCount := client.FetchCount(ctx, "/items")
      client.AddItem(ctx, "foo")
      client.AddItem(ctx, "bar")
      //note that there are several supported ways to pass in the context.  All are equivalent:
      Eventually(ctx, client.FetchCount).WithArguments("/items").Should(BeNumerically("==", initialCount + 2))
      Eventually(client.FetchItems).WithContext(ctx).Should(ContainElement("foo"))
      Eventually(client.FetchItems, ctx).Should(ContainElement("foo"))
  }, SpecTimeout(time.Second * 5))

• But it has some great stuff for E2E tests. For example, when debugging I love sending SIGINFO to running tests and see the current state (makes debugging ofEventually very easy). So I would prefer to limit Ginkgo to E2E tests.
• For unit tests - I prefer table driven tests, but in my opinion it is ok to apply different approach from test to test.
• I agree with a poin on redability - jumping form test body to inputs struct is not ideal. The fact that many functions take input of kube objects and their definitions are usually lenthy makes things worse. But on the other hand - you read the body once and understand what is going on in each case. Also big test body might be a good indicator that either the function needs to be split into smaller functions or the test needs to be split into multiple tests/subtests.

[ncdc]

Also not a fan of Ginkgo.

Would not set a hard rule requiring or prohibiting table tests. Sometimes it makes sense; sometimes it doesn't.

Would recommend using testify for simple assertions to save some typing.

Would recommend against too many shared helper functions that set up CRs and whatnot - it's easy to forget/be unaware of what they do exactly...

[perdasilva]

ok - I'm going to close this discussion. I'm opening a ticket for us to document our unit testing guidelines. I believe what Andy has up here is a good place to start. I'll also create some ticket for refactoring the current unit tests from ginkgo to go test.

[camilamacedo86]

I agree that keeping one standard is better than having multiple styles (+1).

I’d like to just clarify a point that may be misunderstood:

I do not like because it Adds additional files (*_suite.go)

This is not true. A *_suite.go file is not required at all. It’s simply a convention to structure tests in a way that keeps them more manageable, allowing all tests to run under the same suite with shared setup and teardown logic. This approach can be helpful as it avoids reimplementing code for setup and teardown in each test file.

However, you can structure your suite within the same file as the tests, similar to standard Go testing, if you prefer. The *_suite.go file is just a tool to help keep files smaller and more maintainable—not a requirement.

Example

package mypackage_test

import (
    "testing"
    . "github.com/onsi/ginkgo/v2"
    . "github.com/onsi/gomega"
)

func TestMyCode(t *testing.T) {
    RegisterFailHandler(Fail)
    RunSpecs(t, "MyCode Suite")
}

var _ = Describe("MyCode", func() {
    It("should perform action A", func() {
        Expect(true).To(BeTrue())
    })

    It("should perform action B", func() {
        Expect(true).To(BeTrue())
    })
})

[camilamacedo86]

Summary of the Cons Raised Here and Discussed via Slack with My Comments

Cons of Using Ginkgo (Based on Discussions with My comments/thoughts)

1. Verbosity and Complexity:

   • Ginkgo introduces additional boilerplate (e.g., *_suite.go).

     IHMO: 👎 Not true: See discussion.

   • Structured syntax like Describe, Context, and It, which can feel verbose and unnatural for Go developers.

   IHMO: 👎 More about POV: Our QE and Kubernetes ecosystem contributors are familiar with this structure.

2. Debugging Challenges:

   • Debugging Ginkgo tests can be more challenging, with runtime errors (e.g., issues in BeforeEach/AfterEach) being harder to identify than compile-time errors.

     IHMO: 👎 I debug these in the same way as standard tests and haven’t faced issues.

   • Finding and running individual tests requires extra effort, such as plugins or specific CLI commands, which may still not provide an intuitive experience. For example, running a specific It block often requires plugins, and even then, support may be inconsistent or difficult to configure.

     IHMO: 👍 True, I agree 100% with this point.

3. Tooling Integration:

   • Ginkgo is not as well-integrated with IDEs as Go’s standard testing library.

     IHMO: 🆗 I don’t consider this a strong argument—other tools also require plugin. However, IDEs might not generate code as easily as without it.

4. Overloaded Features:

   • Ginkgo and Gomega provide multiple ways to perform the same task (e.g., passing contexts or arguments to Eventually), leading to cognitive overload and potential inconsistency in tests.

     IHMO: :MAYBE: But all languages and frameworks offer multiple ways to solve scenarios. This isn’t unique to Ginkgo. For example, the same occurs without Ginkgo. I'm not sure if it is a strong argument.

5. Subjective Complexity:

   • Some contributors feel Ginkgo is clumsy and not intuitive, particularly compared to Go’s standard library, which is minimalistic and direct.

     IHMO: :MAYBE: But it’s widely adopted in the Kubernetes ecosystem, and contributors familiar with Kubernetes or OCP are more likely to be comfortable with Ginkgo than other frameworks.

6. Runtime vs Compile-Time Issues:

   • Problems in Ginkgo are often discovered at runtime rather than compile time, making debugging slower and less efficient.

     **IHMO:**❓ I haven’t encountered this issue so far, maybe I missed something

7. Not Universally Adopted:

   • While common in Kubernetes-related projects, Ginkgo is not universally preferred, especially for projects that avoid additional dependencies.

   IHMO: 👎 True for projects outside our ecosystem / k8s*, but as part of the CNCF and Kubernetes ecosystems, it makes sense to stick with the conventions adopted there. inside of our context, it is universally adopted.

---

Pros of Using Ginkgo

1. Powerful Assertions:

   • Combined with Gomega, Ginkgo provides robust assertion methods like Eventually and Consistently, which are particularly valuable for asynchronous tests.

2. E2E Support:

   • Ginkgo excels in end-to-end testing, offering features like dynamic test configuration, useful debugging tools (e.g., SIGINFO), and flexible timeout handling.

3. Ecosystem Alignment:

   • Widely used in Kubernetes projects like controller-runtime, kubebuilder, and Kubernetes itself, making it familiar to contributors working in this ecosystem.
   • Upstream tests executed downstream are easier to manage when using consistent frameworks. This alignment simplifies the process and increases collaboration with QE.
   • Ginkgo is already used in CatalogD and other Operator Framework solutions. Aligning with these practices strengthens consistency across the ecosystem.

4. Readability and Flexibility (Subjective):

   • Some contributors find Ginkgo’s descriptive style easier to read and understand, particularly for complex test flows.
   • Offers advanced features for structuring tests, such as handling nested contexts, dynamic input data, and custom test hooks.

IHMO: I would stick with Gingko for E2E tests (at least) because

• As pointed out above, many of the arguments raised as cons (IHMO) are not ALL partially accurate and seem to result more from a lack of familiarity with Ginkgo rather than inherent issues with the framework.
• Ginkgo makes tests easier to read and organize (while acknowledging that not everyone may share this opinion).
• 👉 Sticking with what is expected in our context (Kubernetes and OCP ecosystems) ensures that contributors are already familiar with the framework, streamlines upstream and downstream efforts, and fosters consistency.
• 👉 It may also be easier for us to receive help from QE and likely streamline the process of having the upstream tests run seamlessly in downstream environments.

These are why I would prefer to keep everything aligned with the Kubernetes/OCP ecosystem. I’ve tried to address all the arguments raised against using Ginkgo and outlined the cons where applicable with my comments.

[bentito]

We have very little ginkgo in our current v1 items. operator-controller looks like it has it due to registry using it. catalogd has a bit more and I went and looked at what we're doing with it there. I have to say those tests are fine, but they're opaque to my IDE that can do such a nice job running and debugging gotest tests. Want to run just one test and hit a breakpoint to help reason about a complex failure? Works with gotest tests, but you get no help with the gingko ones.

IMO gingko kind of slipped into our project and we should really standardize on not using it.

[camilamacedo86]

You can debug with Ginkgo.
All Kubebuilder tests are with Ginkgo, and I debug all of them without any issues.

My bet is the problem with tests that use ENVTEST.
Could you share what tests you cannot debug so I can try to help?