Skip to content

Commit a3de11e

Browse files
stephanosstephanos
committed
JUnit summary with failures
1 parent 956f6ec commit a3de11e

7 files changed

Lines changed: 427 additions & 56 deletions

File tree

‎.github/workflows/run-tests.yml‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -328,16 +328,6 @@ jobs:
328328
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
329329
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
330330
331-
- name: Generate test summary
332-
uses: mikepenz/action-junit-report@v6
333-
if: ${{ !cancelled() }}
334-
with:
335-
report_paths: ./.testoutput/junit.*.xml
336-
detailed_summary: true
337-
check_annotations: false
338-
annotate_only: true
339-
skip_annotations: true
340-
341331
- name: Upload code coverage to Codecov
342332
uses: codecov/codecov-action@v5
343333
with:
@@ -428,16 +418,6 @@ jobs:
428418
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
429419
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
430420
431-
- name: Generate test summary
432-
uses: mikepenz/action-junit-report@v6
433-
if: ${{ !cancelled() }}
434-
with:
435-
report_paths: ./.testoutput/junit.*.xml
436-
detailed_summary: true
437-
check_annotations: false
438-
annotate_only: true
439-
skip_annotations: true
440-
441421
- name: Upload code coverage to Codecov
442422
uses: codecov/codecov-action@v5
443423
with:
@@ -567,16 +547,6 @@ jobs:
567547
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
568548
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
569549
570-
- name: Generate test summary
571-
uses: mikepenz/action-junit-report@v6
572-
if: ${{ !cancelled() }}
573-
with:
574-
report_paths: ./.testoutput/junit.*.xml
575-
detailed_summary: true
576-
check_annotations: false
577-
annotate_only: true
578-
skip_annotations: true
579-
580550
- name: Upload code coverage to Codecov
581551
uses: codecov/codecov-action@v5
582552
with:

‎tools/testrunner/junit.go‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ func (j *junitReport) write() error {
6969
return nil
7070
}
7171

