Skip to content

Commit

Permalink
fix(table-test): make guidance less presciptive
Browse files Browse the repository at this point in the history
  • Loading branch information
tyler-french committed Apr 18, 2023
1 parent cb642aa commit ce13a7e
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 4 deletions.
90 changes: 88 additions & 2 deletions src/test-table.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
# Test Tables

Use table-driven tests with [subtests] to avoid duplicating code when the core
test logic is repetitive.
Table-driven tests with [subtests] can be a helpful pattern for writing tests
to avoid duplicating code when the core test logic is repetitive.

If a system under test needs to be tested against _multiple conditions_ where
certain parts of the the inputs and outputs change, a table-driven test should
be used to reduce redundancy and improve readability.

[subtests]: https://blog.golang.org/subtests

Expand Down Expand Up @@ -100,6 +104,88 @@ for _, tt := range tests {
}
```

## Avoid Unnecessary Complexity in Table Tests

Table tests can be difficult to read and maintain if the subtests contain conditional
assertions or other branching logic. Table tests should **NOT** be used whenever
there needs to be complex or conditional logic inside subtests (i.e. logic inside the `for` loop).

Large, complex table tests harm readability and maintainability because test readers may
have difficulty debugging test failures that occur.

Table tests like this should be split into either multiple test tables or multiple
individual `Test...` functions.

Some ideals to aim for are:

* Focus on the narrowest unit of behavior
* Minimize `if` statements and other control flow (if possible)
* Ensure that all table fields are used in all tests
* Ensure that all test logic runs for all table cases

Concretely, that means that you should avoid
having multiple branching pathways (e.g. `shouldError`, `expectCall`, etc.),
using `if` statements for specific mock expectations (e.g. `shouldCallFoo`),
or placing functions inside the table (e.g. `setupMocks func(*FooMock)`).

As an exception, if the test body is short and straightforward,
it's acceptable to have a single branching pathway for success versus failure cases
with a table field like `wantErr` to specify error expectations.

For example, the following table test is confusing to read:

**Bad:**
```go
func TestComplicatedTable(t *testing.T) {
tests := []struct {
give string
want string
wantErr error
shouldCallX bool
shouldCallY bool
giveXResponse string
giveXErr error
giveYResponse string
giveYErr error
}{
// ...
}

for _, tt := range tests {
t.Run(tt.give, func(t *testing.T) {
// setup mocks
ctrl := gomock.NewController(t)
xMock := xmock.NewMockX(ctrl)
if tt.shouldCallX {
xMock.EXPECT().Call().Return(tt.giveXResponse, tt.giveXErr)
}
yMock := ymock.NewMockY(ctrl)
if tt.shouldCallY {
yMock.EXPECT().Call().Return(tt.giveYResponse, tt.giveYErr)
}

got, err := DoComplexThing(tt.give, xMock, yMock)

// verify results
if tt.wantErr != nil {
require.EqualError(t, err, tt.wantErr)
return
}
require.NoError(t, err)
assert.Equal(t, want, got)
})
}
}
```
This complexity makes it more difficult to change, understand, and prove the
correctness of the test.

While there are no strict guidelines, readability and maintainability should
always be top-of-mind when deciding between Table Tests versus separate tests
for multiple inputs/outputs to a system.

## Parallel Tests

Parallel tests, like some specialized loops (for example, those that spawn
goroutines or capture references as part of the loop body),
must take care to explicitly assign loop variables within the loop's scope to
Expand Down
92 changes: 90 additions & 2 deletions style.md
Original file line number Diff line number Diff line change
Expand Up @@ -3475,8 +3475,12 @@ See also [go vet: Printf family check](https://kuzminva.wordpress.com/2017/11/07

### Test Tables

Use table-driven tests with [subtests](https://blog.golang.org/subtests) to avoid duplicating code when the core
test logic is repetitive.
Table-driven tests with [subtests](https://blog.golang.org/subtests) can be a helpful pattern for writing tests
to avoid duplicating code when the core test logic is repetitive.

If a system under test needs to be tested against *multiple conditions* where
certain parts of the the inputs and outputs change, a table-driven test should
be used to reduce redundancy and improve readability.

<table>
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
Expand Down Expand Up @@ -3573,6 +3577,90 @@ for _, tt := range tests {
}
```

#### Avoid Unnecessary Complexity in Table Tests

Table tests can be difficult to read and maintain if the subtests contain conditional
assertions or other branching logic. Table tests should **NOT** be used whenever
there needs to be complex or conditional logic inside subtests (i.e. logic inside the `for` loop).

Large, complex table tests harm readability and maintainability because test readers may
have difficulty debugging test failures that occur.

Table tests like this should be split into either multiple test tables or multiple
individual `Test...` functions.

Some ideals to aim for are:

* Focus on the narrowest unit of behavior
* Minimize `if` statements and other control flow (if possible)
* Ensure that all table fields are used in all tests
* Ensure that all test logic runs for all table cases

Concretely, that means that you should avoid
having multiple branching pathways (e.g. `shouldError`, `expectCall`, etc.),
using `if` statements for specific mock expectations (e.g. `shouldCallFoo`),
or placing functions inside the table (e.g. `setupMocks func(*FooMock)`).

As an exception, if the test body is short and straightforward,
it's acceptable to have a single branching pathway for success versus failure cases
with a table field like `wantErr` to specify error expectations.

For example, the following table test is confusing to read:

**Bad:**

```go
func TestComplicatedTable(t *testing.T) {
tests := []struct {
give string
want string
wantErr error
shouldCallX bool
shouldCallY bool
giveXResponse string
giveXErr error
giveYResponse string
giveYErr error
}{
// ...
}

for _, tt := range tests {
t.Run(tt.give, func(t *testing.T) {
// setup mocks
ctrl := gomock.NewController(t)
xMock := xmock.NewMockX(ctrl)
if tt.shouldCallX {
xMock.EXPECT().Call().Return(tt.giveXResponse, tt.giveXErr)
}
yMock := ymock.NewMockY(ctrl)
if tt.shouldCallY {
yMock.EXPECT().Call().Return(tt.giveYResponse, tt.giveYErr)
}

got, err := DoComplexThing(tt.give, xMock, yMock)

// verify results
if tt.wantErr != nil {
require.EqualError(t, err, tt.wantErr)
return
}
require.NoError(t, err)
assert.Equal(t, want, got)
})
}
}
```

This complexity makes it more difficult to change, understand, and prove the
correctness of the test.

While there are no strict guidelines, readability and maintainability should
always be top-of-mind when deciding between Table Tests versus separate tests
for multiple inputs/outputs to a system.

#### Parallel Tests

Parallel tests, like some specialized loops (for example, those that spawn
goroutines or capture references as part of the loop body),
must take care to explicitly assign loop variables within the loop's scope to
Expand Down

0 comments on commit ce13a7e

Please sign in to comment.