Skip to content

Commit 9091a7f

Browse files
committed
JUnit summary with failures
1 parent 956f6ec commit 9091a7f

9 files changed

Lines changed: 727 additions & 73 deletions

File tree

.github/actions/get-job-id/action.yml

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,27 +31,40 @@ runs:
3131
run: |
3232
set -euo pipefail
3333
job_url="https://api.github.com/repos/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}/jobs?per_page=100"
34-
json=$(curl -sSL \
35-
-H "Accept: application/vnd.github+json" \
36-
-H "Authorization: Bearer $GITHUB_TOKEN" \
37-
"$job_url")
34+
job_id=""
35+
json=""
36+
for attempt in 1 2 3 4 5 6; do
37+
json=$(curl -sSL \
38+
-H "Accept: application/vnd.github+json" \
39+
-H "Authorization: Bearer $GITHUB_TOKEN" \
40+
"$job_url")
3841
39-
# Prefer matching both job name and the current runner name to disambiguate matrix jobs
40-
job_id=$(jq -r --arg name "$JOB_NAME" --arg runner "$RUNNER_NAME" '
41-
(.jobs // [])
42-
| map(select(.name == $name and (.runner_name // "") == $runner))
43-
| (.[0].id // empty)
44-
' <<< "$json" )
45-
46-
# Fallback: match by name only
47-
if [ -z "${job_id:-}" ]; then
48-
job_id=$(jq -r --arg name "$JOB_NAME" '
49-
(.jobs // []) | map(select(.name == $name)) | (.[0].id // empty)
42+
# Prefer matching both job name and the current runner name to disambiguate matrix jobs
43+
job_id=$(jq -r --arg name "$JOB_NAME" --arg runner "$RUNNER_NAME" '
44+
(.jobs // [])
45+
| map(select(.name == $name and (.runner_name // "") == $runner))
46+
| (.[0].id // empty)
5047
' <<< "$json" )
51-
fi
48+
49+
# Fallback: match by name only
50+
if [ -z "${job_id:-}" ]; then
51+
job_id=$(jq -r --arg name "$JOB_NAME" '
52+
(.jobs // []) | map(select(.name == $name)) | (.[0].id // empty)
53+
' <<< "$json" )
54+
fi
55+
56+
if [ -n "${job_id:-}" ] && [ "$job_id" != "null" ]; then
57+
break
58+
fi
59+
60+
if [ "$attempt" -lt 6 ]; then
61+
echo "::notice::Job ID for '$JOB_NAME' not visible yet (attempt $attempt/6); retrying in 5s"
62+
sleep 5
63+
fi
64+
done
5265
5366
if [ -z "${job_id:-}" ] || [ "$job_id" = "null" ]; then
54-
echo "::error::Failed to resolve job ID for name '$JOB_NAME' on runner '$RUNNER_NAME' in run '$RUN_ID'" >&2
67+
echo "::error::Failed to resolve job ID for name '$JOB_NAME' on runner '$RUNNER_NAME' in run '$RUN_ID' after retries" >&2
5568
exit 1
5669
fi
5770
echo "job_id=$job_id" >> "$GITHUB_OUTPUT"

.github/workflows/run-tests.yml

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,18 @@ jobs:
314314
path: ~/.cache/go-build
315315
key: go-${{ runner.os }}${{ runner.arch }}-build-${{ env.COMMIT }}
316316

317+
- name: Get job ID
318+
id: get_job_id
319+
uses: ./.github/actions/get-job-id
320+
with:
321+
job_name: Unit test
322+
run_id: ${{ github.run_id }}
323+
317324
- name: Run unit tests
318325
timeout-minutes: 20
319326
run: TEST_TIMEOUT=15m ./develop/github/monitor_test.sh make unit-test-coverage
327+
env:
328+
GITHUB_JOB_ID: ${{ steps.get_job_id.outputs.job_id }}
320329

321330
- name: Print memory snapshot
322331
if: always()
@@ -328,16 +337,6 @@ jobs:
328337
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
329338
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
330339
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-
341340
- name: Upload code coverage to Codecov
342341
uses: codecov/codecov-action@v5
343342
with:
@@ -354,13 +353,6 @@ jobs:
354353
flags: unit-test
355354
report_type: test_results
356355

357-
- name: Get job ID
358-
id: get_job_id
359-
uses: ./.github/actions/get-job-id
360-
with:
361-
job_name: Unit test
362-
run_id: ${{ github.run_id }}
363-
364356
- name: Upload test results to GitHub
365357
# Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
366358
uses: actions/upload-artifact@v6
@@ -414,9 +406,18 @@ jobs:
414406
# shellcheck disable=SC2046
415407
docker compose -f ${{ env.DOCKER_COMPOSE_FILE }} up --wait $(docker compose -f ${{ env.DOCKER_COMPOSE_FILE }} ps --services)
416408
409+
- name: Get job ID
410+
id: get_job_id
411+
uses: ./.github/actions/get-job-id
412+
with:
413+
job_name: Integration test
414+
run_id: ${{ github.run_id }}
415+
417416
- name: Run integration test
418417
timeout-minutes: 15
419418
run: ./develop/github/monitor_test.sh make integration-test-coverage
419+
env:
420+
GITHUB_JOB_ID: ${{ steps.get_job_id.outputs.job_id }}
420421

421422
- name: Print memory snapshot
422423
if: always()
@@ -428,16 +429,6 @@ jobs:
428429
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
429430
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
430431
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-
441432
- name: Upload code coverage to Codecov
442433
uses: codecov/codecov-action@v5
443434
with:
@@ -454,13 +445,6 @@ jobs:
454445
flags: integration-test
455446
report_type: test_results
456447

457-
- name: Get job ID
458-
id: get_job_id
459-
uses: ./.github/actions/get-job-id
460-
with:
461-
job_name: Integration test
462-
run_id: ${{ github.run_id }}
463-
464448
- name: Upload test results to GitHub
465449
# Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
466450
uses: actions/upload-artifact@v6
@@ -547,6 +531,7 @@ jobs:
547531
timeout-minutes: ${{ matrix.github_timeout }}
548532
run: ./develop/github/monitor_test.sh ${{ matrix.cmd }}
549533
env:
534+
GITHUB_JOB_ID: ${{ steps.get_job_id.outputs.job_id }}
550535
TEST_TOTAL_SHARDS: ${{ matrix.total_shards }}
551536
TEST_SHARD_INDEX: ${{ matrix.total_shards && matrix.shard_index }} # guard with total_shards to avoid falsy eval of shard_index=0
552537
TEST_ARGS: "${{ matrix.test_args }}"
@@ -567,16 +552,6 @@ jobs:
567552
[ "$(find .testoutput -maxdepth 1 -name 'junit.*.xml' | wc -l)" -lt "$MAX_TEST_ATTEMPTS" ] &&
568553
CRASH_REPORT_NAME="$GITHUB_JOB" make report-test-crash
569554
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-
580555
- name: Upload code coverage to Codecov
581556
uses: codecov/codecov-action@v5
582557
with:

tools/testrunner/junit.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ import (
1313
"github.com/jstemmer/go-junit-report/v2/junit"
1414
)
1515

16+
// alertsSuiteName is the JUnit suite name used for structural alerts (data
17+
// races, panics, fatal errors). It is also used by collectSummaryRows to
18+
// distinguish alert entries from regular test failures.
19+
const alertsSuiteName = "ALERTS"
20+
1621
type junitReport struct {
1722
junit.Testsuites
1823
path string
@@ -83,19 +88,22 @@ func (j *junitReport) appendAlertsSuite(alerts []alert) {
8388
if p := primaryTestName(a.Tests); p != "" {
8489
name = fmt.Sprintf("%s — in %s", name, p)
8590
}
86-
// Include only test names for context, not the full log details to avoid XML malformation
87-
var details string
91+
var sb strings.Builder
92+
if a.Details != "" {
93+
sb.WriteString(sanitizeXML(a.Details))
94+
sb.WriteByte('\n')
95+
}
8896
if len(a.Tests) > 0 {
89-
details = fmt.Sprintf("Detected in tests:\n\t%s", strings.Join(a.Tests, "\n\t"))
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,
95103
})
96104
}
97105
suite := junit.Testsuite{
98-
Name: "ALERTS",
106+
Name: alertsSuiteName,
99107
Failures: len(cases),
100108
Tests: len(cases),
101109
Testcases: cases,
@@ -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 {
@@ -224,6 +247,9 @@ func mergeReports(reports []*junitReport) (*junitReport, error) {
224247
// Discard test case parents since they provide no value.
225248
continue
226249
}
250+
if testCase.Failure != nil && testCase.Failure.Data != "" {
251+
testCase.Failure.Data = parseFailureDetails(testCase.Failure.Data)
252+
}
227253
testCase.Name += suffix
228254
newSuite.Testcases = append(newSuite.Testcases, testCase)
229255
}

tools/testrunner/junit_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,19 @@ func TestMergeReports_SingleReport(t *testing.T) {
7373
require.Len(t, testNames, 5)
7474
require.NotContains(t, testNames, "TestCallbacksSuite")
7575
require.NotContains(t, testNames, "TestCallbacksSuite/TestWorkflowNexusCallbacks_CarriedOver")
76+
var failureData string
77+
for _, tc := range suites[0].Testcases {
78+
if tc.Name == "TestCallbacksSuite/TestWorkflowCallbacks_InvalidArgument" {
79+
require.NotNil(t, tc.Failure)
80+
failureData = tc.Failure.Data
81+
break
82+
}
83+
}
84+
require.NotEmpty(t, failureData)
85+
require.Contains(t, failureData, "Error Trace:")
86+
require.Contains(t, failureData, "expected: 1")
87+
require.NotContains(t, failureData, "=== RUN")
88+
require.NotContains(t, failureData, "--- FAIL:")
7689
}
7790

7891
func TestMergeReports_MultipleReports(t *testing.T) {

0 commit comments

Comments
 (0)