From 893f9a65a7e6266abfebf776fa941583ae07cffe Mon Sep 17 00:00:00 2001 From: Stephan Behnke Date: Mon, 13 Apr 2026 09:03:50 -0700 Subject: [PATCH 1/4] JUnit summary with failures --- .github/actions/get-job-id/action.yml | 48 ++++-- .github/workflows/run-tests.yml | 42 +++-- Makefile | 4 + tools/testrunner/junit.go | 84 +++++++++- tools/testrunner/junit_test.go | 129 ++++++++++----- tools/testrunner/log.go | 72 +++++++++ tools/testrunner/log_test.go | 103 ++++++++++++ tools/testrunner/summary.go | 117 ++++++++++++++ tools/testrunner/summary_test.go | 148 ++++++++++++++++++ .../testdata/junit-alert-data-race.xml | 14 ++ .../testrunner/testdata/junit-alert-panic.xml | 7 + .../testdata/junit-alerts-output.xml | 10 +- .../testdata/junit-assertion-failure.xml | 12 ++ .../testdata/junit-crash-output.xml | 4 +- tools/testrunner/testdata/junit-empty.xml | 2 +- .../testdata/junit-single-failure.xml | 7 + .../testdata/junit-timeout-output.xml | 6 +- .../junit-unexpected-call-failure.xml | 14 ++ tools/testrunner/testrunner.go | 59 ++++++- tools/testrunner/testrunner_test.go | 76 +++++++-- 20 files changed, 850 insertions(+), 108 deletions(-) create mode 100644 tools/testrunner/summary.go create mode 100644 tools/testrunner/summary_test.go create mode 100644 tools/testrunner/testdata/junit-alert-data-race.xml create mode 100644 tools/testrunner/testdata/junit-alert-panic.xml create mode 100644 tools/testrunner/testdata/junit-assertion-failure.xml create mode 100644 tools/testrunner/testdata/junit-single-failure.xml create mode 100644 tools/testrunner/testdata/junit-unexpected-call-failure.xml diff --git a/.github/actions/get-job-id/action.yml b/.github/actions/get-job-id/action.yml index 123b096fa3d..6605bb53e4d 100644 --- a/.github/actions/get-job-id/action.yml +++ b/.github/actions/get-job-id/action.yml @@ -30,28 +30,44 @@ runs: # The script will first try to match by (name, runner_name), then fall back to name-only. run: | set -euo pipefail + job_url="https://api.github.com/repos/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}/jobs?per_page=100" - json=$(curl -sSL \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer $GITHUB_TOKEN" \ - "$job_url") + max_retries=6 + job_id="" + json="" - # Prefer matching both job name and the current runner name to disambiguate matrix jobs - job_id=$(jq -r --arg name "$JOB_NAME" --arg runner "$RUNNER_NAME" ' - (.jobs // []) - | map(select(.name == $name and (.runner_name // "") == $runner)) - | (.[0].id // empty) - ' <<< "$json" ) + for ((attempt = 1; attempt <= max_retries; attempt++)); do + json=$(curl -sSL \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer $GITHUB_TOKEN" \ + "$job_url") - # Fallback: match by name only - if [ -z "${job_id:-}" ]; then - job_id=$(jq -r --arg name "$JOB_NAME" ' - (.jobs // []) | map(select(.name == $name)) | (.[0].id // empty) + # Prefer matching both job name and the current runner name to disambiguate matrix jobs + job_id=$(jq -r --arg name "$JOB_NAME" --arg runner "$RUNNER_NAME" ' + (.jobs // []) + | map(select(.name == $name and (.runner_name // "") == $runner)) + | (.[0].id // empty) ' <<< "$json" ) - fi + + # Fallback: match by name only + if [ -z "${job_id:-}" ]; then + job_id=$(jq -r --arg name "$JOB_NAME" ' + (.jobs // []) | map(select(.name == $name)) | (.[0].id // empty) + ' <<< "$json" ) + fi + + if [ -n "${job_id:-}" ] && [ "$job_id" != "null" ]; then + break + fi + + if [ "$attempt" -lt "$max_retries" ]; then + echo "::notice::Job ID for '$JOB_NAME' not visible yet (attempt $attempt/$max_retries); retrying in 5s" + sleep 5 + fi + done if [ -z "${job_id:-}" ] || [ "$job_id" = "null" ]; then - echo "::error::Failed to resolve job ID for name '$JOB_NAME' on runner '$RUNNER_NAME' in run '$RUN_ID'" >&2 + echo "::error::Failed to resolve job ID for name '$JOB_NAME' on runner '$RUNNER_NAME' in run '$RUN_ID' after retries" >&2 exit 1 fi echo "job_id=$job_id" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 6422c701f5b..1f56c6e5293 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -328,15 +328,13 @@ jobs: [ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] && CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash - - name: Generate test summary - uses: mikepenz/action-junit-report@v6 + - name: Write test summary if: ${{ !cancelled() }} - with: - report_paths: ./.testoutput/junit.*.xml - detailed_summary: true - check_annotations: false - annotate_only: true - skip_annotations: true + run: | + summary="$(make -s print-test-summary)" + if [ -n "$summary" ]; then + printf '%s\n' "$summary" > "$GITHUB_STEP_SUMMARY" + fi - name: Upload code coverage to Codecov uses: codecov/codecov-action@v5 @@ -428,15 +426,13 @@ jobs: [ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] && CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash - - name: Generate test summary - uses: mikepenz/action-junit-report@v6 + - name: Write test summary if: ${{ !cancelled() }} - with: - report_paths: ./.testoutput/junit.*.xml - detailed_summary: true - check_annotations: false - annotate_only: true - skip_annotations: true + run: | + summary="$(make -s print-test-summary)" + if [ -n "$summary" ]; then + printf '%s\n' "$summary" > "$GITHUB_STEP_SUMMARY" + fi - name: Upload code coverage to Codecov uses: codecov/codecov-action@v5 @@ -567,15 +563,13 @@ jobs: [ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] && CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash - - name: Generate test summary - uses: mikepenz/action-junit-report@v6 + - name: Write test summary if: ${{ !cancelled() }} - with: - report_paths: ./.testoutput/junit.*.xml - detailed_summary: true - check_annotations: false - annotate_only: true - skip_annotations: true + run: | + summary="$(make -s print-test-summary)" + if [ -n "$summary" ]; then + printf '%s\n' "$summary" > "$GITHUB_STEP_SUMMARY" + fi - name: Upload code coverage to Codecov uses: codecov/codecov-action@v5 diff --git a/Makefile b/Makefile index 893c5bf5712..184d577fd11 100644 --- a/Makefile +++ b/Makefile @@ -557,6 +557,10 @@ report-test-crash: $(TEST_OUTPUT_ROOT) --junitfile=$(TEST_OUTPUT_ROOT)/junit.crash.xml \ --crashreportname=$(CRASH_REPORT_NAME) +print-test-summary: $(TEST_OUTPUT_ROOT) + @go run ./cmd/tools/test-runner print-summary \ + --junit-glob=$(TEST_OUTPUT_ROOT)/junit.*.xml + ##### Schema ##### install-schema-cass-es: temporal-cassandra-tool install-schema-es @printf $(COLOR) "Install Cassandra schema..." diff --git a/tools/testrunner/junit.go b/tools/testrunner/junit.go index 446f75c9801..9b2be8a2f68 100644 --- a/tools/testrunner/junit.go +++ b/tools/testrunner/junit.go @@ -13,6 +13,18 @@ import ( "github.com/jstemmer/go-junit-report/v2/junit" ) +// alertsSuiteName is the JUnit suite name used for structural alerts (data +// races, panics, fatal errors). +const alertsSuiteName = "ALERTS" + +const junitAlertDetailsMaxBytes = 64 * 1024 + +const ( + failureKindFailed = "Failed" + failureKindTimeout = "TIMEOUT" + failureKindCrash = "CRASH" +) + type junitReport struct { junit.Testsuites path string @@ -33,12 +45,15 @@ func (j *junitReport) read() error { return nil } +// generateStatic creates synthetic failures for non-JUnit failure modes such as +// timeouts and crashes. Failure.Type is the canonical failure kind for these +// synthetic rows (for example TIMEOUT or CRASH), and Failure.Data is left empty. func generateStatic(names []string, suffix string, message string) *junitReport { var testcases []junit.Testcase for _, name := range names { testcases = append(testcases, junit.Testcase{ Name: fmt.Sprintf("%s (%s)", name, suffix), - Failure: &junit.Result{Message: message}, + Failure: newSyntheticFailure(message, ""), }) } return &junitReport{ @@ -53,6 +68,14 @@ func generateStatic(names []string, suffix string, message string) *junitReport } } +func newSyntheticFailure(kind, data string) *junit.Result { + return &junit.Result{ + Message: kind, + Type: kind, + Data: data, + } +} + func (j *junitReport) write() error { f, err := os.Create(j.path) if err != nil { @@ -77,25 +100,32 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) { if len(alerts) == 0 { return } + + // Convert alerts to JUnit test cases. var cases []junit.Testcase for _, a := range alerts { name := fmt.Sprintf("%s: %s", a.Kind, a.Summary) if p := primaryTestName(a.Tests); p != "" { name = fmt.Sprintf("%s — in %s", name, p) } - // Include only test names for context, not the full log details to avoid XML malformation - var details string + var sb strings.Builder + if a.Details != "" { + sb.WriteString(truncateAlertDetails(sanitizeXML(a.Details))) + sb.WriteByte('\n') + } if len(a.Tests) > 0 { - details = fmt.Sprintf("Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t")) + fmt.Fprintf(&sb, "Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t")) } - r := &junit.Result{Message: string(a.Kind), Data: details} + r := newSyntheticFailure(string(a.Kind), strings.TrimRight(sb.String(), "\n")) cases = append(cases, junit.Testcase{ Name: name, Failure: r, }) } + + // Append the alerts suite to the report. suite := junit.Testsuite{ - Name: "ALERTS", + Name: alertsSuiteName, Failures: len(cases), Tests: len(cases), Testcases: cases, @@ -105,6 +135,30 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) { j.Tests += suite.Tests } +// sanitizeXML removes characters that are invalid in XML 1.0. Go's xml.Encoder +// escapes <, >, & etc., but control characters other than \t, \n, \r are not +// legal XML and cause parsers to reject the document. +func sanitizeXML(s string) string { + return strings.Map(func(r rune) rune { + if r == '\t' || r == '\n' || r == '\r' { + return r + } + if r < 0x20 || r == 0xFFFE || r == 0xFFFF { + return -1 + } + return r + }, s) +} + +// truncateAlertDetails keeps alert payloads from bloating the JUnit artifact. +func truncateAlertDetails(s string) string { + if len(s) <= junitAlertDetailsMaxBytes { + return s + } + const marker = "\n... (truncated) ...\n" + return s[:junitAlertDetailsMaxBytes-len(marker)] + marker +} + // dedupeAlerts removes duplicate alerts (e.g., repeated across retries) based // on kind and details while preserving the first-seen order. func dedupeAlerts(alerts []alert) []alert { @@ -224,6 +278,24 @@ func mergeReports(reports []*junitReport) (*junitReport, error) { // Discard test case parents since they provide no value. continue } + + // Parse failure details from Failure.Data, if present. + if testCase.Failure != nil && testCase.Failure.Data != "" { + if details := parseFailureDetails(testCase.Failure.Data); details != noFailureDetails { + testCase.Failure.Data = details + } + } + + // Failure.Type carries the canonical kind in merged JUnit. + if testCase.Failure != nil { + if suite.Name == alertsSuiteName { + if testCase.Failure.Type == "" { + testCase.Failure.Type = testCase.Failure.Message + } + } else { + testCase.Failure.Type = failureKindFailed + } + } testCase.Name += suffix newSuite.Testcases = append(newSuite.Testcases, testCase) } diff --git a/tools/testrunner/junit_test.go b/tools/testrunner/junit_test.go index be77f41d12a..8384aac3704 100644 --- a/tools/testrunner/junit_test.go +++ b/tools/testrunner/junit_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "slices" + "strings" "testing" "github.com/jstemmer/go-junit-report/v2/junit" @@ -12,8 +13,7 @@ import ( ) func TestReadJUnitReport(t *testing.T) { - j := &junitReport{path: "testdata/junit-attempt-1.xml"} - require.NoError(t, j.read()) + j := mustReadReportFixture(t, "testdata/junit-attempt-1.xml") require.Len(t, j.Suites, 1) require.Equal(t, 2, j.Failures) require.Equal(t, []string{"TestCallbacksSuite/TestWorkflowCallbacks_InvalidArgument"}, j.collectTestCaseFailures()) @@ -28,7 +28,7 @@ func TestGenerateJUnitReportForTimedoutTests(t *testing.T) { "TestCallbacksSuite/TestWorkflowCallbacks_1", "TestCallbacksSuite/TestWorkflowCallbacks_2", } - j := generateStatic(testNames, "timeout", "Timeout") + j := generateStatic(testNames, "timeout", failureKindTimeout) j.path = out.Name() require.NoError(t, j.write()) requireReportEquals(t, "testdata/junit-timeout-output.xml", out.Name()) @@ -58,9 +58,49 @@ func TestNode(t *testing.T) { require.Equal(t, []string{"a", "a/b", "b"}, paths) } +func TestAppendAlertsSuite(t *testing.T) { + j := &junitReport{} + alerts := []alert{ + {Kind: alertKindDataRace, Summary: "Data race detected", Details: "WARNING: DATA RACE\n...", Tests: []string{"go.temporal.io/server/tools/testrunner.TestShowPanic"}}, + {Kind: alertKindPanic, Summary: "This is a panic", Details: "panic: This is a panic\n...", Tests: []string{"TestPanicExample"}}, + } + j.appendAlertsSuite(alerts) + + // Write the report to a temporary file for comparison + out, err := os.CreateTemp("", "junit-alerts-*.xml") + require.NoError(t, err) + defer func() { + require.NoError(t, os.Remove(out.Name())) + }() + + j.path = out.Name() + require.NoError(t, j.write()) + + // Compare against the expected output file + requireReportEquals(t, "testdata/junit-alerts-output.xml", out.Name()) +} + +func TestAppendAlertsSuite_TruncatesLargeDetails(t *testing.T) { + j := &junitReport{} + details := "panic: large panic\n" + strings.Repeat("x", junitAlertDetailsMaxBytes) + "\ntrailing sentinel" + j.appendAlertsSuite([]alert{{ + Kind: alertKindPanic, + Summary: "large panic", + Details: details, + Tests: []string{"TestLargePanic"}, + }}) + + require.Len(t, j.Suites, 1) + require.Len(t, j.Suites[0].Testcases, 1) + got := j.Suites[0].Testcases[0].Failure.Data + require.Contains(t, got, "... (truncated) ...") + require.Contains(t, got, "panic: large panic") + require.NotContains(t, got, "trailing sentinel") + require.LessOrEqual(t, len(got), junitAlertDetailsMaxBytes+len("\nDetected in tests:\n\tTestLargePanic")) +} + func TestMergeReports_SingleReport(t *testing.T) { - j1 := &junitReport{path: "testdata/junit-attempt-1.xml"} - require.NoError(t, j1.read()) + j1 := mustReadReportFixture(t, "testdata/junit-attempt-1.xml") report, err := mergeReports([]*junitReport{j1}) require.NoError(t, err) @@ -73,13 +113,29 @@ func TestMergeReports_SingleReport(t *testing.T) { require.Len(t, testNames, 5) require.NotContains(t, testNames, "TestCallbacksSuite") require.NotContains(t, testNames, "TestCallbacksSuite/TestWorkflowNexusCallbacks_CarriedOver") + var failureData string + for _, tc := range suites[0].Testcases { + if tc.Name == "TestCallbacksSuite/TestWorkflowCallbacks_InvalidArgument" { + require.NotNil(t, tc.Failure) + failureData = tc.Failure.Data + break + } + } + require.NotEmpty(t, failureData) + require.Contains(t, failureData, "Error Trace:") + require.Contains(t, failureData, "expected: 1") + require.NotContains(t, failureData, "=== RUN") + require.NotContains(t, failureData, "--- FAIL:") + for _, tc := range suites[0].Testcases { + if tc.Failure != nil { + require.Equal(t, failureKindFailed, tc.Failure.Type) + } + } } func TestMergeReports_MultipleReports(t *testing.T) { - j1 := &junitReport{path: "testdata/junit-attempt-1.xml"} - require.NoError(t, j1.read()) - j2 := &junitReport{path: "testdata/junit-attempt-2.xml"} - require.NoError(t, j2.read()) + j1 := mustReadReportFixture(t, "testdata/junit-attempt-1.xml") + j2 := mustReadReportFixture(t, "testdata/junit-attempt-2.xml") report, err := mergeReports([]*junitReport{j1, j2}) require.NoError(t, err) @@ -103,8 +159,7 @@ func TestMergeReports_IterationSuffixPreserved(t *testing.T) { // Tests with #XX suffixes (iteration markers) should NOT be discarded as parent tests. // Previously, TestDatanodeSuite/TestLineageFork was incorrectly discarded because // TestDatanodeSuite/TestLineageFork#01 has it as a prefix (without the "/" check). - j := &junitReport{path: "testdata/junit-iteration-suffix.xml"} - require.NoError(t, j.read()) + j := mustReadReportFixture(t, "testdata/junit-iteration-suffix.xml") report, err := mergeReports([]*junitReport{j}) require.NoError(t, err) @@ -126,14 +181,10 @@ func TestMergeReports_IterationSuffixPreserved(t *testing.T) { } func TestMergeReports_MissingRerun(t *testing.T) { - j1 := &junitReport{path: "testdata/junit-attempt-1.xml"} - require.NoError(t, j1.read()) - j2 := &junitReport{path: "testdata/junit-empty.xml"} - require.NoError(t, j2.read()) - j3 := &junitReport{path: "testdata/junit-attempt-2.xml"} - require.NoError(t, j3.read()) - j4 := &junitReport{path: "testdata/junit-empty.xml"} - require.NoError(t, j4.read()) + j1 := mustReadReportFixture(t, "testdata/junit-attempt-1.xml") + j2 := mustReadReportFixture(t, "testdata/junit-empty.xml") + j3 := mustReadReportFixture(t, "testdata/junit-attempt-2.xml") + j4 := mustReadReportFixture(t, "testdata/junit-empty.xml") report, err := mergeReports([]*junitReport{j1, j2, j3, j4}) require.NoError(t, err) @@ -142,26 +193,25 @@ func TestMergeReports_MissingRerun(t *testing.T) { require.Equal(t, errors.New("expected rerun of all failures from previous attempt, missing: [TestCallbacksSuite/TestWorkflowCallbacks_InvalidArgument]"), report.reportingErrs[1]) } -func TestAppendAlertsSuite(t *testing.T) { - j := &junitReport{} - alerts := []alert{ - {Kind: alertKindDataRace, Summary: "Data race detected", Details: "WARNING: DATA RACE\n...", Tests: []string{"go.temporal.io/server/tools/testrunner.TestShowPanic"}}, - {Kind: alertKindPanic, Summary: "This is a panic", Details: "panic: This is a panic\n...", Tests: []string{"TestPanicExample"}}, - } - j.appendAlertsSuite(alerts) +func TestMergeReports_PreservesAlertFailureMessage(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-alert-panic.xml") - // Write the report to a temporary file for comparison - out, err := os.CreateTemp("", "junit-alerts-*.xml") + merged, err := mergeReports([]*junitReport{report}) require.NoError(t, err) - defer func() { - require.NoError(t, os.Remove(out.Name())) - }() + require.Len(t, merged.Suites, 1) + require.Len(t, merged.Suites[0].Testcases, 1) + require.NotNil(t, merged.Suites[0].Testcases[0].Failure) + require.Equal(t, "PANIC", merged.Suites[0].Testcases[0].Failure.Type) +} - j.path = out.Name() - require.NoError(t, j.write()) +func TestMergeReports_PreservesOriginalFailureDataWhenExtractionFindsNothing(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-single-failure.xml") - // Compare against the expected output file - requireReportEquals(t, "testdata/junit-alerts-output.xml", out.Name()) + merged, err := mergeReports([]*junitReport{report}) + require.NoError(t, err) + require.Len(t, merged.Suites, 1) + require.Len(t, merged.Suites[0].Testcases, 1) + require.Equal(t, "plain failure output with no recognizable block", merged.Suites[0].Testcases[0].Failure.Data) } func collectTestNames(suites []junit.Testsuite) []string { @@ -180,7 +230,7 @@ func requireReportEquals(t *testing.T, expectedFile, actualFile string) { actualReport, err := os.ReadFile(actualFile) require.NoError(t, err) - require.Equal(t, string(expectedReport), string(actualReport)) + require.Equal(t, strings.TrimSuffix(string(expectedReport), "\n"), string(actualReport)) } func TestJUnitXMLWellFormed(t *testing.T) { @@ -273,3 +323,10 @@ func TestJUnitXMLWellFormed(t *testing.T) { }) } } + +func mustReadReportFixture(t *testing.T, path string) *junitReport { + t.Helper() + report := &junitReport{path: path} + require.NoError(t, report.read()) + return report +} diff --git a/tools/testrunner/log.go b/tools/testrunner/log.go index 94017ec1c4f..007989378bd 100644 --- a/tools/testrunner/log.go +++ b/tools/testrunner/log.go @@ -9,6 +9,10 @@ import ( "github.com/maruel/panicparse/v2/stack" ) +// noFailureDetails is returned by parseFailureDetails when no recognisable +// failure block is found. +const noFailureDetails = "(error details not found)" + // parseTestTimeouts parses the stdout of a test run and returns the stacktrace and names of tests that timed out. func parseTestTimeouts(stdout string) (stacktrace string, timedoutTests []string) { lines := strings.Split(strings.ReplaceAll(stdout, "\r\n", "\n"), "\n") @@ -351,3 +355,71 @@ func parseFailedTestsFromOutput(stdout string) []string { } return failed } + +// parseFailureDetails extracts the actionable part of a JUnit failure Data block. +func parseFailureDetails(data string) string { + lines := normalizedFailureLines(data) + + if start, end, ok := findTestifyFailureBlock(lines); ok { + return strings.Join(lines[start:end], "\n") + } + if start, end, ok := findLastFailureBlock(lines); ok { + return strings.Join(lines[start:end], "\n") + } + return noFailureDetails +} + +func normalizedFailureLines(data string) []string { + lines := strings.Split(strings.ReplaceAll(data, "\r\n", "\n"), "\n") + for len(lines) > 0 { + t := strings.TrimSpace(lines[len(lines)-1]) + if t != "" && t != "FAIL" { + break + } + lines = lines[:len(lines)-1] + } + return lines +} + +func findTestifyFailureBlock(lines []string) (start, end int, ok bool) { + for i, line := range lines { + if !strings.Contains(line, "Error Trace:") { + continue + } + start = i + if i > 0 { + start = i - 1 + } + return start, endOfFailureBlock(lines, start+1), true + } + return 0, 0, false +} + +func findLastFailureBlock(lines []string) (start, end int, ok bool) { + lastStart := -1 + for i, line := range lines { + if isTestOutputLine(line) { + lastStart = i + } + } + if lastStart < 0 { + return 0, 0, false + } + return lastStart, endOfFailureBlock(lines, lastStart+1), true +} + +func endOfFailureBlock(lines []string, start int) int { + end := start + for end < len(lines) && !isTestOutputLine(lines[end]) && lines[end] != "" { + end++ + } + return end +} + +// isTestOutputLine reports whether line is a Go test-framework output line, +// i.e. " file.go:N: …" — exactly 4 spaces then a non-whitespace character. +// Testify assertion content is indented further (8+ spaces or tabs), so this +// distinguishes log entries from assertion block content. +func isTestOutputLine(line string) bool { + return len(line) > 4 && line[:4] == " " && line[4] != ' ' && line[4] != '\t' +} diff --git a/tools/testrunner/log_test.go b/tools/testrunner/log_test.go index 2cde8bcf5e7..138874f8a73 100644 --- a/tools/testrunner/log_test.go +++ b/tools/testrunner/log_test.go @@ -85,3 +85,106 @@ func TestParseAlerts_DataRaceAndPanic(t *testing.T) { deduped := dedupeAlerts(append(alerts, alerts...)) require.Equal(t, len(deduped), len(alerts)) } + +func TestParseFailureDetails(t *testing.T) { + tests := []struct { + name string + data string + contains []string + notContains []string + exact string + }{ + { + name: "extracts testify block", + data: ` suite_test.go:42: some log line + suite_test.go:43: another log line + suite_test.go:85: + Error Trace: suite_test.go:85 + Error: Not equal: + expected: 1 + actual : 2 +FAIL`, + contains: []string{"Error Trace:", "expected: 1"}, + notContains: []string{"some log line", "FAIL"}, + }, + { + name: "keeps testify file header line", + data: ` suite_test.go:85: + Error Trace: suite_test.go:85 + Error: Not equal: + expected: 1 + actual : 2 +FAIL`, + contains: []string{ + "suite_test.go:85:", + "Error Trace:", + }, + }, + { + name: "uses last gomock style block", + data: `=== RUN TestFoo +=== CONT TestFoo + test_env.go:1: some log + mock_file.go:42: Unexpected call to SomeMethod(...) + expected call at foo_test.go:10 doesn't match argument at index 0. + Got: "actual" + Want: is equal to "expected" +FAIL`, + contains: []string{"Unexpected call", "Got:", "Want:"}, + notContains: []string{"some log"}, + }, + { + name: "uses last failure block when multiple exist", + data: `=== RUN TestFoo + helper.go:1: some setup log + first_mock.go:10: first failure + first continuation + second_mock.go:20: second failure + second continuation +FAIL`, + contains: []string{"second_mock.go:20", "second continuation"}, + notContains: []string{"first_mock.go:10", "first continuation", "some setup log"}, + }, + { + name: "returns sentinel when no output exists", + data: "FAIL\n", + exact: noFailureDetails, + }, + { + name: "stops before logs printed after assertion block", + data: ` suite_test.go:481: + Error Trace: suite_test.go:481 + Error: Not equal: + expected: false + actual : true + Test: TestSuite/TestCase + logger.go:146: some info log after the assertion + controller.go:97: missing call(s) to ... +FAIL`, + contains: []string{"Error Trace:", "expected: false"}, + notContains: []string{"logger.go", "controller.go"}, + }, + { + name: "strips trailing FAIL marker", + data: " foo_test.go:1:\n\tError Trace:\tfoo_test.go:1\n\tError:\toops\n\n\nFAIL\n", + contains: []string{"Error Trace:"}, + notContains: []string{"FAIL"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseFailureDetails(tt.data) + if tt.exact != "" { + require.Equal(t, tt.exact, got) + return + } + for _, want := range tt.contains { + require.Contains(t, got, want) + } + for _, unwanted := range tt.notContains { + require.NotContains(t, got, unwanted) + } + }) + } +} diff --git a/tools/testrunner/summary.go b/tools/testrunner/summary.go new file mode 100644 index 00000000000..3f22df06eea --- /dev/null +++ b/tools/testrunner/summary.go @@ -0,0 +1,117 @@ +package testrunner + +import ( + "fmt" + "html" + "slices" + "strings" +) + +const summaryMaxBytes = 1000 * 1024 +const summaryMaxDetailBytes = 1024 + +type summary struct { + rows []summaryRow +} + +// summaryRow is a single entry in the summary table. +type summaryRow struct { + kind string // e.g. failureKindFailed, DATA RACE, PANIC + name string // test or alert name + details string // failure body +} + +func newSummaryFromReports(reports []*junitReport) summary { + return summary{ + rows: newSummaryRowsFromReports(reports), + } +} + +func newSummaryRowsFromReports(reports []*junitReport) []summaryRow { + var rows []summaryRow + for _, report := range reports { + for _, suite := range report.Suites { + for _, tc := range suite.Testcases { + if tc.Failure == nil { + continue + } + rows = append(rows, summaryRow{ + kind: tc.Failure.Type, + name: tc.Name, + details: tc.Failure.Data, + }) + } + } + } + slices.SortFunc(rows, func(a, b summaryRow) int { + if byName := strings.Compare(a.name, b.name); byName != 0 { + return byName + } + if byKind := strings.Compare(a.kind, b.kind); byKind != 0 { + return byKind + } + return strings.Compare(a.details, b.details) + }) + return rows +} + +// String renders the GitHub step summary HTML and enforces both the total +// summary budget and per-row detail truncation. +func (s summary) String() string { + if len(s.rows) == 0 { + return "" + } + + var sb strings.Builder + sb.WriteString("\n\n") + + // Reserve bytes for the closing tag so we can always finish the table. + const tableClose = "
KindTest
\n" + budget := summaryMaxBytes - sb.Len() - len(tableClose) + + written := 0 + for _, row := range s.rows { + rendered := row.String() + if len(rendered) > budget { + omitted := len(s.rows) - written + fmt.Fprintf(&sb, "… %d failure(s) not shown — see full output in job logs\n", omitted) + break + } + sb.WriteString(rendered) + budget -= len(rendered) + written++ + } + + sb.WriteString(tableClose) + return sb.String() +} + +// String renders one summary table row. +func (row summaryRow) String() string { + details := row.details + truncated := false + if len(details) > summaryMaxDetailBytes { + details = details[:summaryMaxDetailBytes] + truncated = true + } + + kind := row.kind + if strings.Contains(row.name, "(final)") { + kind = "❌ " + kind + } + + var sb strings.Builder + fmt.Fprintf(&sb, "%s", html.EscapeString(kind)) + if details != "" { + escaped := html.EscapeString(details) + if truncated { + escaped += "\n… (truncated — see full output in job logs)" + } + fmt.Fprintf(&sb, "
%s
%s
", + html.EscapeString(row.name), escaped) + } else { + sb.WriteString(html.EscapeString(row.name)) + } + sb.WriteString("\n") + return sb.String() +} diff --git a/tools/testrunner/summary_test.go b/tools/testrunner/summary_test.go new file mode 100644 index 00000000000..55ce37b45e7 --- /dev/null +++ b/tools/testrunner/summary_test.go @@ -0,0 +1,148 @@ +package testrunner + +import ( + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRenderSummaryFromReports_RendersAssertionFailure(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-assertion-failure.xml") + + summary := mustNewMergedSummary(t, report) + require.Equal(t, []summaryRow{{ + kind: failureKindFailed, + name: "TestBar", + details: " bar_test.go:9:\n Error Trace:\t__SOURCE__:2\n Error: \tNot equal:\n \texpected: 1\n \tactual : 2", + }}, summary.rows) + + rendered := summary.String() + require.Contains(t, rendered, "") + require.Contains(t, rendered, "
TestBar") + require.NotContains(t, rendered, "
") + require.Contains(t, rendered, "expected: 1") +} + +func TestRenderSummaryFromReports_EmptyWhenNoFailures(t *testing.T) { + s := newSummaryFromReports([]*junitReport{mustReadReportFixture(t, "testdata/junit-empty.xml")}) + require.Empty(t, s.rows) + require.Empty(t, s.String()) +} + +func TestRenderSummaryFromReports_KeepsFailureAndAlertRows(t *testing.T) { + failureReport := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + failureReport.Suites[0].Testcases[0].Name = "TestFoo" + failureReport.Suites[0].Testcases[0].Failure.Data = "FAIL\n" + + alertReport := mustReadReportFixture(t, "testdata/junit-alert-panic.xml") + + summary := mustNewMergedSummary(t, failureReport, alertReport) + require.Equal(t, []summaryRow{ + {kind: failureKindFailed, name: "TestFoo", details: "FAIL\n"}, + {kind: string(alertKindPanic), name: "panic test (retry 1) (final)", details: "panic: boom"}, + }, summary.rows) + + s := summary.String() + require.Contains(t, s, "panic: boom") + require.Contains(t, s, "❌ PANIC") + require.Contains(t, s, "
FAIL\n
") + require.Contains(t, s, "
TestFoo") + require.Contains(t, s, "
panic test (retry 1) (final)") + require.Equal(t, 2, strings.Count(s, "
Details
")) +} + +func TestRenderSummaryFromReports_RendersTrimmedFailureBody(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-unexpected-call-failure.xml") + + summary := mustNewMergedSummary(t, report) + require.Equal(t, []summaryRow{{ + kind: failureKindFailed, + name: "TestBaz", + details: " mock_file.go:42: Unexpected call to SomeMethod(...)\n expected call at foo_test.go:10 doesn't match argument at index 0.\n Got: \"actual\"\n Want: is equal to \"expected\"", + }}, summary.rows) + + s := summary.String() + require.Contains(t, s, "
TestBaz") + require.Contains(t, s, "Unexpected call to SomeMethod") + require.NotContains(t, s, "some setup log") +} + +func TestRenderSummaryFromReports_RendersAlertRow(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-alert-data-race.xml") + + s := newSummaryFromReports([]*junitReport{report}) + require.Equal(t, []summaryRow{{ + kind: string(alertKindDataRace), + name: "DATA RACE: Data race detected in TestFoo", + details: "WARNING: DATA RACE\nWrite at 0x00c000123456 by goroutine 7:\n main.TestFoo()\n /path/to/foo_test.go:42 +0x123\n\nPrevious read at 0x00c000123456 by goroutine 8:\n main.TestFoo()\n /path/to/foo_test.go:43 +0x456", + }}, s.rows) + rendered := s.String() + require.Contains(t, rendered, string(alertKindDataRace)) + require.Contains(t, rendered, "Write at 0x00c000123456 by goroutine 7") +} + +func TestRenderSummaryFromReports_TruncatesOversizedDetail(t *testing.T) { + report := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + report.Suites[0].Testcases[0].Failure.Data = strings.Repeat("x", summaryMaxDetailBytes+1) + + summary := mustNewMergedSummary(t, report) + require.Equal(t, []summaryRow{{ + kind: failureKindFailed, + name: "TestStandalone", + details: strings.Repeat("x", summaryMaxDetailBytes+1), + }}, summary.rows) + + s := summary.String() + require.Contains(t, s, "truncated") + require.Less(t, len(s), summaryMaxBytes) +} + +func TestRenderSummaryFromReports_OmitsRowsWhenBudgetExceeded(t *testing.T) { + n := summaryMaxBytes/summaryMaxDetailBytes + 1 + reports := make([]*junitReport, n) + for i := range n { + report := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + report.Suites[0].Testcases[0].Name = "Test" + strconv.Itoa(i) + report.Suites[0].Testcases[0].Failure.Data = strings.Repeat("x", summaryMaxDetailBytes) + reports[i] = report + } + summary := newSummaryFromReports(reports) + require.Len(t, summary.rows, n) + + s := summary.String() + require.Less(t, len(s), summaryMaxBytes+len("
\n")+200) + require.Contains(t, s, "not shown") + require.Contains(t, s, "") +} + +func TestRenderSummaryFromReports_MergesMultipleReportsIntoSingleTable(t *testing.T) { + reportOld := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + reportOld.Suites[0].Name = "SuiteA" + reportOld.Suites[0].Testcases[0].Name = "TestOld" + reportOld.Suites[0].Testcases[0].Failure.Data = "old failure" + + reportNew := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + reportNew.Suites[0].Name = "SuiteB" + reportNew.Suites[0].Testcases[0].Name = "TestNew" + reportNew.Suites[0].Testcases[0].Failure.Data = "new failure" + + summary := newSummaryFromReports([]*junitReport{reportOld, reportNew}) + require.Equal(t, []summaryRow{ + {kind: failureKindFailed, name: "TestNew", details: "new failure"}, + {kind: failureKindFailed, name: "TestOld", details: "old failure"}, + }, summary.rows) + + s := summary.String() + require.Equal(t, 1, strings.Count(s, "")) + require.Contains(t, s, "TestNew") + require.Contains(t, s, "TestOld") +} + +func mustNewMergedSummary(t *testing.T, reports ...*junitReport) summary { + t.Helper() + merged, err := mergeReports(reports) + require.NoError(t, err) + return newSummaryFromReports([]*junitReport{merged}) +} diff --git a/tools/testrunner/testdata/junit-alert-data-race.xml b/tools/testrunner/testdata/junit-alert-data-race.xml new file mode 100644 index 00000000000..16808caab44 --- /dev/null +++ b/tools/testrunner/testdata/junit-alert-data-race.xml @@ -0,0 +1,14 @@ + + + + + + + diff --git a/tools/testrunner/testdata/junit-alert-panic.xml b/tools/testrunner/testdata/junit-alert-panic.xml new file mode 100644 index 00000000000..a06f7547f51 --- /dev/null +++ b/tools/testrunner/testdata/junit-alert-panic.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/tools/testrunner/testdata/junit-alerts-output.xml b/tools/testrunner/testdata/junit-alerts-output.xml index 2b06ce4d380..2cb485aa515 100644 --- a/tools/testrunner/testdata/junit-alerts-output.xml +++ b/tools/testrunner/testdata/junit-alerts-output.xml @@ -1,12 +1,16 @@ - - - \ No newline at end of file + diff --git a/tools/testrunner/testdata/junit-assertion-failure.xml b/tools/testrunner/testdata/junit-assertion-failure.xml new file mode 100644 index 00000000000..3b96e89dafa --- /dev/null +++ b/tools/testrunner/testdata/junit-assertion-failure.xml @@ -0,0 +1,12 @@ + + + + + + + diff --git a/tools/testrunner/testdata/junit-crash-output.xml b/tools/testrunner/testdata/junit-crash-output.xml index c35498e3350..109db4bda6e 100644 --- a/tools/testrunner/testdata/junit-crash-output.xml +++ b/tools/testrunner/testdata/junit-crash-output.xml @@ -1,7 +1,7 @@ - + - \ No newline at end of file + diff --git a/tools/testrunner/testdata/junit-empty.xml b/tools/testrunner/testdata/junit-empty.xml index d7cfe74ad85..7131b1d6311 100644 --- a/tools/testrunner/testdata/junit-empty.xml +++ b/tools/testrunner/testdata/junit-empty.xml @@ -1,3 +1,3 @@ - + diff --git a/tools/testrunner/testdata/junit-single-failure.xml b/tools/testrunner/testdata/junit-single-failure.xml new file mode 100644 index 00000000000..57575ed6039 --- /dev/null +++ b/tools/testrunner/testdata/junit-single-failure.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/tools/testrunner/testdata/junit-timeout-output.xml b/tools/testrunner/testdata/junit-timeout-output.xml index 122b447aa20..8b99355a557 100644 --- a/tools/testrunner/testdata/junit-timeout-output.xml +++ b/tools/testrunner/testdata/junit-timeout-output.xml @@ -1,10 +1,10 @@ - + - + - \ No newline at end of file + diff --git a/tools/testrunner/testdata/junit-unexpected-call-failure.xml b/tools/testrunner/testdata/junit-unexpected-call-failure.xml new file mode 100644 index 00000000000..d7a13071d16 --- /dev/null +++ b/tools/testrunner/testdata/junit-unexpected-call-failure.xml @@ -0,0 +1,14 @@ + + + + + + + diff --git a/tools/testrunner/testrunner.go b/tools/testrunner/testrunner.go index f0565d5bc11..651de40ac77 100644 --- a/tools/testrunner/testrunner.go +++ b/tools/testrunner/testrunner.go @@ -23,6 +23,7 @@ const ( maxAttemptsFlag = "--max-attempts=" coverProfileFlag = "-coverprofile=" junitReportFlag = "--junitfile=" + junitGlobFlag = "--junit-glob=" crashReportNameFlag = "--crashreportname=" gotestsumPathFlag = "--gotestsum-path=" @@ -39,6 +40,7 @@ const ( const ( testCommand = "test" crashReportCommand = "report-crash" + summaryCommand = "print-summary" ) type attempt struct { @@ -75,6 +77,7 @@ type runner struct { attempts []*attempt maxAttempts int crashName string + junitGlob string alerts []alert totalTimeout time.Duration // derived from the -timeout go test flag } @@ -129,6 +132,10 @@ func (r *runner) sanitizeAndParseArgs(command string, args []string) ([]string, continue // this is a `testrunner` only arg and not passed through } + if strings.HasPrefix(arg, junitGlobFlag) { + r.junitGlob = strings.Split(arg, "=")[1] + continue + } if strings.HasPrefix(arg, coverProfileFlag) { r.coverProfilePath = strings.Split(arg, "=")[1] } else if strings.HasPrefix(arg, junitReportFlag) { @@ -139,10 +146,6 @@ func (r *runner) sanitizeAndParseArgs(command string, args []string) ([]string, sanitizedArgs = append(sanitizedArgs, arg) } - if r.junitOutputPath == "" { - return nil, fmt.Errorf("missing required argument %q", junitReportFlag) - } - switch command { case testCommand: if r.coverProfilePath == "" { @@ -155,9 +158,16 @@ func (r *runner) sanitizeAndParseArgs(command string, args []string) ([]string, return nil, fmt.Errorf("missing required argument %q", gotestsumPathFlag) } case crashReportCommand: + if r.junitOutputPath == "" { + return nil, fmt.Errorf("missing required argument %q", junitReportFlag) + } if r.crashName == "" { return nil, fmt.Errorf("missing required argument %q", crashReportNameFlag) } + case summaryCommand: + if r.junitGlob == "" { + return nil, fmt.Errorf("missing required argument %q", junitGlobFlag) + } default: return nil, fmt.Errorf("unknown command %q", command) } @@ -218,6 +228,10 @@ func Main() { r.runTests(ctx, args) case crashReportCommand: r.reportCrash() + case summaryCommand: + if err := r.printSummary(); err != nil { + log.Fatal(err) + } default: log.Fatalf("unknown command %q", command) } @@ -225,13 +239,42 @@ func Main() { // nolint:revive,deep-exit func (r *runner) reportCrash() { - jr := generateStatic([]string{r.crashName}, "crash", "Crash") + jr := generateStatic([]string{r.crashName}, "crash", failureKindCrash) jr.path = r.junitOutputPath if err := jr.write(); err != nil { log.Fatal(err) } } +func (r *runner) printSummary() error { + paths, err := filepath.Glob(r.junitGlob) + if err != nil { + return fmt.Errorf("failed to expand junit glob %q: %w", r.junitGlob, err) + } + if len(paths) == 0 { + return nil + } + slices.Sort(paths) + + reports := make([]*junitReport, 0, len(paths)) + for _, path := range paths { + report := &junitReport{path: path} + if err := report.read(); err != nil { + return fmt.Errorf("failed to read junit report %q: %w", path, err) + } + reports = append(reports, report) + } + + content := newSummaryFromReports(reports).String() + if content == "" { + return nil + } + if _, err := os.Stdout.WriteString(content); err != nil { + return fmt.Errorf("failed to write summary to stdout: %w", err) + } + return nil +} + // writeCurrentReport writes the merged report from all completed attempts to the // final output path. It is called after each attempt so that partial results // survive if the process is killed externally between attempts. @@ -278,7 +321,7 @@ func (r *runner) runTests(ctx context.Context, args []string) { // gotestsum didn't finish writing a JUnit XML. Fall back to parsing // stdout for any "--- FAIL:" lines that completed before the kill. if failedTests := parseFailedTestsFromOutput(stdout); len(failedTests) > 0 { - currentAttempt.junitReport = generateStatic(failedTests, "total timeout", "Timeout") + currentAttempt.junitReport = generateStatic(failedTests, "total timeout", failureKindTimeout) } // If no failed tests are found either, the current attempt's report // remains empty and mergeReports will include only prior attempts. @@ -294,7 +337,7 @@ func (r *runner) runTests(ctx context.Context, args []string) { if len(timedoutTests) > 0 { // Run timed out and was aborted. // Update JUnit XML output for timed out tests since none will have been generated. - currentAttempt.junitReport = generateStatic(timedoutTests, "timed out", "Timeout") + currentAttempt.junitReport = generateStatic(timedoutTests, "timed out", failureKindTimeout) log.Print(stacktrace) // Don't retry. @@ -356,8 +399,10 @@ func (r *runner) runTests(ctx context.Context, args []string) { if err = mergedReport.write(); err != nil { log.Fatal(err) } + // Skip the strict rerun-coverage check when the total timeout fired: the // in-progress attempt was killed before it could execute all expected tests. + if len(mergedReport.reportingErrs) > 0 && ctx.Err() == nil { log.Fatal(mergedReport.reportingErrs) } diff --git a/tools/testrunner/testrunner_test.go b/tools/testrunner/testrunner_test.go index 1eccfcb5b44..e94b873666b 100644 --- a/tools/testrunner/testrunner_test.go +++ b/tools/testrunner/testrunner_test.go @@ -1,7 +1,10 @@ package testrunner import ( + "io" "os" + "path/filepath" + "strings" "testing" "time" @@ -140,6 +143,20 @@ func TestRunnerSanitizeAndParseArgs(t *testing.T) { }) require.ErrorContains(t, err, `missing required argument "-coverprofile="`) }) + + t.Run("WriteSummaryRequiresJunitGlob", func(t *testing.T) { + r := newRunner() + _, err := r.sanitizeAndParseArgs(summaryCommand, nil) + require.ErrorContains(t, err, `missing required argument "--junit-glob="`) + }) + + t.Run("ReportCrashRequiresJunitfile", func(t *testing.T) { + r := newRunner() + _, err := r.sanitizeAndParseArgs(crashReportCommand, []string{ + "--crashreportname=my-test", + }) + require.ErrorContains(t, err, `missing required argument "--junitfile="`) + }) } func TestStripRunFromArgs(t *testing.T) { @@ -163,8 +180,7 @@ func TestWriteCurrentReport(t *testing.T) { r.junitOutputPath = out.Name() // Simulate attempt 1 completing with failures. - j1 := &junitReport{path: "testdata/junit-attempt-1.xml"} - require.NoError(t, j1.read()) + j1 := mustReadReportFixture(t, "testdata/junit-attempt-1.xml") a1 := r.newAttempt() a1.junitReport = j1 @@ -178,8 +194,7 @@ func TestWriteCurrentReport(t *testing.T) { // Simulate attempt 2 also completing. The intermediate write should now // contain failures from both attempts, so that if the process is killed // before attempt 3 the file on disk already has the full picture. - j2 := &junitReport{path: "testdata/junit-attempt-2.xml"} - require.NoError(t, j2.read()) + j2 := mustReadReportFixture(t, "testdata/junit-attempt-2.xml") a2 := r.newAttempt() a2.junitReport = j2 @@ -192,17 +207,58 @@ func TestWriteCurrentReport(t *testing.T) { } func TestRunnerReportCrash(t *testing.T) { - out, err := os.CreateTemp("", "junit-report-*.xml") - require.NoError(t, err) - defer func() { _ = os.Remove(out.Name()) }() + dir := t.TempDir() + out := filepath.Join(dir, "junit-report.xml") r := newRunner() - _, err = r.sanitizeAndParseArgs(crashReportCommand, []string{ - "--junitfile=" + out.Name(), + _, err := r.sanitizeAndParseArgs(crashReportCommand, []string{ + "--junitfile=" + out, "--crashreportname=my-test", }) require.NoError(t, err) r.reportCrash() - requireReportEquals(t, "testdata/junit-crash-output.xml", out.Name()) + requireReportEquals(t, "testdata/junit-crash-output.xml", out) +} + +func TestRunnerPrintSummary(t *testing.T) { + dir := t.TempDir() + report1 := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + report1.Suites[0].Name = "SuiteA" + report1.Suites[0].Testcases[0].Name = "TestAlpha" + report1.Suites[0].Testcases[0].Failure.Type = failureKindFailed + report1.Suites[0].Testcases[0].Failure.Data = "alpha failure" + report1.path = filepath.Join(dir, "junit.alpha.xml") + require.NoError(t, report1.write()) + report2 := mustReadReportFixture(t, "testdata/junit-single-failure.xml") + report2.Suites[0].Name = "SuiteB" + report2.Suites[0].Testcases[0].Name = "TestBeta" + report2.Suites[0].Testcases[0].Failure.Type = failureKindFailed + report2.Suites[0].Testcases[0].Failure.Data = "beta failure" + report2.path = filepath.Join(dir, "junit.beta.xml") + require.NoError(t, report2.write()) + + r := newRunner() + _, err := r.sanitizeAndParseArgs(summaryCommand, []string{ + "--junit-glob=" + filepath.Join(dir, "junit.*.xml"), + }) + require.NoError(t, err) + + stdout := os.Stdout + rpipe, wpipe, err := os.Pipe() + require.NoError(t, err) + defer func() { + os.Stdout = stdout + require.NoError(t, rpipe.Close()) + }() + + os.Stdout = wpipe + require.NoError(t, r.printSummary()) + require.NoError(t, wpipe.Close()) + + body, err := io.ReadAll(rpipe) + require.NoError(t, err) + require.Equal(t, 1, strings.Count(string(body), "
")) + require.Contains(t, string(body), "TestAlpha") + require.Contains(t, string(body), "TestBeta") } From 77b18fe140a438913065b04407fad7c8c01049c7 Mon Sep 17 00:00:00 2001 From: Stephan Behnke Date: Thu, 16 Apr 2026 08:36:51 -0700 Subject: [PATCH 2/4] consolidate as failureType --- tools/testrunner/junit.go | 43 +++++++++++++++++------------ tools/testrunner/junit_test.go | 20 +++++++------- tools/testrunner/log.go | 17 +++--------- tools/testrunner/summary.go | 8 +++--- tools/testrunner/summary_test.go | 18 ++++++------ tools/testrunner/testrunner.go | 6 ++-- tools/testrunner/testrunner_test.go | 4 +-- 7 files changed, 57 insertions(+), 59 deletions(-) diff --git a/tools/testrunner/junit.go b/tools/testrunner/junit.go index 9b2be8a2f68..5c9ef4df45e 100644 --- a/tools/testrunner/junit.go +++ b/tools/testrunner/junit.go @@ -19,10 +19,16 @@ const alertsSuiteName = "ALERTS" const junitAlertDetailsMaxBytes = 64 * 1024 +type failureType string + const ( - failureKindFailed = "Failed" - failureKindTimeout = "TIMEOUT" - failureKindCrash = "CRASH" + // failureTypeFailed marks a failed assertion. + failureTypeFailed failureType = "Failed" + failureTypeTimeout failureType = "TIMEOUT" + failureTypeCrash failureType = "CRASH" + failureTypeDataRace failureType = "DATA RACE" + failureTypePanic failureType = "PANIC" + failureTypeFatal failureType = "FATAL" ) type junitReport struct { @@ -45,15 +51,16 @@ func (j *junitReport) read() error { return nil } -// generateStatic creates synthetic failures for non-JUnit failure modes such as -// timeouts and crashes. Failure.Type is the canonical failure kind for these -// synthetic rows (for example TIMEOUT or CRASH), and Failure.Data is left empty. -func generateStatic(names []string, suffix string, message string) *junitReport { +// generateReport builds a JUnit report for failures that the runner +// derives itself, such as timeouts and crashes. Failure.Type stores the +// canonical failure type (for example TIMEOUT or CRASH), and Failure.Data is +// intentionally left empty. +func generateReport(names []string, suffix string, kind failureType) *junitReport { var testcases []junit.Testcase for _, name := range names { testcases = append(testcases, junit.Testcase{ Name: fmt.Sprintf("%s (%s)", name, suffix), - Failure: newSyntheticFailure(message, ""), + Failure: generateFailure(kind, ""), }) } return &junitReport{ @@ -68,10 +75,10 @@ func generateStatic(names []string, suffix string, message string) *junitReport } } -func newSyntheticFailure(kind, data string) *junit.Result { +func generateFailure(kind failureType, data string) *junit.Result { return &junit.Result{ - Message: kind, - Type: kind, + Message: string(kind), + Type: string(kind), Data: data, } } @@ -95,7 +102,7 @@ func (j *junitReport) write() error { // appendAlertsSuite adds a synthetic JUnit suite summarizing high-priority alerts // (data races, panics, fatals) so that CI surfaces them prominently. func (j *junitReport) appendAlertsSuite(alerts []alert) { - // Deduplicate by kind+details to avoid noisy repeats across retries. + // Deduplicate by type+details to avoid noisy repeats across retries. alerts = dedupeAlerts(alerts) if len(alerts) == 0 { return @@ -104,7 +111,7 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) { // Convert alerts to JUnit test cases. var cases []junit.Testcase for _, a := range alerts { - name := fmt.Sprintf("%s: %s", a.Kind, a.Summary) + name := fmt.Sprintf("%s: %s", a.Type, a.Summary) if p := primaryTestName(a.Tests); p != "" { name = fmt.Sprintf("%s — in %s", name, p) } @@ -116,10 +123,10 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) { if len(a.Tests) > 0 { fmt.Fprintf(&sb, "Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t")) } - r := newSyntheticFailure(string(a.Kind), strings.TrimRight(sb.String(), "\n")) + f := generateFailure(a.Type, strings.TrimRight(sb.String(), "\n")) cases = append(cases, junit.Testcase{ Name: name, - Failure: r, + Failure: f, }) } @@ -160,12 +167,12 @@ func truncateAlertDetails(s string) string { } // dedupeAlerts removes duplicate alerts (e.g., repeated across retries) based -// on kind and details while preserving the first-seen order. +// on type and details while preserving the first-seen order. func dedupeAlerts(alerts []alert) []alert { seen := make(map[string]struct{}, len(alerts)) var out []alert for _, a := range alerts { - key := string(a.Kind) + "\n" + a.Details + key := string(a.Type) + "\n" + a.Details if _, ok := seen[key]; ok { continue } @@ -293,7 +300,7 @@ func mergeReports(reports []*junitReport) (*junitReport, error) { testCase.Failure.Type = testCase.Failure.Message } } else { - testCase.Failure.Type = failureKindFailed + testCase.Failure.Type = string(failureTypeFailed) } } testCase.Name += suffix diff --git a/tools/testrunner/junit_test.go b/tools/testrunner/junit_test.go index 8384aac3704..48f5fd2a42a 100644 --- a/tools/testrunner/junit_test.go +++ b/tools/testrunner/junit_test.go @@ -28,7 +28,7 @@ func TestGenerateJUnitReportForTimedoutTests(t *testing.T) { "TestCallbacksSuite/TestWorkflowCallbacks_1", "TestCallbacksSuite/TestWorkflowCallbacks_2", } - j := generateStatic(testNames, "timeout", failureKindTimeout) + j := generateReport(testNames, "timeout", failureTypeTimeout) j.path = out.Name() require.NoError(t, j.write()) requireReportEquals(t, "testdata/junit-timeout-output.xml", out.Name()) @@ -61,8 +61,8 @@ func TestNode(t *testing.T) { func TestAppendAlertsSuite(t *testing.T) { j := &junitReport{} alerts := []alert{ - {Kind: alertKindDataRace, Summary: "Data race detected", Details: "WARNING: DATA RACE\n...", Tests: []string{"go.temporal.io/server/tools/testrunner.TestShowPanic"}}, - {Kind: alertKindPanic, Summary: "This is a panic", Details: "panic: This is a panic\n...", Tests: []string{"TestPanicExample"}}, + {Type: failureTypeDataRace, Summary: "Data race detected", Details: "WARNING: DATA RACE\n...", Tests: []string{"go.temporal.io/server/tools/testrunner.TestShowPanic"}}, + {Type: failureTypePanic, Summary: "This is a panic", Details: "panic: This is a panic\n...", Tests: []string{"TestPanicExample"}}, } j.appendAlertsSuite(alerts) @@ -84,7 +84,7 @@ func TestAppendAlertsSuite_TruncatesLargeDetails(t *testing.T) { j := &junitReport{} details := "panic: large panic\n" + strings.Repeat("x", junitAlertDetailsMaxBytes) + "\ntrailing sentinel" j.appendAlertsSuite([]alert{{ - Kind: alertKindPanic, + Type: failureTypePanic, Summary: "large panic", Details: details, Tests: []string{"TestLargePanic"}, @@ -128,7 +128,7 @@ func TestMergeReports_SingleReport(t *testing.T) { require.NotContains(t, failureData, "--- FAIL:") for _, tc := range suites[0].Testcases { if tc.Failure != nil { - require.Equal(t, failureKindFailed, tc.Failure.Type) + require.Equal(t, string(failureTypeFailed), tc.Failure.Type) } } } @@ -201,7 +201,7 @@ func TestMergeReports_PreservesAlertFailureMessage(t *testing.T) { require.Len(t, merged.Suites, 1) require.Len(t, merged.Suites[0].Testcases, 1) require.NotNil(t, merged.Suites[0].Testcases[0].Failure) - require.Equal(t, "PANIC", merged.Suites[0].Testcases[0].Failure.Type) + require.Equal(t, string(failureTypePanic), merged.Suites[0].Testcases[0].Failure.Type) } func TestMergeReports_PreservesOriginalFailureDataWhenExtractionFindsNothing(t *testing.T) { @@ -242,7 +242,7 @@ func TestJUnitXMLWellFormed(t *testing.T) { { name: "basic_report", setup: func() *junitReport { - return generateStatic([]string{"TestBasic"}, "test", "Test failed") + return generateReport([]string{"TestBasic"}, "test", "Test failed") }, }, { @@ -251,7 +251,7 @@ func TestJUnitXMLWellFormed(t *testing.T) { j := &junitReport{} alerts := []alert{ { - Kind: alertKindPanic, + Type: failureTypePanic, Summary: "runtime error: invalid memory address or nil pointer dereference", Details: "panic: runtime error: invalid memory address or nil pointer dereference\n[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x123456]\n\ngoroutine 1 [running]:\nmain.TestPanic()\n\t/path/to/test.go:123 +0x456", Tests: []string{ @@ -261,7 +261,7 @@ func TestJUnitXMLWellFormed(t *testing.T) { }, }, { - Kind: alertKindDataRace, + Type: failureTypeDataRace, Summary: "Data race detected", Details: "WARNING: DATA RACE\nWrite at 0x00c000123456 by goroutine 7:\n runtime.racewrite()\n /usr/local/go/src/runtime/race_amd64.s:269 +0x21\n main.TestDataRace()\n /path/to/test.go:456 +0x789\n\nPrevious read at 0x00c000123456 by goroutine 8:\n runtime.raceread()\n /usr/local/go/src/runtime/race_amd64.s:260 +0x21\n main.TestDataRace()\n /path/to/test.go:789 +0xabc", Tests: []string{"TestDataRace"}, @@ -277,7 +277,7 @@ func TestJUnitXMLWellFormed(t *testing.T) { j := &junitReport{} alerts := []alert{ { - Kind: alertKindFatal, + Type: failureTypeFatal, Summary: "Fatal error with special chars: <>&\"'", Details: "fatal error: unexpected signal during runtime execution\n[signal SIGABRT: abort]\n\nStack trace:\n & \"quoted\" 'string'\n\t/path/to/file.go:123", Tests: []string{"TestSpecialChars"}, diff --git a/tools/testrunner/log.go b/tools/testrunner/log.go index 007989378bd..c5d780cd35b 100644 --- a/tools/testrunner/log.go +++ b/tools/testrunner/log.go @@ -70,18 +70,9 @@ func testOnlyStacktrace(stacktrace string) string { return res } -// alertKind represents a category of high-priority alert detected in test output. -type alertKind string - -const ( - alertKindDataRace alertKind = "DATA RACE" - alertKindPanic alertKind = "PANIC" - alertKindFatal alertKind = "FATAL" -) - // alert captures a prominent issue detected from stdout/stderr of test runs. type alert struct { - Kind alertKind + Type failureType Summary string Details string Tests []string @@ -263,7 +254,7 @@ func tryParseDataRace(lines []string, i int, line string) (alert, int, bool) { return false }) return alert{ - Kind: alertKindDataRace, + Type: failureTypeDataRace, Summary: "Data race detected", Details: block, Tests: extractTestNames(block), @@ -277,7 +268,7 @@ func tryParsePanic(lines []string, i int, line string) (alert, int, bool) { } block, end := collectBlock(lines, i, shouldStopOnTestBoundary) return alert{ - Kind: alertKindPanic, + Type: failureTypePanic, Summary: strings.TrimSpace(strings.TrimPrefix(line, "panic: ")), Details: block, Tests: extractTestNames(block), @@ -291,7 +282,7 @@ func tryParseFatal(lines []string, i int, line string) (alert, int, bool) { } block, end := collectBlock(lines, i, shouldStopOnTestBoundary) return alert{ - Kind: alertKindFatal, + Type: failureTypeFatal, Summary: strings.TrimSpace(strings.TrimPrefix(line, "fatal error: ")), Details: block, Tests: extractTestNames(block), diff --git a/tools/testrunner/summary.go b/tools/testrunner/summary.go index 3f22df06eea..cf1092c9bca 100644 --- a/tools/testrunner/summary.go +++ b/tools/testrunner/summary.go @@ -16,7 +16,7 @@ type summary struct { // summaryRow is a single entry in the summary table. type summaryRow struct { - kind string // e.g. failureKindFailed, DATA RACE, PANIC + kind failureType name string // test or alert name details string // failure body } @@ -36,7 +36,7 @@ func newSummaryRowsFromReports(reports []*junitReport) []summaryRow { continue } rows = append(rows, summaryRow{ - kind: tc.Failure.Type, + kind: failureType(tc.Failure.Type), name: tc.Name, details: tc.Failure.Data, }) @@ -47,7 +47,7 @@ func newSummaryRowsFromReports(reports []*junitReport) []summaryRow { if byName := strings.Compare(a.name, b.name); byName != 0 { return byName } - if byKind := strings.Compare(a.kind, b.kind); byKind != 0 { + if byKind := strings.Compare(string(a.kind), string(b.kind)); byKind != 0 { return byKind } return strings.Compare(a.details, b.details) @@ -95,7 +95,7 @@ func (row summaryRow) String() string { truncated = true } - kind := row.kind + kind := string(row.kind) if strings.Contains(row.name, "(final)") { kind = "❌ " + kind } diff --git a/tools/testrunner/summary_test.go b/tools/testrunner/summary_test.go index 55ce37b45e7..7e700afa910 100644 --- a/tools/testrunner/summary_test.go +++ b/tools/testrunner/summary_test.go @@ -13,7 +13,7 @@ func TestRenderSummaryFromReports_RendersAssertionFailure(t *testing.T) { summary := mustNewMergedSummary(t, report) require.Equal(t, []summaryRow{{ - kind: failureKindFailed, + kind: failureTypeFailed, name: "TestBar", details: " bar_test.go:9:\n Error Trace:\t__SOURCE__:2\n Error: \tNot equal:\n \texpected: 1\n \tactual : 2", }}, summary.rows) @@ -40,8 +40,8 @@ func TestRenderSummaryFromReports_KeepsFailureAndAlertRows(t *testing.T) { summary := mustNewMergedSummary(t, failureReport, alertReport) require.Equal(t, []summaryRow{ - {kind: failureKindFailed, name: "TestFoo", details: "FAIL\n"}, - {kind: string(alertKindPanic), name: "panic test (retry 1) (final)", details: "panic: boom"}, + {kind: failureTypeFailed, name: "TestFoo", details: "FAIL\n"}, + {kind: failureTypePanic, name: "panic test (retry 1) (final)", details: "panic: boom"}, }, summary.rows) s := summary.String() @@ -58,7 +58,7 @@ func TestRenderSummaryFromReports_RendersTrimmedFailureBody(t *testing.T) { summary := mustNewMergedSummary(t, report) require.Equal(t, []summaryRow{{ - kind: failureKindFailed, + kind: failureTypeFailed, name: "TestBaz", details: " mock_file.go:42: Unexpected call to SomeMethod(...)\n expected call at foo_test.go:10 doesn't match argument at index 0.\n Got: \"actual\"\n Want: is equal to \"expected\"", }}, summary.rows) @@ -74,12 +74,12 @@ func TestRenderSummaryFromReports_RendersAlertRow(t *testing.T) { s := newSummaryFromReports([]*junitReport{report}) require.Equal(t, []summaryRow{{ - kind: string(alertKindDataRace), + kind: failureTypeDataRace, name: "DATA RACE: Data race detected in TestFoo", details: "WARNING: DATA RACE\nWrite at 0x00c000123456 by goroutine 7:\n main.TestFoo()\n /path/to/foo_test.go:42 +0x123\n\nPrevious read at 0x00c000123456 by goroutine 8:\n main.TestFoo()\n /path/to/foo_test.go:43 +0x456", }}, s.rows) rendered := s.String() - require.Contains(t, rendered, string(alertKindDataRace)) + require.Contains(t, rendered, failureTypeDataRace) require.Contains(t, rendered, "Write at 0x00c000123456 by goroutine 7") } @@ -89,7 +89,7 @@ func TestRenderSummaryFromReports_TruncatesOversizedDetail(t *testing.T) { summary := mustNewMergedSummary(t, report) require.Equal(t, []summaryRow{{ - kind: failureKindFailed, + kind: failureTypeFailed, name: "TestStandalone", details: strings.Repeat("x", summaryMaxDetailBytes+1), }}, summary.rows) @@ -130,8 +130,8 @@ func TestRenderSummaryFromReports_MergesMultipleReportsIntoSingleTable(t *testin summary := newSummaryFromReports([]*junitReport{reportOld, reportNew}) require.Equal(t, []summaryRow{ - {kind: failureKindFailed, name: "TestNew", details: "new failure"}, - {kind: failureKindFailed, name: "TestOld", details: "old failure"}, + {kind: failureTypeFailed, name: "TestNew", details: "new failure"}, + {kind: failureTypeFailed, name: "TestOld", details: "old failure"}, }, summary.rows) s := summary.String() diff --git a/tools/testrunner/testrunner.go b/tools/testrunner/testrunner.go index 651de40ac77..cd8af2372e0 100644 --- a/tools/testrunner/testrunner.go +++ b/tools/testrunner/testrunner.go @@ -239,7 +239,7 @@ func Main() { // nolint:revive,deep-exit func (r *runner) reportCrash() { - jr := generateStatic([]string{r.crashName}, "crash", failureKindCrash) + jr := generateReport([]string{r.crashName}, "crash", failureTypeCrash) jr.path = r.junitOutputPath if err := jr.write(); err != nil { log.Fatal(err) @@ -321,7 +321,7 @@ func (r *runner) runTests(ctx context.Context, args []string) { // gotestsum didn't finish writing a JUnit XML. Fall back to parsing // stdout for any "--- FAIL:" lines that completed before the kill. if failedTests := parseFailedTestsFromOutput(stdout); len(failedTests) > 0 { - currentAttempt.junitReport = generateStatic(failedTests, "total timeout", failureKindTimeout) + currentAttempt.junitReport = generateReport(failedTests, "total timeout", failureTypeTimeout) } // If no failed tests are found either, the current attempt's report // remains empty and mergeReports will include only prior attempts. @@ -337,7 +337,7 @@ func (r *runner) runTests(ctx context.Context, args []string) { if len(timedoutTests) > 0 { // Run timed out and was aborted. // Update JUnit XML output for timed out tests since none will have been generated. - currentAttempt.junitReport = generateStatic(timedoutTests, "timed out", failureKindTimeout) + currentAttempt.junitReport = generateReport(timedoutTests, "timed out", failureTypeTimeout) log.Print(stacktrace) // Don't retry. diff --git a/tools/testrunner/testrunner_test.go b/tools/testrunner/testrunner_test.go index e94b873666b..148e27558d9 100644 --- a/tools/testrunner/testrunner_test.go +++ b/tools/testrunner/testrunner_test.go @@ -226,14 +226,14 @@ func TestRunnerPrintSummary(t *testing.T) { report1 := mustReadReportFixture(t, "testdata/junit-single-failure.xml") report1.Suites[0].Name = "SuiteA" report1.Suites[0].Testcases[0].Name = "TestAlpha" - report1.Suites[0].Testcases[0].Failure.Type = failureKindFailed + report1.Suites[0].Testcases[0].Failure.Type = string(failureTypeFailed) report1.Suites[0].Testcases[0].Failure.Data = "alpha failure" report1.path = filepath.Join(dir, "junit.alpha.xml") require.NoError(t, report1.write()) report2 := mustReadReportFixture(t, "testdata/junit-single-failure.xml") report2.Suites[0].Name = "SuiteB" report2.Suites[0].Testcases[0].Name = "TestBeta" - report2.Suites[0].Testcases[0].Failure.Type = failureKindFailed + report2.Suites[0].Testcases[0].Failure.Type = string(failureTypeFailed) report2.Suites[0].Testcases[0].Failure.Data = "beta failure" report2.path = filepath.Join(dir, "junit.beta.xml") require.NoError(t, report2.write()) From ff416005bc51493a414b6aeaa910ab161023ba51 Mon Sep 17 00:00:00 2001 From: Stephan Behnke Date: Thu, 16 Apr 2026 08:42:49 -0700 Subject: [PATCH 3/4] rewrite sanitizeXML --- tools/testrunner/junit.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/testrunner/junit.go b/tools/testrunner/junit.go index 5c9ef4df45e..fa8be4b138b 100644 --- a/tools/testrunner/junit.go +++ b/tools/testrunner/junit.go @@ -147,10 +147,16 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) { // legal XML and cause parsers to reject the document. func sanitizeXML(s string) string { return strings.Map(func(r rune) rune { - if r == '\t' || r == '\n' || r == '\r' { + switch r { + case '\t', '\n', '\r': return r + case 0xFFFE: + return -1 // Reserved Unicode noncharacter; disallowed in XML 1.0. + case 0xFFFF: + return -1 // Reserved Unicode noncharacter; disallowed in XML 1.0. } - if r < 0x20 || r == 0xFFFE || r == 0xFFFF { + if r < 0x20 { + // 0x20 is space; lower code points are ASCII control characters. return -1 } return r From 54e41da70ee2e48444c7cca64d999f779e959fe7 Mon Sep 17 00:00:00 2001 From: Stephan Behnke Date: Thu, 16 Apr 2026 08:51:12 -0700 Subject: [PATCH 4/4] group this, too --- tools/testrunner/junit.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/testrunner/junit.go b/tools/testrunner/junit.go index fa8be4b138b..0c96dfe4756 100644 --- a/tools/testrunner/junit.go +++ b/tools/testrunner/junit.go @@ -150,10 +150,8 @@ func sanitizeXML(s string) string { switch r { case '\t', '\n', '\r': return r - case 0xFFFE: - return -1 // Reserved Unicode noncharacter; disallowed in XML 1.0. - case 0xFFFF: - return -1 // Reserved Unicode noncharacter; disallowed in XML 1.0. + case 0xFFFE, 0xFFFF: + return -1 // Reserved Unicode noncharacters; disallowed in XML 1.0. } if r < 0x20 { // 0x20 is space; lower code points are ASCII control characters.