Skip to content

Commit 70afda7

Browse files
committed
fix
1 parent 44b505a commit 70afda7

File tree

3 files changed

+261
-119
lines changed

3 files changed

+261
-119
lines changed

tools/flakeguard/runner/example_test_package/example_tests_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,17 @@ func TestParentWithFailingSubtest(t *testing.T) {
325325
t.Run("PassingSubtest", func(t *testing.T) {
326326
// pass
327327
})
328+
}
328329

329-
// t.Errorf("parent fails")
330+
func TestParentWithFailingParentAndSubtest(t *testing.T) {
331+
// Run a subtest that fails.
332+
t.Run("FailingSubtest", func(t *testing.T) {
333+
t.Errorf("This subtest always fails.")
334+
})
335+
// Run a subtest that passes.
336+
t.Run("PassingSubtest", func(t *testing.T) {
337+
// pass
338+
})
339+
// The parent test also fails.
340+
t.Errorf("parent fails")
330341
}

tools/flakeguard/runner/runner.go

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func (r *Runner) runCmd(testCmd []string, runIndex int) (tempFilePath string, pa
256256
// Non-zero exit code => test failure
257257
if ec.ExitCode() != 0 {
258258
passed = false
259-
err = nil // Well treat non-zero as not an actual error, but a test fail
259+
err = nil // We'll treat non-zero as not an actual error, but a test fail
260260
return
261261
}
262262
// Zero exit code => pass
@@ -283,7 +283,7 @@ type entry struct {
283283

284284
// parseTestResults orchestrates reading and parsing multiple JSON output files.
285285
func (r *Runner) parseTestResults(filePaths []string) ([]reports.TestResult, error) {
286-
// Well store test results keyed by "package/test"
286+
// We'll store test results keyed by "package/test"
287287
testResultsMap := make(map[string]*reports.TestResult)
288288

289289
// Track packages that had panics or data races
@@ -293,9 +293,6 @@ func (r *Runner) parseTestResults(filePaths []string) ([]reports.TestResult, err
293293
// Keep track of package-level outputs
294294
packageLevelOutputs := make(map[string][]string)
295295

296-
// We'll no longer keep testsWithSubTests
297-
// or pass it around to zeroOut logic.
298-
299296
expectedRuns := r.RunCount
300297

301298
for i, filePath := range filePaths {
@@ -499,21 +496,63 @@ func (r *Runner) handleNormalLine(
499496
}
500497
}
501498

502-
// normalizeParentFailures adjusts a test’s counts if the failures are due solely to subtests.
503-
// It uses the parent's root name (i.e. without nested subtest parts) when calling the helper.
499+
/*
500+
failedLinesIndicateRealParentFailure examines the failure output lines for a given parent test,
501+
and determines whether the parent itself has failed—not just one or more of its subtests.
502+
A "real" parent failure is identified by:
503+
1. The presence of a summary line in the output that begins with the expected prefix,
504+
e.g. "--- FAIL: TestParentWithFailingSubtest (".
505+
2. The presence of a file error line (indicated by ".go:") that mentions "parent"
506+
(case-insensitive), which serves as an additional heuristic to confirm that the parent test itself failed.
507+
508+
If both conditions are met, the function returns true; otherwise, it returns false.
509+
Parameters:
510+
- parentName: the name of the parent test (e.g. "TestParentWithFailingSubtest").
511+
- failOuts: a map (e.g. "stdout", "stderr") of output lines collected from the test run.
512+
513+
Returns:
514+
- true if a real parent-level failure is detected, false otherwise.
515+
*/
516+
func failedLinesIndicateRealParentFailure(parentName string, failOuts map[string][]string) bool {
517+
summaryPrefix := "--- FAIL: " + parentName + " ("
518+
foundSummary := false
519+
for _, lines := range failOuts {
520+
for _, line := range lines {
521+
trimmed := strings.TrimSpace(line)
522+
if strings.HasPrefix(trimmed, summaryPrefix) {
523+
foundSummary = true
524+
break
525+
}
526+
}
527+
}
528+
if !foundSummary {
529+
return false
530+
}
531+
// Look for a parent's file error line that mentions "parent".
532+
for _, lines := range failOuts {
533+
for _, line := range lines {
534+
trimmed := strings.TrimSpace(line)
535+
if strings.Contains(trimmed, ".go:") && strings.Contains(strings.ToLower(trimmed), "parent") {
536+
return true
537+
}
538+
}
539+
}
540+
return false
541+
}
542+
543+
// normalizeParentFailures processes only parent test results (those without "/" in TestName).
544+
// If only subtest failures are present (i.e. no real parent-level failure detected), the parent's
545+
// result is marked as successful (Failures set to 0, Successes = Runs, PassRatio = 1.0) while leaving the run count intact.
504546
func normalizeParentFailures(tests map[string]*reports.TestResult) {
505547
for _, res := range tests {
506-
// Skip normalization for subtests (they have a "/" in their TestName).
548+
// Skip subtests (they have "/" in their TestName).
507549
if strings.Contains(res.TestName, "/") {
508550
continue
509551
}
510552
if res.Failures == 0 {
511553
continue
512554
}
513-
// For parent tests, use the TestName as the root name.
514555
parentRootName := res.TestName
515-
// If no real parent-level failure is detected, mark the parent as success
516-
// while keeping the total run count intact.
517556
if !failedLinesIndicateRealParentFailure(parentRootName, res.FailedOutputs) {
518557
res.Failures = 0
519558
res.Successes = res.Runs
@@ -536,32 +575,6 @@ func countLeadingSpaces(s string) int {
536575
return count
537576
}
538577

539-
// failedLinesIndicateRealParentFailure scans the failure outputs for a file stacktrace line
540-
// that indicates a parent's failure. We assume that such a line starts with "/" and has fewer
541-
// than 8 leading spaces. We also skip any line that clearly belongs to a subtest by checking
542-
// that it does not contain the parent's root name immediately followed by a slash.
543-
func failedLinesIndicateRealParentFailure(parentName string, failOuts map[string][]string) bool {
544-
for _, lines := range failOuts {
545-
for _, line := range lines {
546-
trimmed := strings.TrimLeft(line, " ")
547-
// Look for a stacktrace line starting with "/" (file paths)
548-
if strings.HasPrefix(trimmed, "/") {
549-
indent := countLeadingSpaces(line)
550-
// In our logs, subtest stacktraces are indented 8 or more spaces.
551-
if indent < 8 {
552-
// If the line contains the parent's name immediately followed by "/", skip it.
553-
if strings.Contains(line, parentName+"/") {
554-
continue
555-
}
556-
// Otherwise, we have a parent-level failure.
557-
return true
558-
}
559-
}
560-
}
561-
}
562-
return false
563-
}
564-
565578
// getOrCreateTestResult is a small helper to create/lookup a TestResult struct in the map.
566579
func getOrCreateTestResult(
567580
testResultsMap map[string]*reports.TestResult,

0 commit comments

Comments
 (0)