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())
    })
})