Skip to content

Commit 1b86ac2

Browse files
docs: Add anti-pattern guidance for test parallelization (#3682)
1 parent 0554be8 commit 1b86ac2

1 file changed

Lines changed: 34 additions & 0 deletions

File tree

.github/copilot/instructions/build-performance.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,40 @@ for key, pin := range actionPins {
109109

110110
**Key Insight**: Network-bound tests benefit significantly from parallelization since they're I/O-bound rather than CPU-bound. Always capture loop variables when using `t.Parallel()`.
111111

112+
**Anti-Pattern: Parent-Aggregates-Results Tests (2025-11)**:
113+
```go
114+
// ✗ NOT SUITABLE for t.Parallel() - parent aggregates results
115+
func TestMultipleServers(t *testing.T) {
116+
setup := setupTest(t)
117+
defer setup.cleanup() // Runs before parallel subtests!
118+
119+
successCount := 0 // Shared state
120+
121+
for _, server := range servers {
122+
t.Run(server, func(t *testing.T) {
123+
t.Parallel() // ✗ BAD: Parent continues and cleans up
124+
successCount++ // ✗ BAD: Race condition
125+
})
126+
}
127+
128+
// ✗ BAD: This runs before parallel subtests execute!
129+
if successCount == 0 {
130+
t.Error("No servers succeeded")
131+
}
132+
}
133+
```
134+
135+
**Why it fails:**
136+
1. `t.Parallel()` pauses subtests until parent function returns
137+
2. Parent's `defer cleanup()` executes before subtests resume
138+
3. Cleanup deletes resources subtests need
139+
4. Parent checks `successCount` before subtests update it
140+
141+
**Solution:** Either:
142+
- Remove `t.Parallel()` for tests with parent aggregation
143+
- Restructure to avoid parent aggregation pattern
144+
- Use separate parent/child test functions
145+
112146
**Example Benchmark**:
113147
```go
114148
func BenchmarkCompileWorkflow(b *testing.B) {

0 commit comments

Comments
 (0)