From 739f50c3b571d3566f9f21769357010f0989b19c Mon Sep 17 00:00:00 2001 From: Victor Zagorodny Date: Thu, 18 Jul 2019 01:46:46 +0000 Subject: Go guide: be more explicit on testing frameworks + diffing test results --- doc/development/go_guide/index.md | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) (limited to 'doc/development/go_guide') diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index f09339eb3a4..f827d240bf6 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -129,17 +129,50 @@ deploy a new pod, migrating the data automatically. ## Testing +### Testing frameworks + We should not use any specific library or framework for testing, as the [standard library](https://golang.org/pkg/) provides already everything to get -started. For example, some external dependencies might be worth considering in -case we decide to use a specific library or framework: +started. If there is a need for more sophisticated testing tools, the following +external dependencies might be worth considering in case we decide to use a specific +library or framework: - [Testify](https://github.com/stretchr/testify) - [httpexpect](https://github.com/gavv/httpexpect) +### Subtests + Use [subtests](https://blog.golang.org/subtests) whenever possible to improve code readability and test output. +### Better output in tests + +When comparing expected and actual values in tests, use +[testify/require.Equal](https://godoc.org/github.com/stretchr/testify/require#Equal), +[testify/require.EqualError](https://godoc.org/github.com/stretchr/testify/require#EqualError), +[testify/require.EqualValues](https://godoc.org/github.com/stretchr/testify/require#EqualValues), +and others to improve readability when comparing structs, errors, +large portions of text, or JSON documents: + +```go +type TestData struct { + // ... +} + +func FuncUnderTest() TestData { + // ... +} + +func Test(t *testing.T) { + t.Run("FuncUnderTest", func(t *testing.T) { + want := TestData{} + got := FuncUnderTest() + + require.Equal(t, want, got) // note that expected value comes first, then comes the actual one ("diff" semantics) + }) +} +``` + ### Benchmarks Programs handling a lot of IO or complex operations should always include -- cgit v1.2.1 From 105bb6ffca3d53b81ee8d1cce6521ed28c2b1ba7 Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Wed, 24 Jul 2019 12:58:15 +0000 Subject: Add Go test guidelines --- doc/development/go_guide/index.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'doc/development/go_guide') diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index f827d240bf6..9f0ac8cc753 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -173,6 +173,45 @@ func Test(t *testing.T) { } ``` +### Table-Driven Tests + +Using [Table-Driven Tests](https://github.com/golang/go/wiki/TableDrivenTests) +is generally good practice when you have multiple entries of +inputs/outputs for the same function. Below are some guidelines one can +follow when writing table-driven test. These guidelines are mostly +extracted from Go standard library source code. Keep in mind it's OK not +to follow these guidelines when it makes sense. + +#### Defining test cases + +Each table entry is a complete test case with inputs and expected +results, and sometimes with additional information such as a test name +to make the test output easily readable. + +- [Define a slice of anonymous struct](https://github.com/golang/go/blob/50bd1c4d4eb4fac8ddeb5f063c099daccfb71b26/src/encoding/csv/reader_test.go#L16) + inside of the test. +- [Define a slice of anonymous struct](https://github.com/golang/go/blob/55d31e16c12c38d36811bdee65ac1f7772148250/src/cmd/go/internal/module/module_test.go#L9-L66) + outside of the test. +- [Named structs](https://github.com/golang/go/blob/2e0cd2aef5924e48e1ceb74e3d52e76c56dd34cc/src/cmd/go/internal/modfetch/coderepo_test.go#L54-L69) + for code reuse. +- [Using `map[string]struct{}`](https://github.com/golang/go/blob/6d5caf38e37bf9aeba3291f1f0b0081f934b1187/src/cmd/trace/annotations_test.go#L180-L235). + +#### Contents of the test case + +- Ideally, each test case should have a field with a unique identifier + to use for naming subtests. In the Go standard library, this is commonly the + `name string` field. +- Use `want`/`expect`/`actual` when you are specifcing something in the + test case that will be used for assertion. + +#### Variable names + +- Each table-driven test map/slice of struct can be named `tests`. +- When looping through `tests` the anonymous struct can be referred + to as `tt` or `tc`. +- The description of the test can be referred to as + `name`/`testName`/`tn`. + ### Benchmarks Programs handling a lot of IO or complex operations should always include -- cgit v1.2.1 From 89bccb02d1aadb6f336576a16598ba95aad14115 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 12 Aug 2019 14:50:37 +0100 Subject: Don't use go mod vendor This change comes out of a discussion between Ben Kochie and me, around this MR: https://gitlab.com/gitlab-org/gitlab-pages/merge_requests/164 gitlab-elasticsearch-indexer already uses `go mod` without a `vendor/` directory. It has caused some intermittent build failures in the gitlab-ce/ee CI pipelines, but has otherwise been fine. I think that treating our Go dependencies in the same way we do our Ruby or Node.js ones is reasonable, and it has some minor benefits: * Contributors find it easier to submit MRs * MRs are easier to review * Makefiles are easier to write --- doc/development/go_guide/index.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'doc/development/go_guide') diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index 9f0ac8cc753..83444093f9c 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -107,6 +107,32 @@ Modules](https://github.com/golang/go/wiki/Modules). It provides a way to define and lock dependencies for reproducible builds. It should be used whenever possible. +When Go Modules are in use, there should not be a `vendor/` directory. Instead, +Go will automatically download dependencies when they are needed to build the +project. This is in line with how dependencies are handled with Bundler in Ruby +projects, and makes merge requests easier to review. + +In some cases, such as building a Go project for it to act as a dependency of a +CI run for another project, removing the `vendor/` directory means the code must +be downloaded repeatedly, which can lead to intermittent problems due to rate +limiting or network failures. In these circumstances, you should cache the +downloaded code between runs with a `.gitlab-ci.yml` snippet like this: + +```yaml +.go-cache: + variables: + GOPATH: $CI_PROJECT_DIR/.go + before_script: + - mkdir -p .go + cache: + paths: + - .go/pkg/mod/ + +test: + extends: .go-cache + # ... +``` + There was a [bug on modules checksums](https://github.com/golang/go/issues/29278) in Go < v1.11.4, so make sure to use at least this version to avoid `checksum mismatch` errors. -- cgit v1.2.1