Unit testing framework discussion: standard Go style unit testing via `testing` and `go-checker` framework

[christarazi]

(Copied from https://cilium.slack.com/archives/C2B917YHE/p1626113446459700)

In the Cilium codebase, we heavily use https://github.com/go-check/check/tree/v1 as the unit testing framework. I've been increasingly feeling like we don't quite need this framework anymore because the std Go testing package + other small convenience packages for asserting is sufficient.

FWIW, pkg/hubble is written using the std Go style instead of gocheck.

How do people feel about this?

In terms of adoption, one approach would be to add new tests using the std Go style and over time we gradually move away from gocheck. Not advocating for a rewrite immediately.

Used for tracking pros and cons, and capturing other discussions because Slack will lose history

[christarazi]

(Feel free to edit to update)

Std Go style:

Pros:

• More active development
• Subtest support
• Simpler and more familiar with the average Go developer

Cons:

• Lacks consistency in terms of patterns (up to developer to write the comparisons)
  • Mitigation is to use go-cmp or testify
• Lacks setup / teardown funcs
  • Mitigation is to use TestMain for SetUpSuite equivalent (https://pkg.go.dev/testing#hdr-Main)

gochecker:

Pros:

• All-in-one experience (setup / teardown, asserts, comparisons helpers)

Cons:

• Lagging behind in upstream features like subtest support
• Requires learning yet another framework
• More boilerplate code for simple tests that don't require a whole setup/teardown

[gandro]

FWIW, pkg/hubble is written using the std Go style instead of gocheck.

To expand on that, this is due to most code of this package originally being developed out of tree. We noticed the discrepancy at the time, but decided it was much work and not much benefit in moving all code to gochecker, so we stuck to the Std Go style for that package.

We did run into issues with testify (https://github.com/cilium/cilium/issues/12772), so parts of it now use go-cmp instead. I'm not sure if the underlying issue was ever fixed though, it doesn't look like it is.

[ldelossa]

@christarazi I believe we came to an agreement on this ticket. We came to the decision that new code will use the std-library testing package with go-cmp in addition when comparison is necessary.

We won't be "re-writing" any tests which exist today to conform to this, and this decision is for subsequent new code.

Should this ticket remain open?

[christarazi]

I think it makes sense to keep open for visibility.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[xmulligan]

From @lmb in slack

The PR is here, and contains some context why this change is necessary / useful. this is a tree-wide change and so has the possibility to impact a bunch of people, so i want to raise awareness here. i'm planning to get this merged next wednesday-ish, 2023-05-24. the following things will change: gocheck specific flags like -check.f and so on are going away. just use the normal go test -run. the name of specific tests will change, for example MySuite.TestSomeBehaviour will become MySuite/TestSomeBehaviour. the output format will be closer to the standard go test output. some APIs of gocheck have been deprecated or removed (as long as there was no use in the cilium codebase). details are here. please let me know if you think that this change might impact you negatively!