72-
// appendAlertsSuite adds a synthetic JUnit suite summarizing high-priority alerts
73-
// (data races, panics, fatals) so that CI surfaces them prominently.
72+
// appendAlertsSuite adds a synthetic JUnit suite containing all alerts
73+
// (data races, panics, fatals, assertion failures) so that CI surfaces them
74+
// prominently with full details.
7475
func (j *junitReport) appendAlertsSuite(alerts []alert) {
7576
// Deduplicate by kind+details to avoid noisy repeats across retries.
7677
alerts = dedupeAlerts(alerts)
@@ -80,15 +81,22 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) {
8081
var cases []junit.Testcase
8182
for _, a := range alerts {
8283
name := fmt.Sprintf("%s: %s", a.Kind, a.Summary)
83-
if p := primaryTestName(a.Tests); p != "" {
84-
name = fmt.Sprintf("%s — in %s", name, p)
84+
// For assertion failures the Summary is already the test name, so the
85+
// "— in TestName" suffix would be redundant.
86+
if a.Kind != alertKindAssertionFailure {
87+
if p := primaryTestName(a.Tests); p != "" {
88+
name = fmt.Sprintf("%s — in %s", name, p)
89+
}
90+
}
91+
var sb strings.Builder
92+
if a.Details != "" {
93+
sb.WriteString(sanitizeXML(a.Details))
94+
sb.WriteByte('\n')
8595
}
86-
// Include only test names for context, not the full log details to avoid XML malformation
87-
var details string
88-
if len(a.Tests) > 0 {
89-
details = fmt.Sprintf("Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t"))
96+
if a.Kind != alertKindAssertionFailure && len(a.Tests) > 0 {
97+
fmt.Fprintf(&sb, "Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t"))
9098
}
91-
r := &junit.Result{Message: string(a.Kind), Data: details}
99+
r := &junit.Result{Message: string(a.Kind), Data: strings.TrimRight(sb.String(), "\n")}
92100
cases = append(cases, junit.Testcase{
93101
Name: name,
94102
Failure: r,
@@ -105,6 +113,21 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) {
105113
j.Tests += suite.Tests
106114
}
107115

116+
// sanitizeXML removes characters that are invalid in XML 1.0. Go's xml.Encoder
117+
// escapes <, >, & etc., but control characters other than \t, \n, \r are not
118+
// legal XML and cause parsers to reject the document.
119+
func sanitizeXML(s string) string {
120+
return strings.Map(func(r rune) rune {
121+
if r == '\t' || r == '\n' || r == '\r' {
122+
return r
123+
}
124+
if r < 0x20 || r == 0xFFFE || r == 0xFFFF {
125+
return -1
126+
}
127+
return r
128+
}, s)
129+
}
130+
108131
// dedupeAlerts removes duplicate alerts (e.g., repeated across retries) based
109132
// on kind and details while preserving the first-seen order.
110133
func dedupeAlerts(alerts []alert) []alert {

‎tools/testrunner/log.go‎

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ func testOnlyStacktrace(stacktrace string) string {
7070
type alertKind string
7171

7272
const (
73-
alertKindDataRace alertKind = "DATA RACE"
74-
alertKindPanic alertKind = "PANIC"
75-
alertKindFatal alertKind = "FATAL"
73+
alertKindDataRace alertKind = "DATA RACE"
74+
alertKindPanic alertKind = "PANIC"
75+
alertKindFatal alertKind = "FATAL"
76+
alertKindAssertionFailure alertKind = "ASSERTION FAILURE"
7677
)
7778

7879
// alert captures a prominent issue detected from stdout/stderr of test runs.
@@ -118,9 +119,9 @@ func preferFullyQualifiedTestName(tests []string) string {
118119
return primary
119120
}
120121

121-
// parseAlerts scans a gotestsum/go test stdout stream and extracts high-priority
122-
// alerts such as data races and panics. It returns a slice of alerts in the
123-
// order they were encountered.
122+
// parseAlerts scans a gotestsum/go test stdout stream and extracts structural
123+
// alerts: data races, panics, and fatal errors. Each parser advances the line
124+
// cursor past its block so that lines inside one alert are not double-counted.
124125
func parseAlerts(stdout string) []alert {
125126
lines := strings.Split(strings.ReplaceAll(stdout, "\r\n", "\n"), "\n")
126127
var alerts []alert
@@ -148,6 +149,24 @@ func parseAlerts(stdout string) []alert {
148149
return alerts
149150
}
150151

152+
// parseAssertionFailures scans independently for "--- FAIL: TestName" entries
153+
// and collects the output block that follows each one. Because this runs as a
154+
// separate pass from parseAlerts, a test that panics appears in both the PANIC
155+
// section (via parseAlerts) and the ASSERTION FAILURE section here.
156+
func parseAssertionFailures(stdout string) []alert {
157+
lines := strings.Split(strings.ReplaceAll(stdout, "\r\n", "\n"), "\n")
158+
var alerts []alert
159+
160+
for i := 0; i < len(lines); i++ {
161+
if a, next, ok := tryParseAssertionFailure(lines, i, lines[i]); ok {
162+
alerts = append(alerts, a)
163+
i = next
164+
}
165+
}
166+
167+
return alerts
168+
}
169+
151170
// extractTestNames tries to identify Go test function names from a log block.
152171
// It looks for fully-qualified names like pkg.TestXxx(...) and '--- FAIL: TestXxx' lines.
153172
func extractTestNames(block string) []string {
@@ -334,6 +353,39 @@ func shouldStopOnTestBoundary(line string, _ int, _ int) bool {
334353
return isTestResultBoundary(line)
335354
}
336355

356+
// tryParseAssertionFailure parses a "--- FAIL: TestName" entry at position i.
357+
// Summary is the test name (stable across retries); Details holds the body
358+
// lines that follow, stopping at the next test marker. The header line itself
359+
// ("--- FAIL: TestName (Xs)") is omitted from Details so that the dedup key
360+
// (kind+details) is not affected by timing variance.
361+
func tryParseAssertionFailure(lines []string, i int, line string) (alert, int, bool) {
362+
if !strings.HasPrefix(line, "--- FAIL: ") {
363+
return alert{}, i, false
364+
}
365+
name, _ := parseTripleDashTestName(line)
366+
var block strings.Builder
367+
end := i
368+
for j := i + 1; j < len(lines); j++ {
369+
l := lines[j]
370+
if strings.HasPrefix(l, "--- ") || strings.HasPrefix(l, "=== ") || isTestResultBoundary(l) {
371+
break
372+
}
373+
block.WriteString(l)
374+
block.WriteByte('\n')
375+
end = j
376+
}
377+
details := strings.TrimRight(block.String(), "\n")
378+
if details == "" {
379+
return alert{}, i, false
380+
}
381+
return alert{
382+
Kind: alertKindAssertionFailure,
383+
Summary: name,
384+
Details: details,
385+
Tests: []string{name},
386+
}, end, true
387+
}
388+
337389
// parseFailedTestsFromOutput extracts failing test names from gotestsum stdout.
338390
// It looks for "--- FAIL: TestName" lines produced as tests complete, and is
339391
// used when the test binary was killed externally before producing a JUnit XML.

‎tools/testrunner/log_test.go‎

Lines changed: 106 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,118 @@ ok go.temporal.io/server/tests
6666
})
6767
}
6868

69+
func alertsOfKind(alerts []alert, kind alertKind) []alert {
70+
var out []alert
71+
for _, a := range alerts {
72+
if a.Kind == kind {
73+
out = append(out, a)
74+
}
75+
}
76+
return out
77+
}
78+
79+
func TestParseAssertionFailures(t *testing.T) {
80+
t.Run("SingleFailure", func(t *testing.T) {
81+
stdout := `=== RUN TestFoo
82+
--- FAIL: TestFoo (0.01s)
83+
foo_test.go:42: Error Trace: foo_test.go:42
84+
foo_test.go:42: Error: Not equal
85+
foo_test.go:42: expected: 1
86+
foo_test.go:42: actual : 2
87+
FAIL
88+
`
89+
failures := parseAssertionFailures(stdout)
90+
require.Len(t, failures, 1)
91+
require.Equal(t, "TestFoo", failures[0].Summary)
92+
// Details holds the body without the timing-variant header line.
93+
require.NotContains(t, failures[0].Details, "--- FAIL:")
94+
require.Contains(t, failures[0].Details, "Not equal")
95+
})
96+
97+
t.Run("MultipleFailures", func(t *testing.T) {
98+
stdout := `--- FAIL: TestFoo (0.01s)
99+
foo_test.go:10: first failure
100+
--- FAIL: TestBar (0.02s)
101+
bar_test.go:20: second failure
102+
FAIL
103+
`
104+
failures := parseAssertionFailures(stdout)
105+
require.Len(t, failures, 2)
106+
require.Equal(t, "TestFoo", failures[0].Summary)
107+
require.Equal(t, "TestBar", failures[1].Summary)
108+
require.Contains(t, failures[0].Details, "first failure")
109+
require.Contains(t, failures[1].Details, "second failure")
110+
})
111+
112+
t.Run("SubtestName", func(t *testing.T) {
113+
stdout := `--- FAIL: TestSuite/TestCase (0.00s)
114+
foo_test.go:5: assertion error
115+
FAIL
116+
`
117+
failures := parseAssertionFailures(stdout)
118+
require.Len(t, failures, 1)
119+
require.Equal(t, "TestSuite/TestCase", failures[0].Summary)
120+
})
121+
122+
t.Run("StopsAtNextFailBlock", func(t *testing.T) {
123+
stdout := `--- FAIL: TestA (0.01s)
124+
a_test.go:1: only A's output
125+
--- FAIL: TestB (0.01s)
126+
b_test.go:2: only B's output
127+
FAIL
128+
`
129+
failures := parseAssertionFailures(stdout)
130+
require.Len(t, failures, 2)
131+
require.NotContains(t, failures[0].Details, "only B's output")
132+
require.NotContains(t, failures[1].Details, "only A's output")
133+
})
134+
135+
t.Run("PanicBodyCaptured", func(t *testing.T) {
136+
// When a test panics, "--- FAIL:" precedes the panic output. Because
137+
// parseAssertionFailures runs independently from parseAlerts, the panic
138+
// content appears in both the ASSERTION FAILURE and PANIC sections.
139+
stdout := `--- FAIL: TestPanic (0.00s)
140+
panic: something bad
141+
goroutine 1 [running]:
142+
FAIL
143+
`
144+
failures := parseAssertionFailures(stdout)
145+
require.Len(t, failures, 1)
146+
require.Equal(t, "TestPanic", failures[0].Summary)
147+
require.Contains(t, failures[0].Details, "panic: something bad")
148+
})
149+
150+
t.Run("NoFailures", func(t *testing.T) {
151+
stdout := `=== RUN TestPass
152+
--- PASS: TestPass (0.00s)
153+
ok go.temporal.io/server/tests 0.1s
154+
`
155+
require.Empty(t, parseAssertionFailures(stdout))
156+
})
157+
158+
t.Run("EmptyInput", func(t *testing.T) {
159+
require.Empty(t, parseAssertionFailures(""))
160+
})
161+
}
162+
69163
func TestParseAlerts_DataRaceAndPanic(t *testing.T) {
70164
input, err := os.ReadFile("testdata/alerts-input.log")
71165
require.NoError(t, err)
72166

73167
alerts := parseAlerts(string(input))
74168
require.NotEmpty(t, alerts)
75-
require.Equal(t, 2, len(alerts))
76-
77-
require.Contains(t, alerts[0].Details, "WARNING: DATA RACE")
78-
require.Contains(t, alerts[0].Tests[0], "test.TestDataRaceExample")
79-
require.Contains(t, primaryTestName(alerts[0].Tests), "test.TestDataRaceExample")
80-
require.Contains(t, alerts[1].Details, "panic: ")
81-
require.Contains(t, alerts[1].Tests[0], "test.TestPanicExample")
82-
require.Contains(t, primaryTestName(alerts[1].Tests), "TestPanicExample")
169+
170+
raceAlerts := alertsOfKind(alerts, alertKindDataRace)
171+
require.Len(t, raceAlerts, 1)
172+
require.Contains(t, raceAlerts[0].Details, "WARNING: DATA RACE")
173+
require.Contains(t, raceAlerts[0].Tests[0], "test.TestDataRaceExample")
174+
require.Contains(t, primaryTestName(raceAlerts[0].Tests), "test.TestDataRaceExample")
175+
176+
panicAlerts := alertsOfKind(alerts, alertKindPanic)
177+
require.Len(t, panicAlerts, 1)
178+
require.Contains(t, panicAlerts[0].Details, "panic: ")
179+
require.Contains(t, panicAlerts[0].Tests[0], "test.TestPanicExample")
180+
require.Contains(t, primaryTestName(panicAlerts[0].Tests), "TestPanicExample")
83181

84182
// Ensure dedupe works
85183
deduped := dedupeAlerts(append(alerts, alerts...))

‎tools/testrunner/testdata/junit-alerts-output.xml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
<testsuites tests="2" failures="2">
22
<testsuite name="ALERTS" tests="2" failures="2" errors="0" id="0" time="">
33
<testcase name="DATA RACE: Data race detected — in go.temporal.io/server/tools/testrunner.TestShowPanic" classname="">
4-
<failure message="DATA RACE"><![CDATA[Detected in tests:
4+
<failure message="DATA RACE"><![CDATA[WARNING: DATA RACE
5+
...
6+
Detected in tests:
57
go.temporal.io/server/tools/testrunner.TestShowPanic]]></failure>
68
</testcase>
79
<testcase name="PANIC: This is a panic — in TestPanicExample" classname="">
8-
<failure message="PANIC"><![CDATA[Detected in tests:
10+
<failure message="PANIC"><![CDATA[panic: This is a panic
11+
...
12+
Detected in tests:
913
TestPanicExample]]></failure>
1014
</testcase>
1115
</testsuite>

0 commit comments

Comments
 (0)