Skip to content

fix(profile): 58 test targets CI never ran, 110 failing, and a syscall the profiler could not name - #2516

Open
noahgift wants to merge 2 commits into
mainfrom
feat/expose-ptx-profile
Open

fix(profile): 58 test targets CI never ran, 110 failing, and a syscall the profiler could not name#2516
noahgift wants to merge 2 commits into
mainfrom
feat/expose-ptx-profile

Conversation

@noahgift

Copy link
Copy Markdown
Contributor

Test triage of a binary surface area, per the standing instruction: bubble the tests up, fix what can be fixed, delete what cannot.

aprender-profile appears in ci.yml zero times and carries 58 integration test targets. They all compile. Running them:

targets passed failed ignored
before 61 1966 110 48
after 61 2079 0 48

Two root causes

109 failures, one cause

`CARGO_BIN_EXE_renacer` is unset

[lib] name = "renacer" but the binary target is aprender-profile, so that env var never existed. 126 cargo_bin("renacer") call sites across 12 files, dead since the APR-MONO rename. Identical defect to the cgp one in #2496, different crate. Binary name taken from cargo metadata, not assumed.

1 failure, hiding a real defect in the profiler

test_realtime_anomaly_detects_slow_syscall compiles a program that sleeps 50 ms and asserts an anomaly is reported. It wasn't. Two findings:

a. clock_nanosleep (x86_64 syscall 230) had no name. The table jumped (228, clock_gettime)(231, exit_group), so the tracer printed syscall_230 and the 50 ms sleep was invisible as a named syscall. Added 229 clock_getres and 230 clock_nanosleep.

b. The test could never have passed for the right reason. The detector builds a per-syscall baseline and flags >3σ from it — and clock_nanosleep occurs exactly once, so there is no baseline to deviate from. The green runs were unrelated microsecond jitter on write (8–19 μs against a 10 μs baseline) happening to exceed 3σ. Its own comment already read "increased for test stability" — which is what tuning a test that asserts the wrong thing looks like.

Rewritten so the anomaly has a baseline: 200 one-byte writes, then one 4 MiB write. Same syscall, so the detector can see it. Stable across three consecutive runs. It now also asserts the fixture compiled — a gcc failure would otherwise make the test assert about nothing.

The falsifier, because nothing guarded this

Removing the two syscall entries again left all 107 syscall tests green. Every existing test asserts a property — determinism, non-empty — and a property test over 0..500 cannot catch a missing entry, since "unknown" is a valid answer across most of that range. Only naming numbers can.

mod table_completeness names nine ABI-fixed syscalls, asserts the 228–231 clock range has no holes, and carries a non-vacuity arm: 9_999 must still be "unknown", or a permissive lookup would satisfy the rest.

Mutation: restore the gap → the two name assertions go RED, the non-vacuity arm stays green.

Gated

cargo test -p aprender-profile --lib wired into guard-runner-labels (in gate.needs) so the crate cannot go dark again. --lib only: the integration targets shell out to the built binary and take minutes.

Two measurements I got wrong and corrected

  • I first read "21 targets ran, 58 files exist" as 37 files never running. Wrong — cargo stops at the first failing target. cargo metadata confirms all 58 exist; --no-fail-fast gave the real 61/110.
  • I tested a stale binary once and had to resolve the path from cargo --message-format=json — the worktree target/debug/ is not where builds land here.

…l the profiler could not name

Test triage of a binary surface area, per the standing instruction: bubble
the tests up, fix what can be fixed, delete what cannot.

`aprender-profile` appears in ci.yml ZERO times and carries 58 integration
test targets. They all COMPILE. Running them:

  before   61 targets   1966 passed   110 failed   48 ignored
  after    61 targets   2079 passed     0 failed   48 ignored

TWO ROOT CAUSES, 110 failures.

1. 109 failures, one cause: `CARGO_BIN_EXE_renacer` is unset.
   `[lib] name = "renacer"` but the BINARY target is `aprender-profile`,
   so that env var never existed. 126 `cargo_bin("renacer")` call sites
   across 12 files, dead since the APR-MONO rename. Identical defect to
   the cgp one in #2496, different crate. Binary name taken from
   `cargo metadata`, not assumed.

