Skip to content

test(xtask): make skipped fidelity cells a counted, reason-carrying outcome - #7248

Draft
oferchen wants to merge 2 commits into
masterfrom
fix/xtask-validate-skip-ledger
Draft

test(xtask): make skipped fidelity cells a counted, reason-carrying outcome#7248
oferchen wants to merge 2 commits into
masterfrom
fix/xtask-validate-skip-ledger

Conversation

@oferchen

@oferchen oferchen commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Problem

cargo xtask validate counted skipped cells but let them pass. A run that
exercised 5 of 239 cells printed

=== 5 passed, 0 failed, 50 skipped ===

and exited 0. Nothing in the output or the exit status distinguished that
from a healthy run; the totals line simply got smaller.

The model

Upstream does not ban skipping - it makes a skip impossible to mistake for a
pass. testsuite/rsync.fns:448:

test_skipped() {
    echo "$@" >&2
    echo "$@" > "$tmpdir/whyskipped"
    exit 77
}

Three properties carry over: the reason reaches the operator, the reason is left
behind in a machine-readable artifact, and the outcome is distinct from both
pass and fail. What upstream leaves to the caller - noticing the skip count has
grown - is made a hard contract here, in the spirit of
tools/ci/known_failures.conf: a declared number, exceeding it fails the run.

What landed

  • SkipLedger collects every skipped cell with its reason, groups reasons by
    frequency, and prints the count against its declared budget under a
    === skipped cells === heading.
  • A whyskipped artifact is written to the work dir, one tab-separated
    check<TAB>cell<TAB>reason record per skip, named after upstream's file. It
    is written even when nothing skipped - an empty file says "accounted, none",
    which a missing file does not.
  • ExpectedSkips::DEFAULT is the declared budget. Exceeding it fails the run
    with a diagnostic naming the count, the budget, and every reason with its
    cell count.

The measurement that changed the design

Re-deriving the numbers from the raw logs corrected an earlier claim of mine
and exposed a hole in the first version of this gate.

A skip budget alone cannot catch the macOS defect (#371). Measured:

host cells accounted pass fail skip
BSD touch (stock macOS) 55 5 0 50
GNU touch on PATH 239 105 7 127

The stock-macOS run does not skip 184 cells - those cells stop existing.
When a check's fixture setup fails, some checks collapse the transports they
would have run into a single aggregate skip, so the cells are counted nowhere:
not passed, not failed, not skipped. A skip ledger never sees them.

Separately, the 127 skips in the GNU-touch run are not the touch problem at
all - 118 of them are no sshd on localhost:22. The touch defect and the skip
volume are two different things, and conflating them was an error in my earlier
report.

So the accounting contract is two gates, not one:

  • ExpectedSkips::DEFAULT - the skip budget.
  • EXPECTED_CELLS - a floor on cells accounted for, checked separately. A
    floor rather than an equality because --flags and --edge-cases
    legitimately add cells; nothing legitimately removes them. Not applied to a
    narrowed run (--transport), which produces fewer cells by design.

Without the second gate this change would have shipped a check that appears to
validate and does not - the exact class it exists to close.

Proof

All three scenarios surface as counted, reason-carrying outcomes and trip a
gate. Under cargo nextest run --workspace --all-features -E 'test(skips::)':
12 tests run: 12 passed.

Both directions. Neutering exactly the two production gates (exceeds_budget
always false, cell_shortfall always None) turns exactly the three tests that
assert them red:

12 tests run: 9 passed, 3 failed
  FAIL exceeding_the_declared_count_trips_the_gate
  FAIL the_macos_shape_is_over_budget_and_names_its_dominant_cause
  FAIL vanished_cells_are_caught_by_the_floor_not_the_skip_budget

Restored: 12/12.

End-to-end on the real macOS shapes - the run that previously exited 0 now
exits 1:

A. stock macOS (BSD touch)          exit=1
   === 5 passed, 0 failed, 50 skipped ===
   skipped: 50 of budget 10  << OVER BUDGET
   cells accounted: 55 (floor 239)
   matrix accounted for 55 cell(s), but the full selection declares 239 -
   184 cell(s) produced no outcome at all.

B. GNU touch on PATH                exit=1
   === 105 passed, 7 failed, 127 skipped ===
   cells accounted: 239 (floor 239)          <- floor clears
   skipped: 127 of budget 10  << OVER BUDGET
      114  no sshd on localhost:22
        4  no sshd
        2  id remap needs root
        1  daemon module boundary blocks -L on a symlink pointing outside the module
        ...
   skip detail: target/validate/whyskipped   <- 127 records

Verification

cargo fmt --all -- --check clean. cargo clippy --locked --workspace --all-targets --all-features --no-deps -- -D warnings clean. No CI (outage);
local only, on macOS/aarch64.

Stated limits

  • EXPECTED_CELLS = 239 is measured on macOS/aarch64 only. If Linux
    legitimately produces a different cell count the floor will fire there and the
    constant needs re-measuring, possibly per-platform. This is called out in the
    doc comment on the constant.
  • xtask validate runs in no CI workflow. These gates fire when the matrix
    is run, which today is a local pre-push step only. The nextest tests keep the
    gate logic honest in CI, but nothing runs the matrix itself in CI. That is a
    separate gap worth its own task.
  • Optimize rolling checksum multi-byte updates #371 is not fixed here, per the instruction not to let the cell fix delay
    the gate. It is now loudly red instead of silently green. Sizing for whoever
    picks it up: the touch shell-out is not one helper - "touch" appears
    across 42 files under xtask/src/commands/validate/, so moving to the
    filetime crate is a 42-file change, not a one-liner.
  • The reason vocabulary has minor duplicates (no sshd on localhost:22 vs
    no sshd), which splits one cause across two rows in the grouping. Cosmetic,
    left alone.

@github-actions github-actions Bot added the test label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant