Skip to content

Commit 04f007a

Browse files
committed
1 parent eb1cdf6 commit 04f007a

File tree

1 file changed

+69
-49
lines changed

1 file changed

+69
-49
lines changed

tools/flakeguard/runner/runner.go

Lines changed: 69 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (r *Runner) RunTests() (*reports.TestReport, error) {
5656
r.rawOutputs[p] = &bytes.Buffer{}
5757
}
5858
separator := strings.Repeat("-", 80)
59-
r.rawOutputs[p].WriteString(fmt.Sprintf("Run %d%s\n", i+1, separator))
59+
r.rawOutputs[p].WriteString(fmt.Sprintf("Run %d\n%s\n", i+1, separator))
6060
}
6161
jsonFilePath, passed, err := r.runTests(p)
6262
if err != nil {
@@ -83,7 +83,7 @@ func (r *Runner) RunTests() (*reports.TestReport, error) {
8383
}, nil
8484
}
8585

86-
// RawOutput retrieves the raw output from the test runs, if CollectRawOutput enabled.
86+
// RawOutputs retrieves the raw output from the test runs, if CollectRawOutput enabled.
8787
// packageName : raw output
8888
func (r *Runner) RawOutputs() map[string]*bytes.Buffer {
8989
return r.rawOutputs
@@ -120,7 +120,6 @@ func (r *Runner) runTests(packageName string) (string, bool, error) {
120120
selectPattern := strings.Join(r.SelectTests, "$|^")
121121
args = append(args, fmt.Sprintf("-run=^%s$", selectPattern))
122122
}
123-
args = append(args, "2>/dev/null")
124123

125124
if r.Verbose {
126125
log.Printf("Running command: go %s\n", strings.Join(args, " "))
@@ -249,7 +248,6 @@ func parseTestResults(expectedRuns int, filePaths []string) ([]reports.TestResul
249248
result = testDetails[key]
250249
}
251250

252-
// TODO: This is a bit of a logical mess, probably worth a refactor
253251
if entryLine.Output != "" {
254252
if panicDetectionMode || raceDetectionMode { // currently collecting panic or race output
255253
detectedEntries = append(detectedEntries, entryLine)
@@ -281,22 +279,32 @@ func parseTestResults(expectedRuns int, filePaths []string) ([]reports.TestResul
281279
return nil, err
282280
}
283281
panicTestKey := fmt.Sprintf("%s/%s", entryLine.Package, panicTest)
284-
testDetails[panicTestKey].Panic = true
285-
testDetails[panicTestKey].Timeout = timeout
286-
testDetails[panicTestKey].Failures++
287-
testDetails[panicTestKey].Runs++
288-
// TODO: durations and panics are weird in the same way as Runs: lots of double-counting
289-
// duration, err := time.ParseDuration(strconv.FormatFloat(entryLine.Elapsed, 'f', -1, 64) + "s")
290-
// if err != nil {
291-
// return nil, fmt.Errorf("failed to parse duration: %w", err)
292-
// }
293-
// testDetails[panicTestKey].Durations = append(testDetails[panicTestKey].Durations, duration)
294-
testDetails[panicTestKey].Outputs = append(testDetails[panicTestKey].Outputs, entryLine.Output)
282+
283+
// Ensure the test exists in testDetails
284+
result, exists := testDetails[panicTestKey]
285+
if !exists {
286+
// Create a new TestResult if it doesn't exist
287+
result = &reports.TestResult{
288+
TestName: panicTest,
289+
TestPackage: entryLine.Package,
290+
PassRatio: 0,
291+
Outputs: []string{},
292+
PackageOutputs: []string{},
293+
}
294+
testDetails[panicTestKey] = result
295+
}
296+
297+
result.Panic = true
298+
result.Timeout = timeout
299+
result.Failures++
300+
result.Runs++
301+
302+
// Handle outputs
295303
for _, entry := range detectedEntries {
296304
if entry.Test == "" {
297-
testDetails[panicTestKey].PackageOutputs = append(testDetails[panicTestKey].PackageOutputs, entry.Output)
305+
result.PackageOutputs = append(result.PackageOutputs, entry.Output)
298306
} else {
299-
testDetails[panicTestKey].Outputs = append(testDetails[panicTestKey].Outputs, entry.Output)
307+
result.Outputs = append(result.Outputs, entry.Output)
300308
}
301309
}
302310
} else if raceDetectionMode {
@@ -305,21 +313,31 @@ func parseTestResults(expectedRuns int, filePaths []string) ([]reports.TestResul
305313
return nil, err
306314
}
307315
raceTestKey := fmt.Sprintf("%s/%s", entryLine.Package, raceTest)
308-
testDetails[raceTestKey].Race = true
309-
testDetails[raceTestKey].Failures++
310-
testDetails[raceTestKey].Runs++
311-
// TODO: durations and races are weird in the same way as Runs: lots of double-counting
312-
// duration, err := time.ParseDuration(strconv.FormatFloat(entryLine.Elapsed, 'f', -1, 64) + "s")
313-
// if err != nil {
314-
// return nil, fmt.Errorf("failed to parse duration: %w", err)
315-
// }
316-
// testDetails[raceTestKey].Durations = append(testDetails[raceTestKey].Durations, duration)
317-
testDetails[raceTestKey].Outputs = append(testDetails[raceTestKey].Outputs, entryLine.Output)
316+
317+
// Ensure the test exists in testDetails
318+
result, exists := testDetails[raceTestKey]
319+
if !exists {
320+
// Create a new TestResult if it doesn't exist
321+
result = &reports.TestResult{
322+
TestName: raceTest,
323+
TestPackage: entryLine.Package,
324+
PassRatio: 0,
325+
Outputs: []string{},
326+
PackageOutputs: []string{},
327+
}
328+
testDetails[raceTestKey] = result
329+
}
330+
331+
result.Race = true
332+
result.Failures++
333+
result.Runs++
334+
335+
// Handle outputs
318336
for _, entry := range detectedEntries {
319337
if entry.Test == "" {
320-
testDetails[raceTestKey].PackageOutputs = append(testDetails[raceTestKey].PackageOutputs, entry.Output)
338+
result.PackageOutputs = append(result.PackageOutputs, entry.Output)
321339
} else {
322-
testDetails[raceTestKey].Outputs = append(testDetails[raceTestKey].Outputs, entry.Output)
340+
result.Outputs = append(result.Outputs, entry.Output)
323341
}
324342
}
325343
}
@@ -383,28 +401,28 @@ func parseTestResults(expectedRuns int, filePaths []string) ([]reports.TestResul
383401
if parentTestResult, exists := testDetails[parentTestKey]; exists {
384402
if parentTestResult.Panic {
385403
for _, subTest := range subTests {
386-
subTestKey := fmt.Sprintf("%s/%s", parentTestKey, subTest)
387-
if _, exists := testDetails[subTestKey]; exists {
388-
if testDetails[subTestKey].Failures > 0 { // If the parent test panicked, any of its subtests that failed could be the culprit
389-
testDetails[subTestKey].Panic = true
390-
testDetails[subTestKey].Outputs = append(testDetails[subTestKey].Outputs, "Panic in parent test")
404+
subTestKey := fmt.Sprintf("%s/%s", parentTestResult.TestPackage, subTest)
405+
if subTestResult, exists := testDetails[subTestKey]; exists {
406+
if subTestResult.Failures > 0 { // If the parent test panicked, any of its subtests that failed could be the culprit
407+
subTestResult.Panic = true
408+
subTestResult.Outputs = append(subTestResult.Outputs, "Panic in parent test")
391409
}
392410
} else {
393-
fmt.Printf("WARN: expected to fine subtest '%s' inside parent test '%s', but not found\n", parentTestKey, subTestKey)
411+
log.Printf("WARN: expected to find subtest '%s' inside parent test '%s', but not found\n", subTestKey, parentTestKey)
394412
}
395413
}
396414
}
397415
} else {
398-
fmt.Printf("WARN: expected to find parent test '%s' for sub tests, but not found\n", parentTestKey)
416+
log.Printf("WARN: expected to find parent test '%s' for subtests, but not found\n", parentTestKey)
399417
}
400418
}
401419
for _, result := range testDetails {
402-
if result.Runs > expectedRuns { // Panics can introduce double-counting test failures, this is a hacky correction for it
420+
if result.Runs > expectedRuns { // Panics can introduce double-counting test failures, this is a correction for it
403421
if result.Panic {
404422
result.Failures = expectedRuns
405423
result.Runs = expectedRuns
406424
} else {
407-
fmt.Printf("WARN: '%s' has %d test runs, exceeding expected amount of %d in an unexpected way, this may be due to oddly presented panics\n", result.TestName, result.Runs, expectedRuns)
425+
log.Printf("WARN: '%s' has %d test runs, exceeding expected amount of %d; this may be due to unexpected panics\n", result.TestName, result.Runs, expectedRuns)
408426
}
409427
}
410428
// If a package panicked, all tests in that package will be marked as panicking
@@ -420,8 +438,7 @@ func parseTestResults(expectedRuns int, filePaths []string) ([]reports.TestResul
420438
return results, nil
421439
}
422440

423-
// properly attributes panics to the test that caused them
424-
// Go JSON output gets confused, especially when tests are run in parallel
441+
// attributePanicToTest properly attributes panics to the test that caused them.
425442
func attributePanicToTest(panicPackage string, panicEntries []entry) (test string, timeout bool, err error) {
426443
regexSanitizePanicPackage := filepath.Base(panicPackage)
427444
panicAttributionRe := regexp.MustCompile(fmt.Sprintf(`%s\.(Test[^\.\(]+)`, regexSanitizePanicPackage))
@@ -430,31 +447,33 @@ func attributePanicToTest(panicPackage string, panicEntries []entry) (test strin
430447
for _, entry := range panicEntries {
431448
entriesOutputs = append(entriesOutputs, entry.Output)
432449
if matches := panicAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
433-
return matches[1], false, nil
450+
testName := strings.TrimSpace(matches[1])
451+
return testName, false, nil
434452
}
435453
if matches := timeoutAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
436-
return matches[1], true, nil
454+
testName := strings.TrimSpace(matches[1])
455+
return testName, true, nil
437456
}
438457
}
439458
return "", false, fmt.Errorf("failed to attribute panic to test, using regex %s on these strings:\n%s", panicAttributionRe.String(), strings.Join(entriesOutputs, ""))
440459
}
441460

442-
// properly attributes races to the test that caused them
443-
// Go JSON output gets confused, especially when tests are run in parallel
461+
// attributeRaceToTest properly attributes races to the test that caused them.
444462
func attributeRaceToTest(racePackage string, raceEntries []entry) (string, error) {
445463
regexSanitizeRacePackage := filepath.Base(racePackage)
446464
raceAttributionRe := regexp.MustCompile(fmt.Sprintf(`%s\.(Test[^\.\(]+)`, regexSanitizeRacePackage))
447465
entriesOutputs := []string{}
448466
for _, entry := range raceEntries {
449467
entriesOutputs = append(entriesOutputs, entry.Output)
450468
if matches := raceAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
451-
return matches[1], nil
469+
testName := strings.TrimSpace(matches[1])
470+
return testName, nil
452471
}
453472
}
454473
return "", fmt.Errorf("failed to attribute race to test, using regex: %s on these strings:\n%s", raceAttributionRe.String(), strings.Join(entriesOutputs, ""))
455474
}
456475

457-
// parseSubTest checks if a test name is a subtest and returns the parent and sub names
476+
// parseSubTest checks if a test name is a subtest and returns the parent and sub names.
458477
func parseSubTest(testName string) (parentTestName, subTestName string) {
459478
parts := strings.SplitN(testName, "/", 2)
460479
if len(parts) == 1 {
@@ -463,7 +482,7 @@ func parseSubTest(testName string) (parentTestName, subTestName string) {
463482
return parts[0], parts[1]
464483
}
465484

466-
// prettyProjectPath returns the project path formatted for pretty printing in results
485+
// prettyProjectPath returns the project path formatted for pretty printing in results.
467486
func prettyProjectPath(projectPath string) (string, error) {
468487
// Walk up the directory structure to find go.mod
469488
absPath, err := filepath.Abs(projectPath)
@@ -493,8 +512,9 @@ func prettyProjectPath(projectPath string) (string, error) {
493512
for _, line := range strings.Split(string(goModData), "\n") {
494513
if strings.HasPrefix(line, "module ") {
495514
goProject := strings.TrimSpace(strings.TrimPrefix(line, "module "))
496-
projectPath = strings.TrimPrefix(projectPath, goProject)
497-
return filepath.Join(goProject, projectPath), nil
515+
relativePath := strings.TrimPrefix(projectPath, dir)
516+
relativePath = strings.TrimLeft(relativePath, string(os.PathSeparator))
517+
return filepath.Join(goProject, relativePath), nil
498518
}
499519
}
500520

0 commit comments

Comments
 (0)