Fix flakereport

Author: stephanos

.github/workflows/run-tests.yml

@@ -354,12 +354,19 @@ jobs:
           flags: unit-test
           report_type: test_results
 
+      - name: Get job ID
+        id: get_job_id
+        uses: ./.github/actions/get-job-id
+        with:
+          job_name: Unit test
+          run_id: ${{ github.run_id }}
+
       - name: Upload test results to GitHub
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/upload-artifact@v4.4.3
         if: ${{ !cancelled() }}
         with:
-          name: junit-xml--${{github.run_id}}--${{github.run_attempt}}--unit-test
+          name: junit-xml--${{ github.run_id }}--${{ steps.get_job_id.outputs.job_id }}--${{ github.run_attempt }}--unit-test
           path: ./.testoutput/junit.*.xml
           include-hidden-files: true
           retention-days: 28
@@ -447,12 +454,19 @@ jobs:
           flags: integration-test
           report_type: test_results
 
+      - name: Get job ID
+        id: get_job_id
+        uses: ./.github/actions/get-job-id
+        with:
+          job_name: Integration test
+          run_id: ${{ github.run_id }}
+
       - name: Upload test results to GitHub
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/upload-artifact@v4.4.3
         if: ${{ !cancelled() }}
         with:
-          name: junit-xml--${{github.run_id}}--${{github.run_attempt}}--integration-test
+          name: junit-xml--${{ github.run_id }}--${{ steps.get_job_id.outputs.job_id }}--${{ github.run_attempt }}--integration-test
           path: ./.testoutput/junit.*.xml
           include-hidden-files: true
           retention-days: 28

tools/flakereport/flakereport.go

