Repository navigation
ci: print live test results to the job log - #242
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Panic and timeout diagnostics can be lost for interrupted tests, and the prefix-regression fixture does not verify exact test-name matching.
2 open issues
What changed in this PR
2 Important, 0 Nit, 0 Pre-existing.
This PR makes live B2 test results visible in GitHub Actions job logs while preserving the JSON artifact and Summary-page report.
Changes:
- Streams top-level test results and selected diagnostic output.
- Prints summary counts, package timings, and capped failed-test output.
- Adds shell regression tests for report logging and failure handling.
Checked the supplied diff and relevant script context against the stated logging behavior. No tests or live workflow runs were executed during this review. A human code owner must review the workflow changes.
| File | Description |
|---|---|
scripts/live-test-report.sh |
Prints summaries, package timings, and failed-test output to the log. |
scripts/live-test-report_test.sh |
Adds fixtures for log output, exact-name matching, and long failures. |
.github/workflows/test-live.yml |
Streams readable test events while retaining the full JSON artifact. |
Review effort: Balanced
david-santacruz-backblaze
left a comment
There was a problem hiding this comment.
One thing on the failed-test output, see inline.
| (.Test != null and ((key as $k | $f | index($k) != null) or (key as $k | $cut | index($k) != null))) | ||
| or (.Test == null and (.Package as $p | $badpkgs | index($p) != null)))) | ||
| | .Output' "$in" >"$out" | ||
| head -n 300 "$out" |
There was a problem hiding this comment.
head -n 300 keeps the start of the output, but the t.Fatal message usually comes last. I tried a failing test that logs 400 lines first, and the actual failure never made it into the log. Maybe tail -n 300 instead?
There was a problem hiding this comment.
Thanks! will take a look 🚀
There was a problem hiding this comment.
Good catch, reproduced: with 400 lines of noise before the t.Fatal, head -n 300 dropped the failure message. Fixed in the latest commit. I did not switch to tail alone, because a timeout puts its reason (panic: test timed out) first, followed by a long stack, and tail would cut it. The report now keeps the first 100 and last 200 lines with an omitted-lines marker. Tests cover both cases and fail if reverted to head or to tail. I checked it on a real go test run with 400 log lines and a t.Fatal: the failure message is printed. Leaving the thread open for you to resolve.
david-santacruz-backblaze
left a comment
There was a problem hiding this comment.
Hey, give a quick look at these please before merging. All suggestions, no hard violations.
capinscripts/live-test-report.shcan drop key lines the description says are always kept. It takes the first 20panic:/test timed out/fatal error:matches and then subtracts the kept head and tail, so early matches inside the kept lines can crowd out later ones in the omitted middle. Subtracting first and then taking 20 would avoid that. A fixture with 30 earlypanic:lines and atest timed outline in the omitted middle would pin it.- The failed-output group never prints
build-outputevents, so a build failure shows onlyFAIL … [build failed]. The live stream does print them. Consider adding them to the group, or note in the description that it's intentional. - The inline jq filter in
.github/workflows/test-live.yml(fromjson?,build-output,build-fail) has no test. One fixture through the same filter would cover it. - The panic regex lives in both
test-live.ymlandlive-test-report.sh. Defining it once, or a comment in each place pointing to the other, would keep them in sync. - The omitted-lines count (
length - 300) ignores the re-added key lines, so it slightly overstates what was dropped. - In
scripts/live-test-report_test.sh, "(David's case)" could describe the scenario instead. The repeatedawkgenerator and the hardcodedgithub.com/Backblaze/blazer/b2could move into helpers (ev()already takes a package), and acheck_absentwould cover the threeset +eblocks. - In
cap,$keyholds important lines and shadowsdef key, so a rename would help.
Verified: bash scripts/live-test-report_test.sh passes 15/15. The workflow run on GitHub is still NOT RUN.

