test(hostlock): run the advice we print, instead of reading a prefix of it

[justinchuby]

Holden flagged (#1926 thread) that Report::warning tells a benchmarker to run scripts/hostlock.sh run, which #1926 made exit 1 without --reason — advice aimed at exactly the person we most want to succeed, telling them to run something the tool refuses.

His diagnosis is right and his reading of current main is stale. --reason entered that string in #1926 itself (23af6afbc, git log -S), and the advice runs clean today:

$ scripts/hostlock.sh run --owner gaff-1 --reason "..." -- echo ADVICE-RUNS-OK
hostlock: outcome=acquired by gaff-1 (anchor pid 3807209) — ...
ADVICE-RUNS-OK          exit=0

What is real is that nothing holds it that way. Both advice sites are checked by asserting the substring "hostlock.sh run --owner" — a prefix that the accepted form and the refused form both satisfy.

The measurement

On 0e6d01d39. Stripping --reason from Report::warning:

test result: ok. 29 passed; 0 failed; 0 ignored
test result: ok. 11 passed; 0 failed; 0 ignored
test result: ok. 31 passed; 0 failed; 0 ignored
test result: ok.  1 passed; 0 failed; 0 ignored
test result: ok.  2 passed; 0 failed; 0 ignored

The whole crate stays green while the advice has become a refused command. The advice text is referenced by no file but window.rs itself — positive control run first, so the zero means absence rather than a dead grep.

The Python side is the sharper instance, because the existing test states the correct property in its own comment and then tests something weaker:

def test_the_remedy_points_at_the_outer_wrapper(self):
    # refusing is only useful if what it prints is the thing that would have worked.
    self.assertIn("hostlock.sh run --owner", self.GATE.read_text())

Same mutant, the two tests side by side:

	drop --reason from remedy()
new executable test	FAILED
existing prefix assertion	OK

The change

Two tests, no production code. Each pulls the command out of the real advice — Report::warning()'s return value and hostlock_gate.remedy()'s return value, not a copy and not the file's source text — fills its placeholders, and runs it through a shell from the repository root, which is what the reader of the message does. That also pins the relative path the message quotes.

Three details that are load-bearing:

• A control before the verdict. Each requires the extractor to find exactly one command. A reword that dropped the command entirely would otherwise leave nothing to run, and "ran nothing successfully" reaches the result line as a pass.
• Placeholders are asserted substituted, so a rename fires the assertion instead of handing the shell a literal <you>.
• HOSTLOCK_REASON and HOSTLOCK_OWNER are stripped from the child. The script accepts both as substitutes for the flags, so an inherited value would let advice that had lost a flag still exit 0 — the test would pass on the strength of this process's environment rather than of the string under test.

Neither test touches the shared lock: HOSTLOCK_DIR is redirected to scratch (CARGO_TARGET_TMPDIR / target/tmp, removed after), with HOSTLOCK_PRIVATE_OK=1 as the suite already does.

Mutation battery, both directions

mutant	Rust guard
drop --reason	FAILED — stderr quotes the script's own refusal
remove the command entirely	FAILED (control arm)
scripts/ → tools/	FAILED
drop --owner	ok

The last is a true negative, not a gap: --owner is genuinely optional, verified rather than assumed — hostlock.sh run --reason X -- true exits 0 and derives the owner from $USER. That mutant is still a command the script accepts, which is what this test claims to check.

Gates

cargo fmt --check clean · cargo clippy -p onnx-runtime-hostmon --all-targets -- -D warnings exit 0 · cargo test -p onnx-runtime-hostmon 12/12 in the agrees_with_hostlock_sh binary (was 11), 0 ignored · python3 -m unittest scripts.ort_ab.test_gate_conformance 123 tests OK (was 122).

onnx-runtime-hostmon is in offline-linux, offline-cross-platform and lint, so these run — checked with workspace_test_packages.py cargo-args, after my first attempt at that query returned hostmon=0 for every lane because I passed --lane to a positional argument and was counting a usage error. Second dead instrument this session; the control caught both.

Limit

This pins two advice strings to the script's exit status. It does not check that the advice is the right advice — a remedy that suggested a valid command which locked the wrong scope would pass. That property is what test_the_remedy_points_at_the_outer_wrapper is for, and it stays.

Reported by Holden. Related: #1926, #1817.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.76%. Comparing base (743bd7c) to head (45e1088).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2289      +/-   ##
==========================================
- Coverage   81.19%   80.76%   -0.44%     
==========================================
  Files         434      434              
  Lines      222250   222250              
  Branches   222250   222250              
==========================================
- Hits       180452   179494     -958     
- Misses      35869    36826     +957     
- Partials     5929     5930       +1     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (ø)
cli-ort-windows	72.01% <ø> (-0.10%)	⬇️
mlas	85.80% <ø> (-0.10%)	⬇️
offline	80.88% <ø> (-0.45%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 11 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.