@@ -259,8 +259,9 @@ func runGenerateCommand(c *cli.Context) (err error) {
 	// Count test runs by name for failure rate calculation
 	testRunCounts := countTestRuns(allTestRuns)
 
-	// Group failures by test name
+	// Group failures by test name, then remove parent entries whose subtests were observed.
 	grouped := groupFailuresByTest(allFailures)
+	filterParentTests(grouped, testRunCounts)
 	fmt.Printf("Unique tests with failures: %d\n", len(grouped))
 
 	// Classify failures

tools/flakereport/github.go

@@ -191,20 +191,31 @@ func extractArtifactZip(zipPath, outputDir string) ([]string, error) {
 	return xmlFiles, nil
 }
 
-// parseArtifactName extracts run_id and job_id from artifact name
-// Format: {prefix}--{run_id}--{job_id}--{suffix}
-// Returns: runID, jobID (or "unknown" if not parseable)
-func parseArtifactName(artifactName string) (runID string, jobID string) {
+// parseArtifactName extracts run_id, job_id, and matrix_name from artifact name.
+// Functional tests: junit-xml--{run_id}--{job_id}--{run_attempt}--{matrix_name}--{display_name}--functional-test
+// Unit/integration:  junit-xml--{run_id}--{job_id}--{run_attempt}--unit-test
+// Returns: runID, jobID, matrixName ("unknown" for fields that are absent or unparseable)
+func parseArtifactName(artifactName string) (runID string, jobID string, matrixName string) {
 	parts := strings.Split(artifactName, "--")
-	if len(parts) >= 3 {
-		runID = parts[1]
-		jobID = parts[2]
-		if jobID == "" {
-			jobID = "unknown"
-		}
-		return runID, jobID
+	if len(parts) < 3 {
+		return "unknown", "unknown", "unknown"
+	}
+
+	runID = parts[1]
+
+	jobID = parts[2]
+	if jobID == "" {
+		jobID = "unknown"
 	}
-	return "unknown", "unknown"
+
+	// Functional test artifacts carry a matrix name (DB config) at parts[4].
+	if len(parts) >= 7 && parts[len(parts)-1] == "functional-test" {
+		matrixName = parts[4]
+	} else {
+		matrixName = "unknown"
+	}
+
+	return runID, jobID, matrixName
 }
 
 // buildGitHubURL constructs GitHub Actions URL from run/job IDs

tools/flakereport/parallel.go

@@ -135,7 +135,8 @@ func processArtifactJob(ctx context.Context, job ArtifactJob, totalArtifacts int
 		result.Failures = append(result.Failures, failures...)
 
 		// Extract all test runs for failure rate calculation
-		testRuns := extractAllTestRuns(suites, job.RunID)
+		_, jobID, matrixName := parseArtifactName(job.Artifact.Name)
+		testRuns := extractAllTestRuns(suites, job.RunID, jobID, matrixName)
 		result.AllRuns = append(result.AllRuns, testRuns...)
 	}
 

tools/flakereport/parser.go

@@ -68,8 +68,8 @@ func isGoTestSuite(name string) bool {
 func extractFailures(suites *junit.Testsuites, artifactName string, runID int64, timestamp time.Time) []TestFailure {
 	var failures []TestFailure
 
-	// Parse artifact name for run_id and job_id
-	_, jobID := parseArtifactName(artifactName)
+	// Parse artifact name for run_id, job_id, and matrix_name
+	_, jobID, matrixName := parseArtifactName(artifactName)
 
 	for _, suite := range suites.Suites {
 		for _, testcase := range suite.Testcases {
@@ -82,6 +82,7 @@ func extractFailures(suites *junit.Testsuites, artifactName string, runID int64,
 					ArtifactID: artifactName,
 					RunID:      runID,
 					JobID:      jobID,
+					MatrixName: matrixName,
 					Timestamp:  timestamp,
 				}
 				failures = append(failures, failure)
@@ -94,17 +95,19 @@ func extractFailures(suites *junit.Testsuites, artifactName string, runID int64,
 
 // extractAllTestRuns extracts all test runs (including successes) from parsed JUnit data
 // Used for calculating failure rates
-func extractAllTestRuns(suites *junit.Testsuites, runID int64) []TestRun {
+func extractAllTestRuns(suites *junit.Testsuites, runID int64, jobID, matrixName string) []TestRun {
 	var runs []TestRun
 
 	for _, suite := range suites.Suites {
 		for _, testcase := range suite.Testcases {
 			run := TestRun{
-				SuiteName: topLevelTestName(testcase.Name),
-				Name:      testcase.Name,
-				Failed:    testcase.Failure != nil,
-				Skipped:   testcase.Skipped != nil,
-				RunID:     runID,
+				SuiteName:  topLevelTestName(testcase.Name),
+				Name:       testcase.Name,
+				Failed:     testcase.Failure != nil,
+				Skipped:    testcase.Skipped != nil,
+				RunID:      runID,
+				JobID:      jobID,
+				MatrixName: matrixName,
 			}
 			runs = append(runs, run)
 		}
@@ -237,6 +240,25 @@ func convertToReports(grouped map[string][]TestFailure, testRunCounts map[string
 	return reports
 }
 
+// filterParentTests removes top-level test names from grouped when subtests of
+// that parent were observed in testRunCounts. A top-level failure whose subtests
+// ran in other CI jobs is already captured (with a correct denominator) in the
+// Flaky Suites section, so including it in the per-test table produces a
+// misleading 1/1 entry.
+func filterParentTests(grouped map[string][]TestFailure, testRunCounts map[string]int) {
+	suitePrefix := make(map[string]bool, len(testRunCounts))
+	for name := range testRunCounts {
+		if idx := strings.IndexByte(name, '/'); idx >= 0 {
+			suitePrefix[name[:idx]] = true
+		}
+	}
+	for testName := range grouped {
+		if !strings.Contains(testName, "/") && suitePrefix[testName] {
+			delete(grouped, testName)
+		}
+	}
+}
+
 // isFinalRetry returns true if the test name has the "(final)" suffix,
 // indicating the test runner exhausted all retries.
 func isFinalRetry(testName string) bool {
@@ -341,23 +363,35 @@ func identifyCIBreakers(failures []TestFailure) (map[string][]TestFailure, map[s
 	return ciBreakers, ciBreakCount
 }
 
+// suiteRunKey returns a string that uniquely identifies a single (CI run × DB config) pair.
+// Each workflow run may spawn multiple matrix jobs sharing the same RunID but with distinct
+// MatrixNames (DB configs). Keying by (RunID, MatrixName) ensures shards belonging to the
+// same run+config are counted once, regardless of how many shard JobIDs they produce.
+func suiteRunKey(runID int64, matrixName string) string {
+	return fmt.Sprintf("%d:%s", runID, matrixName)
+}
+
 // generateSuiteReports creates per-suite flake breakdown from all failures and test runs.
-// Suite flake rate = % of workflow runs where the suite had at least one non-retry failure.
+// Suite flake rate = % of (CI run × DB config) pairs where the suite had at least one
+// non-retry failure.
 func generateSuiteReports(allFailures []TestFailure, allTestRuns []TestRun) []SuiteReport {
-	// Track unique workflow runs per suite (denominator)
-	suiteRuns := make(map[string]map[int64]bool)
+	// Track unique (CI run × DB config) pairs per suite (denominator).
+	// Using MatrixName avoids the inflation caused by per-shard JobIDs: suites whose
+	// test methods are spread across N shards would otherwise be counted N times per
+	// (run × DB config).
+	suiteRuns := make(map[string]map[string]bool)
 	for _, run := range allTestRuns {
 		if run.Skipped || !isGoTestSuite(run.SuiteName) {
 			continue
 		}
 		if suiteRuns[run.SuiteName] == nil {
-			suiteRuns[run.SuiteName] = make(map[int64]bool)
+			suiteRuns[run.SuiteName] = make(map[string]bool)
 		}
-		suiteRuns[run.SuiteName][run.RunID] = true
+		suiteRuns[run.SuiteName][suiteRunKey(run.RunID, run.MatrixName)] = true
 	}
 
-	// Track workflow runs with non-retry failures per suite (numerator)
-	suiteFailedRuns := make(map[string]map[int64]bool)
+	// Track (CI run × DB config) pairs with non-retry failures per suite (numerator)
+	suiteFailedRuns := make(map[string]map[string]bool)
 	suiteLastFailure := make(map[string]time.Time)
 	for _, failure := range allFailures {
 		if !isGoTestSuite(failure.SuiteName) {
@@ -368,9 +402,9 @@ func generateSuiteReports(allFailures []TestFailure, allTestRuns []TestRun) []Su
 			continue
 		}
 		if suiteFailedRuns[failure.SuiteName] == nil {
-			suiteFailedRuns[failure.SuiteName] = make(map[int64]bool)
+			suiteFailedRuns[failure.SuiteName] = make(map[string]bool)
 		}
-		suiteFailedRuns[failure.SuiteName][failure.RunID] = true
+		suiteFailedRuns[failure.SuiteName][suiteRunKey(failure.RunID, failure.MatrixName)] = true
 		if failure.Timestamp.After(suiteLastFailure[failure.SuiteName]) {
 			suiteLastFailure[failure.SuiteName] = failure.Timestamp
 		}