Motivation
The live test job log showed nothing about what ran.
go test -json ... > test.jsonsent all test output to a file, and the report step wrote its table only to the run's Summary page. In run 40964119112 the log had a 3m26s gap and no test names, counts or failures. You had to open the Summary tab or download the artifact to see that 37 of 37 live tests passed.Changes
test-live.yml:go test -jsonnow runs asgo test -json ... | tee test.json | jq ...underset -o pipefail. The log streams one line per top-level test (PASS pkg Test (time)), plus package-level output, build errors and panics.test.jsonis still written in full and uploaded; the jq side skips non-JSON lines so it cannot cut the artifact short.scripts/live-test-report.sh: prints the summary table to the log as well as the Summary page, then per-package results with timings, then the output of failed tests in a collapsible group (each failing test and failed package gets its own 300-line budget, so a noisy test cannot hide another's failure; past 300 lines the first 100 and last 200 are kept with an omitted-lines marker, andpanic:,test timed outandfatal error:lines from the omitted part are always kept; matched by exact package and test name). The live stream in the workflow also prints those panic and timeout lines as they happen. A panic or timeout is covered too: Go attributes the stack trace to the running test but emits only a package failure, so the group also prints output of tests that started and never finished in a failed package, plus that package's own output.scripts/live-test-report_test.sh(picked up bymake scripts-test): summary in the log and the summary file, per-package line, failed-test output, a failingTestAttrsLongprinting while a passingTestAttrs(prefix name) does not, a timeout fixture whose stack trace is attributed to a cut-off test, finished tests in a failed package not being printed, and a long failure output (5,000 lines) still reaching the::error::lines, thet.Fatalmessage surviving 400 lines of noise, a timeout reason surviving a 600-line stack trace and 150 lines of progress output before it, and three failing tests where the quiet one sits between two noisy ones keeping its message.Testing
make cipasses (includingscripts-test);actionlintandshellcheckare clean. I ran the new jq filter and the report script on the reallive-test-jsonartifact from run 40964119112: the report prints37 of 37live tests andPASS b2 (199.314s),PASS base (15.224s),PASS b2/sync (2.577s).I also reproduced a real
go test -timeout 1shang and ran its JSON through the report: it printspanic: test timed out after 1s. Each fix reverted makes a test fail: the old report script (no log output), matching by substring (prints a passing test's output), reporting only tests with a failure event (the timeout case), and piping straight intohead(exit 141 instead of 1 once output exceeds the pipe buffer).NOT RUN: the workflow on GitHub. It needs the
live-testsenvironment secrets, so the first real check is the next live run onmainor a manualworkflow_dispatch; the log should then list each test.Risk
CI logging only; no library code or wire behavior changes. The log gets longer: one line per top-level live test (a few hundred). No new secret exposure: the added output is test names, timings and test output, and the B2 keys are not printed by either script. A test that itself logged a credential would now show in the job log.
How to revert
Reverting the commits is enough.
Self-review
An independent subagent reviewed the diff and found 4 issues, all fixed in d2c4ace and the follow-up test commit. (1)
jq | head -n 300underset -euo pipefailexited 141 and skipped the::error::lines on long output; now written to a temp file first. (2) The failed-test filter matched by substring, so a passingTestAttrsprinted whenTestAttrsLongfailed; now exact match. (3) Failed-test output was double-spaced; nowjq -j. (4) The workflow filter droppedbuild-outputandbuild-failevents and would abort on a non-JSON line, truncatingtest.json; now handled withfromjson?. Copilot and Codex then raised two more, both fixed in the latest commit: a timeout or panic left no explanation in the log (Go emits no failure event for the cut-off test, so its output was dropped), and the prefix-name fixture had the failing and passing tests reversed, so a substring matcher would still have passed. Not run:/code-reviewand/security-reviewas skills; the reviewer was a general-purpose subagent that did not write the code.Checklist
make cipasses locally./code-review,/security-reviewor a subagent) ran on the diff; findings are fixed or answered below.skip-changelog.Review from David:
head -n 300dropped thet.Fatalmessage when a test logged more than 300 lines first. Fixed by keeping the first 100 and last 200 lines with an omitted-lines marker, nottailalone, which would cut a timeout'spanic: test timed outline (it comes first, followed by a long stack). Reproduced first with a failing test, then confirmed on a realgo testrun that logs 400 lines and then callst.Fatal: the failure message is now printed. Reverting toheador totailalone each fails a test.Review from Copilot on the head-plus-tail fix: with 150 progress lines before
panic: test timed outand 600 stack lines after it, the truncation dropped the reason, and one shared budget let a noisy test hide another test'st.Fatal. Both reproduced first with failing fixtures, then fixed with a per-test budget and always-kept panic and timeout lines; reverting either fails a test. Re-checked on realgo testruns (a 1 s timeout, and 400 log lines thent.Fatal).