2. 1 failure, and it was hiding a real defect in the profiler.
   `test_realtime_anomaly_detects_slow_syscall` compiles a program that
   sleeps 50ms and asserts an anomaly is reported. It was not. Two
   findings behind it:

   a. `clock_nanosleep` (x86_64 syscall 230) had NO NAME. The table
      jumped `(228, clock_gettime)` -> `(231, exit_group)`, so the
      tracer printed `syscall_230` and the 50ms sleep was invisible as a
      named syscall. Added 229 clock_getres and 230 clock_nanosleep.

   b. The test could never have passed for the right reason. The detector
      builds a PER-SYSCALL baseline and flags >3 sigma from it;
      clock_nanosleep occurs exactly ONCE, so there is no baseline for it
      to deviate from. The green runs were unrelated microsecond jitter
      on `write` -- 8-19 us against a 10 us baseline -- happening to
      exceed 3 sigma. Its own comment already read "increased for test
      stability", which is what tuning a test that asserts the wrong
      thing looks like.

      Rewritten so the anomaly HAS a baseline: 200 one-byte writes, then
      one 4 MiB write. Same syscall, so the detector can see it. Stable
      across three consecutive runs. Also asserts the fixture compiled --
      a gcc failure would otherwise make the test assert about nothing.

FALSIFIER FOR (a), because nothing guarded it: removing the two entries
again left all 107 syscall tests GREEN. Every existing test asserts a
PROPERTY -- determinism, non-empty -- and a property test over 0..500
cannot catch a missing entry, since "unknown" is a valid answer across
most of that range. Only naming numbers can.

`mod table_completeness` names nine ABI-fixed syscalls, asserts the
228-231 clock range has no holes, and carries a non-vacuity arm: 9_999
must still be "unknown", or a permissive lookup would satisfy the rest.
Mutation: restore the gap -> the two name assertions go RED, the
non-vacuity arm stays green.

Wired `cargo test -p aprender-profile --lib` into guard-runner-labels
(in gate.needs) so the crate cannot go dark again. --lib only: the
integration targets shell out to the built binary and take minutes.

ONE MEASUREMENT I GOT WRONG AND CORRECTED: I first read "21 targets ran,
58 files exist" as 37 files never running. Wrong -- cargo stops at the
first failing target. `cargo metadata` confirms all 58 targets exist;
`--no-fail-fast` gave the real 61/110. Also tested a stale binary once
and had to resolve the path from cargo --message-format=json.

Refs #2496
noahgift added a commit that referenced this pull request Aug 17, 2026
…N_EXE_...") broke the CI build

The falsifier wiring from the previous commit worked: two of the three targets
ran and passed in CI (3 and 9 tests). The third failed to COMPILE:

    error: environment variable `CARGO_BIN_EXE_aprender-train-shell`
           not defined at compile time
      --> crates/aprender-train-shell/tests/falsify_no_fabricated_fetch_2519.rs:164
          let exe = env!("CARGO_BIN_EXE_aprender-train-shell");

This is the SAME CLASS I spent yesterday removing -- the 126 dead
cargo_bin("renacer") references (#2516) and the `realizar` / `aprender-shell`
ones (#2520) -- reintroduced in a brand-new test of my own. Worth stating
plainly: the class is easy to reintroduce precisely because it compiles fine
wherever the binary happens to have been built already.

WHAT I COULD AND COULD NOT ESTABLISH

The package declares `[[bin]] name = "aprender-train-shell"` with no
`required-features`, so the variable should exist. I reproduced CI's exact
command locally after touching the test file to force a rebuild:

    cargo test -p aprender-train-shell --test falsify_no_fabricated_fetch_2519
    test result: ok. 8 passed

It PASSES here. So my first hypothesis -- that `--test <name>` skips building
the package's bins -- is wrong, and I did not identify the real difference
(cargo version, or a fresh vs warm target dir).

Rather than keep guessing, the fix removes the dependency on compile-time
resolution entirely, which is correct regardless of the cause.

FIX: ask cargo at RUNTIME which executable it produced --
`cargo build --bin ... --message-format=json-render-diagnostics`, then take the
`executable` field. Same pattern already proven for aprender-mcp in #2520, and
the same doctrine as scripts/apr_bin.sh: never construct or assume a binary
path, ask the tool that built it.

The helper FAILS LOUDLY if cargo reports no executable. A test that silently
skipped when the binary was unavailable would be the skip-class escape this repo
bans -- and would have hidden the very defect #2519 is about. It uses a substring
match on the JSON rather than adding a serde dependency to a test crate.

VERIFICATION
  cargo test -p aprender-train-shell --test falsify_no_fabricated_fetch_2519
      8 passed, 0 failed  (all 8, including the -c CLI surface test)
  cargo clippy -p aprender-train-shell --all-targets   0 errors
  cargo fmt -p aprender-train-shell -- --check         rc=0

No compile-time CARGO_BIN_EXE remains in the file; the only `env!` left are a
comment and `env!("CARGO")`, which cargo always sets for tests.

Refs #2519, #2516, #2520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant