Skip to content

Commit 5d507e3

Browse files
Migrate tests to testify (#487)
## Summary - migrate test assertions to testify / patterns across the suite - fix migration regressions by restoring test semantics where assertions were over-rewritten - keep matcher strictness pragmatic ( where numeric type differences are expected)
1 parent 3b54b5c commit 5d507e3

File tree

157 files changed

+13046
-16789
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

157 files changed

+13046
-16789
lines changed

.golangci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ linters:
99
- ineffassign
1010
- modernize
1111
- staticcheck
12+
- testifylint
1213
- unused
1314
settings:
1415
errcheck:

AGENTS.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ Test conventions:
153153
- PostgreSQL-only tests use `//go:build postgres`.
154154
- ACP adapter integration tests use `//go:build integration && acp`.
155155
- Shared helpers live in `internal/testenv/`, `internal/testutil/`, and package-local `*_test_helpers*.go`.
156+
- In tests with more than three assertions, prefer `assert := assert.New(t)` to make grouped assertions cleaner.
157+
- Avoid `assert.False*` / `require.False*` and do not use `assert.Fail`/`require.Fail` in tests.
158+
- Prefer `assert.Equal` for explicit expectations and avoid `assert.True*` / `require.True*` unless checking a boolean result directly.
159+
- Convert redundant `if` wrappers around assertions into direct assertions (for example, `if err != nil { require.NoError(t, err) }` should become `require.NoError(t, err)`).
160+
- Do not replace assertions with manual control-flow (`if`, `t.Fatal*`, `t.Error*`) when a direct testify check covers the same condition.
161+
- Enforce a no-redundant-guards policy for assertions:
162+
- replace `if err != nil { require.NoError(t, err, ...) }` and `if err == nil { ... } else { ... }` with direct `require.Error`/`require.NoError`/`require.NotNil` statements.
163+
- avoid manual `if` prechecks such as `if got != want` or `if cfg != nil`; convert to direct `assert.Equal`/`assert.NotNil` assertions.
164+
- remove `assert`/`require` fail helpers and `t.Fatal`/`t.Fatalf`/`t.Error` usage when a direct assertion provides the same check.
156165

157166
## Search Shortcuts
158167

cmd/roborev/analyze_integration_test.go

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ import (
66
"context"
77
"net/http"
88
"regexp"
9-
"strings"
109
"sync/atomic"
1110
"testing"
1211
"time"
1312

1413
"github.com/roborev-dev/roborev/internal/prompt/analyze"
1514
"github.com/roborev-dev/roborev/internal/storage"
15+
"github.com/stretchr/testify/assert"
16+
"github.com/stretchr/testify/require"
1617
)
1718

1819
func TestWaitForAnalysisJob(t *testing.T) {
@@ -97,24 +98,14 @@ func TestWaitForAnalysisJob(t *testing.T) {
9798
review, err := waitForAnalysisJob(ctx, ts.URL, testJobID)
9899

99100
if tt.wantErr {
100-
if err == nil {
101-
t.Fatal("expected error, got nil")
102-
}
103-
if !strings.Contains(err.Error(), tt.wantErrMsg) {
104-
t.Errorf("error %q should contain %q", err.Error(), tt.wantErrMsg)
105-
}
101+
require.Error(t, err)
102+
assert.Contains(t, err.Error(), tt.wantErrMsg)
106103
return
107104
}
108105

109-
if err != nil {
110-
t.Fatalf("unexpected error: %v", err)
111-
}
112-
if review == nil {
113-
t.Fatal("expected review, got nil")
114-
}
115-
if review.Output != tt.wantReview {
116-
t.Errorf("got review %q, want %q", review.Output, tt.wantReview)
117-
}
106+
require.NoError(t, err)
107+
require.NotNil(t, review, "expected review, got nil")
108+
assert.Equal(t, tt.wantReview, review.Output)
118109
})
119110
}
120111
}
@@ -142,27 +133,15 @@ func TestRunAnalyzeAndFix_Integration(t *testing.T) {
142133
}
143134

144135
err := runAnalyzeAndFix(cmd, ts.URL, repo.Dir, 99, analysisType, opts)
145-
if err != nil {
146-
t.Fatalf("runAnalyzeAndFix failed: %v", err)
147-
}
136+
require.NoError(t, err, "runAnalyzeAndFix failed: %v")
148137

149138
// Verify the workflow was executed
150-
if atomic.LoadInt32(&state.JobsCount) < 2 {
151-
t.Error("should have polled for job status")
152-
}
153-
if atomic.LoadInt32(&state.ReviewCount) == 0 {
154-
t.Error("should have fetched the review")
155-
}
156-
if atomic.LoadInt32(&state.CloseCount) == 0 {
157-
t.Error("should have marked job as closed")
158-
}
139+
assert.GreaterOrEqual(t, atomic.LoadInt32(&state.JobsCount), int32(2))
140+
assert.NotZero(t, atomic.LoadInt32(&state.ReviewCount))
141+
assert.NotZero(t, atomic.LoadInt32(&state.CloseCount))
159142

160143
// Verify output contains analysis result
161144
outputStr := output.String()
162-
if !strings.Contains(outputStr, "CODE SMELLS") {
163-
t.Error("output should contain analysis result")
164-
}
165-
if !regexp.MustCompile(`Analysis job \d+ closed`).MatchString(outputStr) {
166-
t.Errorf("output should match 'Analysis job N closed', got: %s", outputStr)
167-
}
145+
assert.Contains(t, outputStr, "CODE SMELLS")
146+
assert.Regexp(t, regexp.MustCompile(`Analysis job \d+ closed`), outputStr)
168147
}

0 commit comments

Comments
 (0)