Skip to content

Commit ea85f0c

Browse files
kalverrakalverra
committed
Properly attribute races and panics
1 parent 8779ef4 commit ea85f0c

File tree

4 files changed

+547
-92
lines changed

4 files changed

+547
-92
lines changed

‎tools/flakeguard/.gitignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
tmp_test_flaky_state
22
test_results_*.json
3-
debug*.json
3+
debug_outputs/

‎tools/flakeguard/reports/reports.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ type TestResult struct {
1717
TestName string
1818
TestPackage string
1919
Panicked bool // Indicates a test-level panic
20+
Skipped bool // Indicates if the test was skipped
2021
PackagePanicked bool // Indicates a package-level panic
2122
PassRatio float64 // Pass ratio in decimal format like 0.5
2223
PassRatioPercentage string // Pass ratio in percentage format like "50%"
23-
Skipped bool // Indicates if the test was skipped
2424
Runs int // Count of how many times the test was run
2525
Failures int // Count of how many times the test failed
2626
Successes int // Count of how many times the test passed
2727
Panics int // Count of how many times the test panicked
28+
Races int // Count of how many times the test encountered a data race
2829
Skips int // Count of how many times the test was skipped
2930
Outputs []string // Stores outputs for a test
3031
Durations []time.Duration // Stores elapsed time for each run of the test

‎tools/flakeguard/runner/runner.go‎

Lines changed: 125 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"log"
1111
"os"
1212
"os/exec"
13+
"path/filepath"
1314
"regexp"
1415
"strconv"
1516
"strings"
@@ -19,7 +20,8 @@ import (
1920
)
2021

2122
var (
22-
panicRe = regexp.MustCompile(`^panic:`)
23+
startPanicRe = regexp.MustCompile(`^panic:`)
24+
startRaceRe = regexp.MustCompile(`^WARNING: DATA RACE`)
2325
)
2426

2527
type Runner struct {
@@ -40,6 +42,16 @@ func (r *Runner) RunTests() ([]reports.TestResult, error) {
4042
var jsonFilePaths []string
4143
for _, p := range r.SelectedTestPackages {
4244
for i := 0; i < r.RunCount; i++ {
45+
if r.CollectRawOutput { // Collect raw output for debugging
46+
if r.rawOutputs == nil {
47+
r.rawOutputs = make(map[string]*bytes.Buffer)
48+
}
49+
if _, exists := r.rawOutputs[p]; !exists {
50+
r.rawOutputs[p] = &bytes.Buffer{}
51+
}
52+
separator := strings.Repeat("-", 80)
53+
r.rawOutputs[p].WriteString(fmt.Sprintf("%d%s\n", i, separator))
54+
}
4355
jsonFilePath, passed, err := r.runTests(p)
4456
if err != nil {
4557
return nil, fmt.Errorf("failed to run tests in package %s: %w", p, err)
@@ -91,10 +103,6 @@ func (r *Runner) runTests(packageName string) (string, bool, error) {
91103
cmd := exec.Command("go", args...)
92104
cmd.Dir = r.ProjectPath
93105
if r.CollectRawOutput {
94-
if r.rawOutputs == nil {
95-
r.rawOutputs = make(map[string]*bytes.Buffer)
96-
}
97-
r.rawOutputs[packageName] = &bytes.Buffer{}
98106
cmd.Stdout = io.MultiWriter(tmpFile, r.rawOutputs[packageName])
99107
cmd.Stderr = io.MultiWriter(tmpFile, r.rawOutputs[packageName])
100108
} else {
@@ -115,9 +123,25 @@ func (r *Runner) runTests(packageName string) (string, bool, error) {
115123
return tmpFile.Name(), true, nil // Test succeeded
116124
}
117125

126+
type entry struct {
127+
Action string `json:"Action"`
128+
Test string `json:"Test"`
129+
Package string `json:"Package"`
130+
Output string `json:"Output"`
131+
Elapsed float64 `json:"Elapsed"` // Decimal value in seconds
132+
}
133+
118134
// parseTestResults reads the test output files and returns the parsed test results.
119135
func parseTestResults(filePaths []string) ([]reports.TestResult, error) {
120-
testDetails := make(map[string]*reports.TestResult) // Holds run, pass counts, and other details for each test
136+
var (
137+
testDetails = make(map[string]*reports.TestResult) // Holds run, pass counts, and other details for each test
138+
panickedPackages = map[string]struct{}{} // Packages with tests that panicked
139+
racePackages = map[string]struct{}{} // Packages with tests that raced
140+
packageLevelOutputs = map[string][]string{} // Package-level outputs
141+
panicDetectionMode = false
142+
raceDetectionMode = false
143+
detectedEntries = []entry{} // race or panic entries
144+
)
121145

122146
// Process each file
123147
for _, filePath := range filePaths {
@@ -139,14 +163,8 @@ func parseTestResults(filePaths []string) ([]reports.TestResult, error) {
139163
precedingLines = precedingLines[1:]
140164
}
141165

142-
var entry struct {
143-
Action string `json:"Action"`
144-
Test string `json:"Test"`
145-
Package string `json:"Package"`
146-
Output string `json:"Output"`
147-
Elapsed float64 `json:"Elapsed"` // Decimal value in seconds
148-
}
149-
if err := json.Unmarshal(scanner.Bytes(), &entry); err != nil {
166+
var entryLine entry
167+
if err := json.Unmarshal(scanner.Bytes(), &entryLine); err != nil {
150168
// Collect 15 lines after the error for more context
151169
for scanner.Scan() && len(followingLines) < 15 {
152170
followingLines = append(followingLines, scanner.Text())
@@ -159,14 +177,14 @@ func parseTestResults(filePaths []string) ([]reports.TestResult, error) {
159177

160178
// Only create TestResult for test-level entries
161179
var result *reports.TestResult
162-
if entry.Test != "" {
180+
if entryLine.Test != "" {
163181
// Determine the key
164-
key := entry.Package + "/" + entry.Test
182+
key := fmt.Sprintf("%s/%s", entryLine.Package, entryLine.Test)
165183

166184
if _, exists := testDetails[key]; !exists {
167185
testDetails[key] = &reports.TestResult{
168-
TestName: entry.Test,
169-
TestPackage: entry.Package,
186+
TestName: entryLine.Test,
187+
TestPackage: entryLine.Package,
170188
PassRatio: 0,
171189
Outputs: []string{},
172190
PackageOutputs: []string{},
@@ -175,79 +193,91 @@ func parseTestResults(filePaths []string) ([]reports.TestResult, error) {
175193
result = testDetails[key]
176194
}
177195

178-
// Collect outputs
179-
if entry.Output != "" {
180-
if entry.Test != "" {
181-
// Test-level output
182-
result.Outputs = append(result.Outputs, strings.TrimSpace(entry.Output))
183-
} else {
184-
// Package-level output
185-
// Append to PackageOutputs of all TestResults in the same package
186-
for _, res := range testDetails {
187-
if res.TestPackage == entry.Package {
188-
res.PackageOutputs = append(res.PackageOutputs, entry.Output)
196+
// TODO: This is a bit of a logical mess, refactor
197+
if entryLine.Output != "" {
198+
if panicDetectionMode || raceDetectionMode { // currently collecting panic or race output
199+
detectedEntries = append(detectedEntries, entryLine)
200+
if entryLine.Action == "fail" { // End of panic output
201+
if panicDetectionMode {
202+
panicTest, err := attributePanicToTest(entryLine.Package, detectedEntries)
203+
if err != nil {
204+
return nil, err
205+
}
206+
panicTestKey := fmt.Sprintf("%s/%s", entryLine.Package, panicTest)
207+
testDetails[panicTestKey].Panicked = true
208+
testDetails[panicTestKey].Panics++
209+
testDetails[panicTestKey].Runs++
210+
testDetails[panicTestKey].Outputs = append(testDetails[panicTestKey].Outputs, entryLine.Output)
211+
} else if raceDetectionMode {
212+
raceTest, err := attributeRaceToTest(entryLine.Package, detectedEntries)
213+
if err != nil {
214+
return nil, err
215+
}
216+
raceTestKey := fmt.Sprintf("%s/%s", entryLine.Package, raceTest)
217+
testDetails[raceTestKey].Races++
218+
testDetails[raceTestKey].Runs++
219+
testDetails[raceTestKey].Outputs = append(testDetails[raceTestKey].Outputs, entryLine.Output)
189220
}
221+
222+
detectedEntries = []entry{}
223+
panicDetectionMode = false
224+
raceDetectionMode = false
190225
}
226+
continue // Don't process this entry further
227+
} else if startPanicRe.MatchString(entryLine.Output) { // found a panic, start collecting output
228+
panickedPackages[entryLine.Package] = struct{}{}
229+
detectedEntries = append(detectedEntries, entryLine)
230+
panicDetectionMode = true
231+
continue // Don't process this entry further
232+
} else if startRaceRe.MatchString(entryLine.Output) {
233+
racePackages[entryLine.Package] = struct{}{}
234+
detectedEntries = append(detectedEntries, entryLine)
235+
raceDetectionMode = true
236+
continue // Don't process this entry further
237+
} else if entryLine.Test == "" {
238+
if _, exists := packageLevelOutputs[entryLine.Package]; !exists {
239+
packageLevelOutputs[entryLine.Package] = []string{}
240+
}
241+
packageLevelOutputs[entryLine.Package] = append(packageLevelOutputs[entryLine.Package], entryLine.Output)
242+
} else if entryLine.Test != "" {
243+
result.Outputs = append(result.Outputs, entryLine.Output)
191244
}
192245
}
193246

194-
switch entry.Action {
195-
case "run":
196-
if entry.Test != "" {
197-
result.Runs++
198-
}
247+
switch entryLine.Action {
199248
case "pass":
200-
if entry.Test != "" {
201-
duration, err := time.ParseDuration(strconv.FormatFloat(entry.Elapsed, 'f', -1, 64) + "s")
249+
if entryLine.Test != "" {
250+
duration, err := time.ParseDuration(strconv.FormatFloat(entryLine.Elapsed, 'f', -1, 64) + "s")
202251
if err != nil {
203252
return nil, fmt.Errorf("failed to parse duration: %w", err)
204253
}
205254
result.Durations = append(result.Durations, duration)
206255
result.Successes++
256+
result.Runs++
207257
}
208258
case "fail":
209-
if entry.Test != "" {
210-
duration, err := time.ParseDuration(strconv.FormatFloat(entry.Elapsed, 'f', -1, 64) + "s")
259+
if entryLine.Test != "" {
260+
duration, err := time.ParseDuration(strconv.FormatFloat(entryLine.Elapsed, 'f', -1, 64) + "s")
211261
if err != nil {
212262
return nil, fmt.Errorf("failed to parse duration: %w", err)
213263
}
214264
result.Durations = append(result.Durations, duration)
215265
result.Failures++
216-
}
217-
case "output":
218-
// plain output already handled above
219-
if panicRe.MatchString(entry.Output) {
220-
if entry.Test != "" {
221-
duration, err := time.ParseDuration(strconv.FormatFloat(entry.Elapsed, 'f', -1, 64) + "s")
222-
if err != nil {
223-
return nil, fmt.Errorf("failed to parse duration: %w", err)
224-
}
225-
result.Durations = append(result.Durations, duration)
226-
// Test-level panic
227-
result.Panicked = true
228-
result.Panics++
229-
} else {
230-
// Package-level panic
231-
// Mark PackagePanicked for all TestResults in the package
232-
for _, res := range testDetails {
233-
if res.TestPackage == entry.Package {
234-
res.PackagePanicked = true
235-
}
236-
}
237-
}
266+
result.Runs++
238267
}
239268
case "skip":
240-
if entry.Test != "" {
241-
duration, err := time.ParseDuration(strconv.FormatFloat(entry.Elapsed, 'f', -1, 64) + "s")
269+
if entryLine.Test != "" {
270+
duration, err := time.ParseDuration(strconv.FormatFloat(entryLine.Elapsed, 'f', -1, 64) + "s")
242271
if err != nil {
243272
return nil, fmt.Errorf("failed to parse duration: %w", err)
244273
}
245274
result.Durations = append(result.Durations, duration)
246275
result.Skipped = true
247276
result.Skips++
277+
result.Runs++
248278
}
249279
}
250-
if entry.Test != "" {
280+
if entryLine.Test != "" {
251281
result.PassRatio = float64(result.Successes) / float64(result.Runs)
252282
result.PassRatioPercentage = fmt.Sprintf("%.0f%%", result.PassRatio*100)
253283
}
@@ -268,7 +298,39 @@ func parseTestResults(filePaths []string) ([]reports.TestResult, error) {
268298
var results []reports.TestResult
269299
for _, result := range testDetails {
270300
results = append(results, *result)
301+
if _, panicked := panickedPackages[result.TestPackage]; panicked {
302+
result.PackagePanicked = true
303+
}
304+
if outputs, exists := packageLevelOutputs[result.TestPackage]; exists {
305+
result.PackageOutputs = outputs
306+
}
271307
}
272308

273309
return results, nil
274310
}
311+
312+
// properly attributes panics to the test that caused them
313+
// Go JSON output gets confused, especially when tests are run in parallel
314+
func attributePanicToTest(panicPackage string, panicEntries []entry) (string, error) {
315+
regexSanitizePanicPackage := filepath.Base(panicPackage)
316+
panicAttributionRe := regexp.MustCompile(fmt.Sprintf(`%s\.(Test.*?)\(.*\)`, regexSanitizePanicPackage))
317+
for _, entry := range panicEntries {
318+
if matches := panicAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
319+
return matches[1], nil
320+
}
321+
}
322+
return "", fmt.Errorf("failed to attribute panic to test, using regex: %s", panicAttributionRe.String())
323+
}
324+
325+
// properly attributes races to the test that caused them
326+
// Go JSON output gets confused, especially when tests are run in parallel
327+
func attributeRaceToTest(racePackage string, raceEntries []entry) (string, error) {
328+
regexSanitizeRacePackage := filepath.Base(racePackage)
329+
raceAttributionRe := regexp.MustCompile(fmt.Sprintf(`%s\.(Test[^\.]+?)\(.*\)`, regexSanitizeRacePackage))
330+
for _, entry := range raceEntries {
331+
if matches := raceAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
332+
return matches[1], nil
333+
}
334+
}
335+
return "", fmt.Errorf("failed to attribute race to test, using regex: %s", raceAttributionRe.String())
336+
}

0 commit comments

Comments
 (0)