Skip to content
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

Document testing conventions #188

Open
5 tasks
Tracked by #190 ...
perdasilva opened this issue May 9, 2023 Discussed in #168 · 1 comment
Open
5 tasks
Tracked by #190 ...

Document testing conventions #188

perdasilva opened this issue May 9, 2023 Discussed in #168 · 1 comment
Labels
documentation Improvements or additions to documentation good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Comments

@perdasilva
Copy link
Contributor

perdasilva commented May 9, 2023

Discussed in #168

Originally posted by perdasilva April 14, 2023
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.

Acceptance Criteria:

  • document describing development conventions in /docs created
  • development conventions doc covers our testing conventions:
    • No ginkgo for unit tests - use go test
    • Use tabular tests when it makes sense - call out trade-offs
    • Minimize the use of shared helper functions as they can often go forgotten
  • call out specific assertion framework (e.g. testify)
  • add examples for different types of well-organized tests: unit, e2e, integration
  • addressed comments in this issue [e.g.]

For more info, see discussion #168

@perdasilva perdasilva added documentation Improvements or additions to documentation help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 9, 2023
@joelanford
Copy link
Member

Can we add a callout for using a particular assertion framework as well (e.g. testify)?

I agree with most of the points in the discussion about Ginkgo. But the one thing I've found that I like about it is that it generally encourages tests to be written in a way that adding the next test is easy because you can setup layers of contexts. With standard go tests, there are sometimes the questions of:

  • "add this to an existing Test* function or create a new Test* function?"
  • and the corollary, "how big/how many assertions should go in a single Test* function?"

Perhaps having the docs point out examples of well-organized tests would help?

Lastly, I'd very much like to see something that talks about how we choose between unit/integration/e2e. Not sure if that's this doc or another one though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Status: Accepted
Development

No branches or pull requests

2 participants