Repository navigation
fix(testing): keep killed CLI children out of tarpaulin's coverage merge - #2748
Merged
Merged
Conversation
The Coverage job failed on main runs 4588 and 4600, two days apart on
unrelated commits, each time after all 2,441 tests had passed:
Merging coverage reports
thread 'main' panicked at llvm_profparser-0.14.0/.../mod.rs:58:35:
internal error: entered unreachable code: llvm_profparsers works on
complete data
A spawned child inherits the *parent's* `LLVM_PROFILE_FILE` template, so the
instrumented `bashkit` binary writes a profraw named after the parent test
binary into `target/tarpaulin/profraws/` — the directory tarpaulin merges.
The supervised-child harnesses kill their children by design, and a SIGKILLed
process never flushes its profraw, so the merge step hits a truncated file.
It is a race on whether the victim flushed, hence intermittent, and invisible
to required CI, which is not instrumented. Re-running attempt 2 of run 4600
on the same commit went green, which is the race.
`testing::child_profile_path()` hands each child its own path outside the
merge set: one directory per process, a unique file per call, removal left to
the OS. Cleanup is deliberately unguarded — a killed child's foreign
descendant can still be writing there, and a per-call remove that failed with
ENOTEMPTY would sink a security test for a reason unrelated to it.
Applied at every spawn of the instrumented binary: `run_cli_argv_bounded` and
`run_raw_core` in `threat_model_tests` (the latter takes the program as a
parameter, so it is a latent trap), and `cli()` plus `run_bounded_child` in
`cli_oneshot`, whose direct `Command::new(BIN)` now goes through `cli()` so it
cannot drift again. `run_input_shape_bounded` keeps its own path: it asserts
on that profraw as a bounded-artifact receipt, and the rule is only that the
child never writes into the merge set.
Redirecting also fixes attribution: without it the CLI's counters land in a
file tarpaulin credits to the parent test binary.
Two guards. `cli_spawning_tests_redirect_the_child_coverage_profile` scans
every crate and fails any file that reaches the binary without setting a
profile path, failing closed when it finds no spawners at all. The scan cannot
pin the value, so `bounded_cli_children_get_a_profile_path_outside_the_merge_set`
asserts on the built `Command` — not from inside the child, because the
sandbox withholds host env from the script (TM-INF-013), so a bounded run
cannot echo back what its own process received.
All 27 test binaries pass, clippy -D warnings and fmt clean, check-okf
conformant. The one local failure is `ssh_supabase_connects`, which dials a
live port-22 endpoint unreachable from this sandbox.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 3656c7e | Commit Preview URL Branch Preview URL |
Oct 18 2026, 10:47 AM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Every test that spawns the instrumented
bashkitbinary now points the child'sLLVM_PROFILE_FILEoutside the directorycargo tarpaulinmerges, via a newbashkit::testing::child_profile_path().Applied at every spawn site:
run_cli_argv_boundedandrun_raw_coreinthreat_model_tests(the latter takes the program as a parameter, so it was a latent trap), andcli()plusrun_bounded_childincli_oneshot— whose one directCommand::new(BIN)now goes throughcli()so it cannot drift again.run_input_shape_boundedkeeps its own path: it asserts on that profraw as a bounded-artifact receipt. The rule is only that a child never writes into the merge set, not which mechanism gets it there.Why
The Coverage job has been failing on
mainintermittently — runs 4588 and 4600, two days apart on unrelated commits — each time after all 2,441 tests had already passed:A spawned child inherits the parent's
LLVM_PROFILE_FILEtemplate, so the instrumented CLI writes a profraw named after the parent test binary intotarget/tarpaulin/profraws/— exactly the directory tarpaulin merges. The failing log shows this directly: seven profraws for oneintegration-*binary, each with a different child PID.The supervised-child harnesses kill their children by design (
bounded_helper_kills_hung_childand friends), and a SIGKILLed process never flushes its profraw, so the merge step hits a truncated file. Whether the victim flushed in time is a race, which is why it is intermittent — and why required CI never sees it: required CI is not instrumented.Redirecting also fixes attribution. Without it, the CLI's counters land in a file tarpaulin credits to the parent test binary.
Before / After
Before — run 4600 on
main, all tests green then a merge panic:That the cause is a race, not the tree, is settled by re-running the same commit: attempt 2 of run 4600 passed with no code change. The same tree also passed on the PR head it was merged from. So this PR removes the race rather than a wrong result.
After — the two guards, plus the helper's own unit test:
Validation on this branch:
cargo test --workspace --lib --bins --tests --features http_client,ssh,sqlitecargo clippy --workspace --all-targets --features http_client,ssh,sqlite -- -D warningscargo fmt --all --checkjust check-okfThe one local failure is
ssh_supabase_tests::ssh_supabase_connects, which dials the live publicsupabase.shendpoint on port 22 — unreachable from this HTTPS-only sandbox, unrelated to this change.How it is guarded
Two layers, because neither alone is sufficient:
cli_spawning_tests_redirect_the_child_coverage_profilescans every crate and fails any file that reaches the binary (bashkit_cli_path(),CARGO_BIN_EXE_bashkit) without setting a profile path. It fails closed when the scan finds no spawners at all, so a scan that silently matches nothing cannot pass.bounded_cli_children_get_a_profile_path_outside_the_merge_setasserts on the builtCommand: the path ends in.profraw, is not undertarpaulin, and differs per child.That second test asserts on the
Commandrather than from inside the child on purpose. The sandbox deliberately withholds host env from scripts (TM-INF-013), so a bounded run cannot echo back what its own process received — the first draft of this test tried exactly that and read back an empty string.Risk
ENOTEMPTYand sink a security test for a reason unrelated to it — profile hygiene is not one of this suite's receipts.cargo tarpaulinis not run locally, so the proof that the merge no longer sees truncated files is the mechanism plus the guards above. The Coverage job on this PR exercises the real path.Checklist
pub fnon the#[doc(hidden)]testingmodule; no existing signature changes.Generated by Claude Code