Skip to content

feat(crash_aware_tests): tell a Go runtime crash from a failing test, in Go - #1809

Open
jmrplens wants to merge 5 commits into
mainfrom
issue-1114-p5-crash-aware
Open

jmrplens wants to merge 5 commits into
mainfrom
issue-1114-p5-crash-aware

Conversation

@jmrplens

@jmrplens jmrplens commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Refs #1114

This is item 5 of #1114: .github/scripts/go-test-crash-aware.sh ported to Go as cmd/crash_aware_tests. It is the first layer of a stack that ports items 5, 11 and 12 and adds the rule of #1477.

The cross-platform job runs its unit suite through the runner on all three legs. The Windows leg dies inside the Go runtime now and then (#467, explained by Go issue 81238), and the runner keeps that crash apart from a failing test. It is a Go command now, so the step runs in each platform's default shell and the Windows leg needs neither bash nor jq to read its own verdict.

The command keeps the script's classes. It runs the suite through gotestsum and reads the go test -json stream back. A package with a fatal error: line outside the messages the runtime uses for a bug in the code under test is crashed. One with a fail event, no failing test and no line explaining it is dead. Every other package with a fail event is failing and fails the run, listed with its tests. Crashed and dead packages run once more, alone and with the same flags, and pass the run only when every one passes there. The report goes to the log and to the job summary.

Two answers the script gave without meaning to are refused. A first run that wrote no stream exited with gotestsum's own status, which is 0 when gotestsum could not tell anything went wrong; it now fails. A rerun that wrote no stream left every package of it reported as passed when gotestsum exited 0; each is now reported with no stream, and the run fails. A pipe in a crash line is escaped in its table cell.

The script, its declaration and the line of pendingPorts for item 5 are gone.

Taking the declaration out touches cmd/audit_supply_chain, where make coverage-mutants left two mutants of rule 9's untriggeredEvents alive, code this layer did not write: no case named two events in one condition. A commit of this layer adds that case. A last commit regenerates the counts of docs/development/testing/testing.md with -skip-coverage, so make check-testing-docs passes on this layer whether CI checks the freshness gates here or defers them to the top of the stack.

How I checked it

  • The tests came first, against a package holding only its doc comment, where they do not build. Recorded go test -json streams under testdata/ stand for each path the script took: a pass, a crash beside a dead package (the 2026-09-12 Windows run), a failing package beside a crash, and the three reruns. The whole command runs against a stand-in gotestsum, the test binary started again, so the exec path is the real one on every platform.
  • go test -race passes on cmd/crash_aware_tests and cmd/audit_supply_chain, both at 100.0% of statements, and the whole unit suite passes on this layer's head with the freshness gates checked.
  • golangci-lint and its formatters are clean on both packages, and make check-supply-chain, the make targets of the Generated artifacts job and actionlint pass.
  • A run against the real gotestsum over two packages passes and writes the stream it reads.

Mutants

Package Killed Lived Not covered Timed out
cmd/crash_aware_tests 40 0 0 0
cmd/audit_supply_chain 1160 0 0 0

…classes, failing first

Item 5 of #1114 moves .github/scripts/go-test-crash-aware.sh into Go.
These tests come first, against a package holding only its doc comment,
where they do not build (undefined: stream, run, gotestsumCommand).

Recorded go test -json streams under testdata/ stand for each path the
script took: a pass, a crash beside a dead package (the 2026-09-12
Windows run), a failing package beside a crash, and the three reruns.
The whole command is driven against a stand-in gotestsum, this test
binary started again, so the exec path is the real one on every
platform the cross-platform job runs.

Refs #1114

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
@sourcery-ai

sourcery-ai Bot commented Nov 12, 2026

Copy link
Copy Markdown

Sorry @jmrplens, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 6 days by commenting @sourcery-ai review. Upgrade to get a review now.

@github-actions github-actions Bot added security Security-related issue ci Build pipeline, workflows, or lint configuration tooling The audit and generator commands under cmd/, and the Makefile targets that run them labels Nov 12, 2026
@jmrplens
jmrplens added this pull request to stack #1814 November 12, 2026 00:46
@jmrplens jmrplens added feature New feature or MCP tool v3.5.0 Targeted at the 3.5.0 release labels Nov 12, 2026
@jmrplens jmrplens added this to the 3.5.0 milestone Nov 12, 2026
@jmrplens jmrplens changed the title issue 1114 p5 crash aware feat(crash_aware_tests): tell a Go runtime crash from a failing test, in Go Nov 12, 2026
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between c3b8d2c and ef8c79b.

Files selected for processing: 17

  • .github/scripts/go-test-crash-aware.sh
  • .github/workflows/ci.yml
  • cmd/audit_supply_chain/declarations.go
  • cmd/audit_supply_chain/triggers_test.go
  • cmd/crash_aware_tests/doc.go
  • cmd/crash_aware_tests/main.go
  • cmd/crash_aware_tests/main_test.go
  • cmd/crash_aware_tests/stream.go
  • cmd/crash_aware_tests/stream_test.go
  • cmd/crash_aware_tests/summary.go
  • cmd/crash_aware_tests/testdata/crash.json
  • cmd/crash_aware_tests/testdata/failing.json
  • cmd/crash_aware_tests/testdata/pass.json
  • cmd/crash_aware_tests/testdata/rerun-crash.json
  • cmd/crash_aware_tests/testdata/rerun-fail.json
  • cmd/crash_aware_tests/testdata/rerun-pass.json
  • docs/development/testing/testing.md

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • CI Improvements
    • Cross-platform Go tests now use a runner that distinguishes test failures from runtime crashes, retries eligible crashes once, and reports outcomes in the job summary.
  • Bug Fixes
    • Added a check that flags workflow job conditions accepting events not configured to trigger the workflow.

The crash-aware test runner moves from a Bash script to a Go command. The command classifies gotestsum JSON events, reruns selected packages, and reports outcomes. CI invokes the command. The change also adds a workflow-trigger test and updates testing statistics.

Changes: Crash-aware test runner

Layer / File(s) Summary
Classify test stream outcomes
cmd/crash_aware_tests/stream.go, cmd/crash_aware_tests/stream_test.go, cmd/crash_aware_tests/testdata/*
The parser identifies package and test failures, records crash lines, and selects crash or unexplained package failures for reruns. Tests and fixtures cover these classifications.
Run suites and report outcomes
cmd/crash_aware_tests/main.go, cmd/crash_aware_tests/summary.go, cmd/crash_aware_tests/main_test.go, cmd/crash_aware_tests/doc.go
The command runs gotestsum, reports first-run failures, reruns crash candidates, and writes results to stdout and an optional job summary. Documentation and tests cover arguments, outcomes, and error handling.
Wire the Go command into CI
.github/scripts/go-test-crash-aware.sh, .github/workflows/ci.yml, cmd/audit_supply_chain/declarations.go, docs/development/testing/testing.md
The Bash runner and its pending-port declaration are removed. CI invokes cmd/crash_aware_tests, and the testing statistics include the new command package.

Workflow trigger validation test

Layer / File(s) Summary
Check unmatched workflow events
cmd/audit_supply_chain/triggers_test.go
A new test checks that a push-only workflow reports separate problems for job conditions naming schedule and release.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant CI as CI workflow
  participant Runner as crash_aware_tests
  participant Gotestsum as gotestsum
  participant Stream as JSON stream
  participant Reporter as summary reporter
  CI->>Runner: invoke with test flags and packages
  Runner->>Gotestsum: run initial suite
  Gotestsum->>Stream: write test events
  Runner->>Stream: read and classify events
  Runner->>Gotestsum: rerun crash candidates
  Gotestsum->>Stream: write rerun events
  Runner->>Reporter: report outcomes
Loading

Possibly related issues

Possibly related PRs

  • jmrplens/gitlab-mcp-server#1550: It adds the script-declaration mechanism that this PR updates when removing the shell-script declaration and pending-port entry.

Suggested reviewers: sourcery-ai

Merge Risk: Minimal · previous review c3b8d

This change ports the crash-aware test runner from a shell script to a Go command and keeps its behavior. No merge-blocking defect was found in the supplied evidence.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: a Go command, cmd/crash_aware_tests, that separates Go runtime crashes from failing tests, replacing the bash script. It is a single, specific sentence in conventional…
Description check Passed The description follows the template's main sections. It states the purpose and links Refs #1114, and it lists key changes, testing steps, and mutant results. It does not follow the template's exact h…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: a Go command, cmd/crash_aware_tests, that separates Go runtime crashes from failing tests, replacing the bash script. It is a single, specific sentence in conventional-commit form, and the changeset matches it. The title is long (75 characters) but still clear.

Full details: Description check

Explanation

The description follows the template's main sections. It states the purpose and links Refs #1114, and it lists key changes, testing steps, and mutant results. It does not follow the template's exact headings. It omits the 'Type of Change' checkboxes, the 'Breaking Changes / Migration Notes' section, and the 'Screenshots / Logs' section. It also does not fill in the Checklist. The 'Closes #' line is replaced by 'Refs #1114', which the template permits as an alternative. The content is substantive and specific, so the description is mostly complete.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between db7e30a and c3b8d2c.

Files selected for processing: 17

  • .github/scripts/go-test-crash-aware.sh
  • .github/workflows/ci.yml
  • cmd/audit_supply_chain/declarations.go
  • cmd/audit_supply_chain/triggers_test.go
  • cmd/crash_aware_tests/doc.go
  • cmd/crash_aware_tests/main.go
  • cmd/crash_aware_tests/main_test.go
  • cmd/crash_aware_tests/stream.go
  • cmd/crash_aware_tests/stream_test.go
  • cmd/crash_aware_tests/summary.go
  • cmd/crash_aware_tests/testdata/crash.json
  • cmd/crash_aware_tests/testdata/failing.json
  • cmd/crash_aware_tests/testdata/pass.json
  • cmd/crash_aware_tests/testdata/rerun-crash.json
  • cmd/crash_aware_tests/testdata/rerun-fail.json
  • cmd/crash_aware_tests/testdata/rerun-pass.json
  • docs/development/testing/testing.md
Files with no reviewable changes: 2

  • .github/scripts/go-test-crash-aware.sh
  • cmd/audit_supply_chain/declarations.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@jmrplens jmrplens closed this Nov 12, 2026
@jmrplens jmrplens reopened this Nov 12, 2026
… in Go

The command reads gotestsum's go test -json stream back and keeps the
script's classes: a package with a "fatal error:" line outside the list
of messages the runtime uses for a bug in the code under test is
crashed, one with a fail event, no failing test and no line explaining
it is dead, and every other package with a fail event is failing.
Failing packages fail the run with their tests listed; crashed and dead
ones are run once more, alone and with the same flags, and pass the run
only when every one of them passes there. The report goes to the log
and to the job summary GitHub names.

Two answers the script gave without meaning to are refused. A first
run that wrote no stream exited with gotestsum's own status, which is
0 when gotestsum could not tell anything went wrong; it now fails. A
rerun that wrote no stream reported every package as passed when
gotestsum exited 0; each is now reported with no stream, and the run
fails. A pipe in a crash line is escaped in its table cell.

100.0% of statements; make coverage-mutants: 41 killed, 0 lived, 0 not
covered, 0 timed out.

Refs #1114

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
The unit suite of the three cross-platform legs runs through the Go
command instead of .github/scripts/go-test-crash-aware.sh, in each
platform's default shell, so the Windows leg needs neither bash nor jq
to read its own verdict. The script, its declaration and the line of
pendingPorts for item 5 of #1114 are gone, and the verdict job's
comment names the command.

Refs #1114

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
make coverage-mutants on cmd/audit_supply_chain, which this stack
touches, left two mutants of untriggeredEvents alive: reading only the
first github.event_name comparison of a condition passed every case,
since none named two events. A condition naming two events the
workflow is not triggered by is now reported for each of them.

make coverage-mutants: 1158 killed and 2 lived before this case.

Refs #1114

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
docs/development/testing/testing.md is regenerated with -skip-coverage
for the test functions and files cmd/crash_aware_tests adds, so `make
check-testing-docs` passes on this layer whether CI checks the
freshness gates here or defers them to the top of the stack. The new
package reads n/a until the top of the stack measures its coverage.

Refs #1114

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
@jmrplens
jmrplens force-pushed the issue-1114-p5-crash-aware branch from c3b8d2c to ef8c79b Compare November 12, 2026 03:47
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Build pipeline, workflows, or lint configuration feature New feature or MCP tool security Security-related issue tooling The audit and generator commands under cmd/, and the Makefile targets that run them v3.5.0 Targeted at the 3.5.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant