Skip to content

Commit 309888d

Browse files
authored
new checker suite-broken-parallel (#118)
* suite-broken-parallel: debug cases * new checker 'suite-broken-parallel' * suite-broken-parallel: self-review
1 parent 78fc7cd commit 309888d

11 files changed

+702
-10
lines changed

README.md

+53-2
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ https://golangci-lint.run/usage/linters/#testifylint
103103
| [negative-positive](#negative-positive) |||
104104
| [nil-compare](#nil-compare) |||
105105
| [require-error](#require-error) |||
106+
| [suite-broken-parallel](#suite-broken-parallel) |||
106107
| [suite-dont-use-pkg](#suite-dont-use-pkg) |||
107108
| [suite-extra-assert-call](#suite-extra-assert-call) |||
108109
| [suite-subtest-run](#suite-subtest-run) |||
@@ -667,11 +668,61 @@ Also, to minimize false positives, `require-error` ignores:
667668

668669
---
669670

670-
### suite-dont-use-pkg
671+
### suite-broken-parallel
671672

672673
```go
673-
import "github.com/stretchr/testify/assert"
674+
func (s *MySuite) SetupTest() {
675+
s.T().Parallel() ❌
676+
}
677+
678+
// And other hooks...
679+
680+
func (s *MySuite) TestSomething() {
681+
s.T().Parallel() ❌
682+
683+
for _, tt := range cases {
684+
s.Run(tt.name, func() {
685+
s.T().Parallel() ❌
686+
})
687+
688+
s.T().Run(tt.name, func(t *testing.T) {
689+
t.Parallel() ❌
690+
})
691+
}
692+
}
693+
```
694+
695+
**Autofix**: true. <br>
696+
**Enabled by default**: true. <br>
697+
**Reason**: Protection from undefined behaviour.
698+
699+
`v1` of `testify` doesn't support suite's parallel tests and subtests.
700+
701+
Depending on [case](./analyzer/testdata/src/debug/suite_broken_parallel_test.go) using of `t.Parallel()` leads to
674702

703+
- data race
704+
- panic
705+
- non-working suite hooks
706+
- silent ignoring of this directive
707+
708+
So, `testify`'s maintainers recommend discourage parallel tests inside suite.
709+
710+
<details>
711+
<summary>Related issues...</summary>
712+
713+
- https://github.com/stretchr/testify/issues/187
714+
- https://github.com/stretchr/testify/issues/466
715+
- https://github.com/stretchr/testify/issues/934
716+
- https://github.com/stretchr/testify/issues/1139
717+
- https://github.com/stretchr/testify/issues/1253
718+
719+
</details>
720+
721+
---
722+
723+
### suite-dont-use-pkg
724+
725+
```go
675726
func (s *MySuite) TestSomething() {
676727
❌ assert.Equal(s.T(), 42, value)
677728
✅ s.Equal(42, value)

analyzer/checkers_factory_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,14 @@ func Test_newCheckers(t *testing.T) {
5151
checkers.NewBlankImport(),
5252
checkers.NewGoRequire(),
5353
checkers.NewRequireError(),
54+
checkers.NewSuiteBrokenParallel(),
5455
checkers.NewSuiteSubtestRun(),
5556
}
5657
allAdvancedCheckers := []checkers.AdvancedChecker{
5758
checkers.NewBlankImport(),
5859
checkers.NewGoRequire(),
5960
checkers.NewRequireError(),
61+
checkers.NewSuiteBrokenParallel(),
6062
checkers.NewSuiteSubtestRun(),
6163
checkers.NewSuiteTHelper(),
6264
}

analyzer/testdata/src/checkers-default/suite-broken-parallel/suite_broken_parallel_test.go

+92
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Code generated by testifylint/internal/testgen. DO NOT EDIT.
2+
3+
package suitebrokenparallel
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/suite"
10+
)
11+
12+
type SuiteBrokenParallelCheckerSuite struct {
13+
suite.Suite
14+
}
15+
16+
func TestSuiteBrokenParallelCheckerSuite(t *testing.T) {
17+
t.Parallel()
18+
suite.Run(t, new(SuiteBrokenParallelCheckerSuite))
19+
}
20+
21+
func (s *SuiteBrokenParallelCheckerSuite) BeforeTest(_, _ string) {
22+
}
23+
24+
func (s *SuiteBrokenParallelCheckerSuite) SetupTest() {
25+
}
26+
27+
func (s *SuiteBrokenParallelCheckerSuite) TestAll() {
28+
29+
s.Run("", func() {
30+
s.Equal(1, 2)
31+
})
32+
33+
s.T().Run("", func(t *testing.T) {
34+
assert.Equal(t, 1, 2)
35+
})
36+
37+
s.Run("", func() {
38+
s.Equal(1, 2)
39+
40+
s.Run("", func() {
41+
s.Equal(1, 2)
42+
43+
s.Run("", func() {
44+
s.Equal(1, 2)
45+
})
46+
47+
s.T().Run("", func(t *testing.T) {
48+
assert.Equal(t, 1, 2)
49+
})
50+
})
51+
})
52+
}
53+
54+
func (s *SuiteBrokenParallelCheckerSuite) TestTable() {
55+
cases := []struct{ Name string }{}
56+
57+
for _, tt := range cases {
58+
tt := tt
59+
s.T().Run(tt.Name, func(t *testing.T) {
60+
s.Equal(1, 2)
61+
})
62+
}
63+
64+
for _, tt := range cases {
65+
tt := tt
66+
s.Run(tt.Name, func() {
67+
s.Equal(1, 2)
68+
})
69+
}
70+
}
71+
72+
func TestSimpleTable(t *testing.T) {
73+
t.Parallel()
74+
75+
cases := []struct{ Name string }{}
76+
for _, tt := range cases {
77+
tt := tt
78+
t.Run(tt.Name, func(t *testing.T) {
79+
t.Parallel()
80+
assert.Equal(t, 1, 2)
81+
})
82+
}
83+
}

0 commit comments

Comments
 (0)