mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
ci: improve 'tfail in goroutine' ruleguard rule (#19682)
This PR improves the ruleguard rule for detecting `t.Fail` calls in goroutines. It picks up additional violations, of which are fixed in this PR. See self-review for details. The motivation for fixing this comes from a flake I fixed in https://github.com/coder/coder/pull/19599, where tests would fail from a `require` in an `Eventually`.
This commit is contained in:
@@ -9,6 +9,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/archive"
|
||||
@@ -88,7 +89,7 @@ func TestPostFiles(t *testing.T) {
|
||||
data := make([]byte, 1024)
|
||||
_, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data))
|
||||
end.Done()
|
||||
require.NoError(t, err)
|
||||
assert.NoError(t, err)
|
||||
}()
|
||||
}
|
||||
wg.Done()
|
||||
|
||||
@@ -409,8 +409,8 @@ func TestPGCoordinatorSingle_SendsHeartbeats(t *testing.T) {
|
||||
if len(heartbeats) < 2 {
|
||||
return false
|
||||
}
|
||||
require.Greater(t, heartbeats[0].Sub(start), time.Duration(0))
|
||||
require.Greater(t, heartbeats[1].Sub(start), time.Duration(0))
|
||||
assert.Greater(t, heartbeats[0].Sub(start), time.Duration(0))
|
||||
assert.Greater(t, heartbeats[1].Sub(start), time.Duration(0))
|
||||
return assert.Greater(t, heartbeats[1].Sub(heartbeats[0]), tailnet.HeartbeatPeriod*3/4)
|
||||
}, testutil.WaitMedium, testutil.IntervalMedium)
|
||||
}
|
||||
|
||||
@@ -257,7 +257,7 @@ func Test_Runner(t *testing.T) {
|
||||
err := runner.Run(runnerCtx, "1", logs)
|
||||
logsStr := logs.String()
|
||||
t.Log("Runner logs:\n\n" + logsStr)
|
||||
require.ErrorIs(t, err, context.Canceled)
|
||||
assert.ErrorIs(t, err, context.Canceled)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
|
||||
+4
-8
@@ -182,32 +182,28 @@ func doNotCallTFailNowInsideGoroutine(m dsl.Matcher) {
|
||||
m.Match(`
|
||||
go func($*_){
|
||||
$*_
|
||||
$require.$_($*_)
|
||||
require.$_($*_)
|
||||
$*_
|
||||
}($*_)`).
|
||||
At(m["require"]).
|
||||
Where(m["require"].Text == "require").
|
||||
Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)")
|
||||
|
||||
// require.Eventually runs the function in a goroutine.
|
||||
m.Match(`
|
||||
require.Eventually(t, func() bool {
|
||||
$*_
|
||||
$require.$_($*_)
|
||||
require.$_($*_)
|
||||
$*_
|
||||
}, $*_)`).
|
||||
At(m["require"]).
|
||||
Where(m["require"].Text == "require").
|
||||
Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)")
|
||||
|
||||
m.Match(`
|
||||
go func($*_){
|
||||
$*_
|
||||
$t.$fail($*_)
|
||||
t.$fail($*_)
|
||||
$*_
|
||||
}($*_)`).
|
||||
At(m["fail"]).
|
||||
Where(m["t"].Type.Implements("testing.TB") && m["fail"].Text.Matches("^(FailNow|Fatal|Fatalf)$")).
|
||||
Where(m["fail"].Text.Matches("^(FailNow|Fatal|Fatalf)$")).
|
||||
Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)")
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user