google / google/badwolf

Refactor tests in BadWolf to use t.Run()

Open
#152 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
984
Forks
65
PR merge metrics
No merged PRs in 30d

Description

Given a test table, when running subtests for each element in the table it is helpful to have a short description to showcase what exactly is being tested there, and use `t.Run()` so these subtests will be printed separately. This improves readability and makes it easier to understand what exactly went wrong in the case a given test fails.

For reference, have a look at:

1) "Go by Example: Testing": https://gobyexample.com/testing

2) "Subtests and Sub-benchmarks": https://golang.org/pkg/testing/#hdr-Subtests_and_Sub_benchmarks

To illustrate with an example taken from 1):

```
func TestIntMinTableDriven(t *testing.T) {
var tests = []struct {
a, b int
want int
}{
{0, 1, 0},
{1, 0, 0},
{2, -2, -2},
{0, -1, -1},
{-1, 0, -1},
}

for _, tt := range tests {
testname := fmt.Sprintf("%d,%d", tt.a, tt.b)
t.Run(testname, func(t *testing.T) {
ans := IntMin(tt.a, tt.b)
if ans != tt.want {
t.Errorf("got %d, want %d", ans, tt.want)
}
})
}
}
```

Which is printed as:

```
$ go test -v
=== RUN TestIntMinTableDriven
=== RUN TestIntMinTableDriven/0,1
=== RUN TestIntMinTableDriven/1,0
=== RUN TestIntMinTableDriven/2,-2
=== RUN TestIntMinTableDriven/0,-1
=== RUN TestIntMinTableDriven/-1,0
--- PASS: TestIntMinTableDriven (0.00s)
--- PASS: TestIntMinTableDriven/0,1 (0.00s)
--- PASS: TestIntMinTableDriven/1,0 (0.00s)
--- PASS: TestIntMinTableDriven/2,-2 (0.00s)
--- PASS: TestIntMinTableDriven/0,-1 (0.00s)
--- PASS: TestIntMinTableDriven/-1,0 (0.00s)
```

The subtests names are also shown when a test fails, with `_` being automatically used to separate the words in the complete name of the test/subtest. To illustrate, if inside `TestFilterClauseManipulation` in `semantic_test.go` you use `t.Run("workingFilter initial states ok", func(t *testing.T) {` for a subtest it will be printed as:

```
TestFilterClauseManipulation/workingFilter_initial_states_ok
```

Note that there is also this recommendation that sometimes it is good to include the expected return for your test case, like "workingFilter with initial states success"/"returns OK" above, or "final length filters list expects 0" (from `TestWhereFilterClauseHookError` in `hooks_test.go`).

For reference, the test functions `TestWhereFilterClauseHook` and `TestWhereFilterClauseHookError` in `hooks_test.go` are already using `t.Run()` and following these guidelines, if you want to have a look.

This Issue was born from comments made by a Go readability reviewer in the PRs 127 and 149.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.