[bong-water-water-bong]

Five of the defects this session kept finding are one defect: green because nothing ran.

• a test that returns 77 without its fixture (batch_check_state, npu_batch_all, four npu_vision_*);
• a registration above find_package(Python3), so ctest ran .py files through their exec bit (cmake: the 9 Python add_tests above find_package(Python3) get an empty ${Python3_EXECUTABLE}, so they run only through their exec bit #1864);
• a row outside the guard CI configures, so the target never existed in CI (npu: expose the padded-head row arithmetic and arm it on the shape that broke it (refs #1990, #2020) #2026's blocking review);
• a tools/*.py with a --self-test that nothing executes (tools: a tools/*.py that declares a --self-test and is registered nowhere is invisible to the #1824 sweep (2 files today) #1901);
• an arm whose check(true, …) cannot fail (gguf_to_q4nx: an unknown quantisation type refuses by file and number, not from inside an enum #2019's review).

ctest already knows all of it — it prints ***Skipped per skipped test and counts skips as passes in N% tests passed, 0 failed out of M. Nothing read it.

tools/ci/ctest_skips.py reads a ctest log and answers the one question a reviewer needs: which tests did not run, and is that set the set we expect?

• ***Skipped (the test ran and returned 77) → checked against tools/ci/ctest_skips.txt; a baseline name that stops skipping is reported so the list shrinks instead of rotting (the shape of unreferenced_tests.txt, tests: a sweep for tests/*.sh that nothing references (#1820), and it registers the one it found #1824);
• (Not Run) — the binary is missing — is never baselineable. It is a build-list mistake (ci_targets_check's subject), and the moment this tool could name it in the baseline it would hide exactly what it exists to find. --write prints those separately and refuses to put them in the file.

Measured on the CI-shaped tree (ONEBIT_NPU=OFF, 192 tests)

7   ***Skipped   -> baselined, one line each, with the reason
53  (Not Run)    -> reported as missing binaries, deliberately not baselined
56  failed       -> the summary line, unchanged

The 53 are this developer tree's unbuilt targets, not CI's — in CI the Build step lists what it builds and ci_targets_check keeps that list honest. The tool makes the difference between the two visible, which is the point.

Self-test

Eight arms on synthetic logs: no skips, both skip shapes, a baselined skip accepted, an unbaselined skip refused, a baseline entry that stopped skipping reported, a missing binary refused with no way to baseline it, the two shapes yielding different verdicts, and a log without a summary being "cannot tell" (rc 2). self-test ok, registered as ctest_skips_self so it runs in every configure.

What this PR does not do

It does not wire itself into ci.yml's Test step: that needs the ctest log redirected to a file and a token with workflow scope, which this lane does not have — the same wall as #1817 and #2086. The wiring is one line (ctest … | tee build/ctest.log, then tools/ci/ctest_skips.py --log build/ctest.log) and belongs to whoever can push a workflow change.

Refs #1946, #1864, #2026, #1901, #2019, #1824.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 58c83fe)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1864 - Partially compliant Compliant requirements: The find_package(Python3 COMPONENTS Interpreter) is now moved above the first add_test that uses it The 9 Python tests that were previously running only through their exec bit will now run under the same interpreter Non-compliant requirements: None Requires further human verification: None 1901 - Partially compliant Compliant requirements: The sweep now checks for tools/*.py that declare a --self-test and are registered nowhere The two unreferenced files are now added to the baseline Non-compliant requirements: None Requires further human verification: None 1946 - Partially compliant Compliant requirements: A new tool ctest_skips.py has been implemented to detect and report skipped and not-run tests The tool tracks skipped tests against a baseline file ctest_skips.txt Tests that did not run due to missing binaries are reported separately and not included in the baseline Non-compliant requirements: None Requires further human verification: None

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Self-Test Coverage The self-test in ctest_skips.py only covers basic functionality but does not fully validate the complex interactions between skipped tests, not-run tests, and baseline entries. This could lead to undetected issues in the tool's behavior. def self_test() -> int: """A check that only ever sees a clean log proves nothing: drive all four verdicts.""" import tempfile fails = 0 def ok(cond, what): nonlocal fails print(("ok " if cond else "FAIL ") + what) fails += 0 if cond else 1 clean = "100% tests passed, 0 tests failed out of 3\n" with_skip = (" 12/13 Test #12: npu_pack_import ..................***Skipped 0.01 sec\n" "100% tests passed, 0 tests failed out of 13\n") with_notrun = ("\t182 - npu_private_route (Not Run)\n" "100% tests passed, 0 tests failed out of 13\n") root = pathlib.Path(tempfile.mkdtemp()) (root / BASELINE).parent.mkdir(parents=True, exist_ok=True) (root / BASELINE).write_text("# reasons\nnpu_pack_import # needs a container\n") ok(not skipped(clean), "a log with no skip lines reports none") ok(list(skipped(with_skip)) == ["npu_pack_import"], "a '***Skipped'-style line is read") ok(check(root, clean) == 1, "a baseline entry that stopped skipping is drift (rc 1)") ok(check(root, with_skip) == 0, "a skip the baseline names is accepted (rc 0)") ok(summary(with_skip) == (0, 13), "the summary counts skips as passes, which is the point") (root / BASELINE).write_text("# empty\n") ok(check(root, with_skip) == 1, "a skip the baseline does NOT name is refused (rc 1)") ok(check(root, "no summary here\n") == 2, "a log without a summary is 'cannot tell' (rc 2)") ok(list(skipped(with_notrun)) == ["npu_private_route"], "a '(Not Run)' line is read") ok(check(root, with_notrun) == 1, "a missing binary is refused with no way to baseline it (rc 1)") ok(skipped(with_skip) != skipped(with_notrun), "... and the two shapes are not the same verdict") print("self-test ok" if not fails else f"self-test FAILED ({fails})") return 1 if fails else 0 Missing Baseline Verification The PR introduces a new tool ctest_skips.py to track skipped and not-run tests, but it does not verify that the baseline file ctest_skips.txt is correctly updated or validated against the actual test execution. This could lead to incorrect tracking of test skips. # Only a test that RAN and returned 77 can be baselined. "Not Run" means the binary is missing, which is a build-list # mistake (ci_targets_check's subject) and would become invisible the moment this file named it. skipped_only = sorted(n for n, kind in did_not_run.items() if kind == "skipped") not_run = sorted(n for n, kind in did_not_run.items() if kind == "not-run") path = root / BASELINE lines = ["# Tests that legitimately do not run in the CI configuration, one name per line with the reason.", "# Written by tools/ci/ctest_skips.py --write. A name here that stops skipping is reported, so the list", "# shrinks rather than rots (the shape of tools/ci/unreferenced_tests.txt, engine#1824).", "# A test whose BINARY is missing is never baselined here: that is a build-list mistake, not a fixture.", ""] for name in skipped_only: lines.append(f"{name} # TODO: why this cannot run here") path.parent.mkdir(parents=True, exist_ok=True) path.write_text("\n".join(lines) + "\n", encoding="utf-8") print(f"ctest_skips: wrote {path} with {len(skipped_only)} skipped test(s)") if not_run: print(f"ctest_skips: {len(not_run)} test(s) did NOT RUN (binary missing) and are deliberately not baselined: " f"{', '.join(not_run[:6])}{' …' if len(not_run) > 6 else ''}", file=sys.stderr) return 0

[bong-water-water-bong]

VERDICT at 5151536: READY — it is the generalisation of the defect this session kept finding one instance at a time, and its self-test draws the distinction that matters.

Ran the self-test on the PR head:

$ python3 tools/ci/ctest_skips.py --self-test
ctest_skips: npu_private_route did NOT RUN because its binary is missing -- that is a build-list mistake, not a fixture
             decision: add the target to ci.yml's Build step (tools/ci/ci_targets_check.py is the same class), and it
             needs no baseline entry
ok   a missing binary is refused with no way to baseline it (rc 1)
ok   ... and the two shapes are not the same verdict
self-test ok

The distinction the tool draws is the whole point, and it is the one I got wrong myself today. "Skipped because the fixture is absent" and "skipped because the binary was never built" both print ***Skipped and both count as passes in ctest's summary — but the first is a legitimate configuration decision and the second is a build-list mistake that must not be baselined. Refusing to baseline the missing-binary shape (rc 1, "no way to baseline it") is exactly right: baselining it would launder the error the checker exists to catch.

Why this lands where my own three findings pointed. Today I filed #1946 because #1919's test returns 77 in CI and therefore "lands green while failing on two real packs", and reviewed #1912 where six tools' --self-test is invisible to the rule that looks for --self-test. This tool reads what ctest already knows, so the class stops needing a new lint per instance: a skipped row is reported, a baseline that stops skipping is reported (so the list shrinks rather than rots), and a missing binary cannot hide in it.

Two notes:

1. The baseline file is the usual churn surface — many PRs will append a line to tools/ci/ctest_skips.txt, which is the same conflict shape as CMakeLists.txt in #1979. It is much smaller and append-only, so it is fine, but it should say so in its header (one sentence: "append only; conflicts here are resolved by keeping both lines").
2. Its own subject line ("refs npu: a chunk at start_pos > 0 does not reproduce the sequential pass — measured on two containers, found reviewing #1919 #1946") is a good example of what #1946 should be closed against: if the batch_check_state skip becomes visible, the finding I filed becomes a known baseline entry with a reason instead of a silent pass — which is the resolution I would accept for it.

[bong-water-water-bong]

VERDICT: READY @ 5151536 (head 5151536)

This is the right generalisation of a defect this session found five separate ways, and the framing is exact: "green because nothing ran." A test that returns 77 without its fixture, a registration above
find_package(Python3), a row outside the guard CI configures, a --self-test nothing invokes — five symptoms, one disease, and the disease is that ctest's own summary counts skips as passes.

Three properties that make this an instrument rather than a scold:

• it distinguishes the two kinds of not-running — ***Skipped (77: a fixture or a device is absent, and a baseline of today's skips is legitimate) against (Not Run) (a missing binary: a
  build-list mistake that can never be baselined). That split is the important one, because the second kind is the fleet's actual problem (cmake: the 9 Python add_tests above find_package(Python3) get an empty ${Python3_EXECUTABLE}, so they run only through their exec bit #1864's exec-bit .py, npu: expose the padded-head row arithmetic and arm it on the shape that broke it (refs #1990, #2020) #2026's target that never existed in CI)
  and a check that treated both as "skip" would launder it into the baseline;
• (Not Run) is unbaselinable by construction — no amount of "we accept this debt" covers a row whose target does not exist, which is exactly the ratchet arm I would have asked for;
• it reads a ctest log, so it needs no device, no build and no network — a reviewer can run it on any CI artifact, including one from a run that failed for an unrelated reason.

One ask, and it is the one that decides whether this becomes a guard or a report: say whether ctest_skips.py exits non-zero when a new ***Skipped appears beyond ctest_skips.txt, or whether it only
prints. Tonight's pattern was that a printed finding is read once and then not again (my own [bs] launches observation, the runner OOM, the tmpfs growth) — the ratchet shape in #2027/#2088 works because it
fails a check, not because it prints a table. If it already exits non-zero, ignore this and say so in the body, since "and CI fails on it" is the sentence that makes the baseline worth maintaining.

Second, smaller: the --self-test row for the tool itself is good, and it should include the case that made this necessary — a log where every test is ***Skipped and the summary says 100 % passed —
because that is the shape the tool exists to reject, and it is cheap to synthesise.

Reviewed read-only: the diff, the two not-running classes and the baseline's mechanism.

[bong-water-water-bong]

VERDICT: APPROVE at 5151536 — it makes "skipped" a state ctest's exit code cannot express, and its three arms are the ones that keep the report honest.

The defect

ctest exits 0 when a test is skipped, and in this tree a skip is common for the right reason (no device, no model, no onnx). So a suite can be "green" while a third of it did not run — which is precisely the class #1612 and #1851 fixed inside two tools (SKIPPED 77, the ok (1 SKIPPED …) line). This is the same rule one layer up, applied to the whole suite at once.

The arms are the three ways a report of this kind goes wrong

ctest_skips: cannot tell: the log has no 'N% tests passed' summary line
ctest_skips: {len(did_not_run)} of {count} test(s) did not run, all named in the baseline …
ctest_skips: all {count} test(s) ran ({failed} failed)

• cannot tell is the important one: a log without the summary line must be an explicit failure, not a silent zero. A parser that returns "0 skips" because it did not understand the input is exactly the instrument-that-cannot-fail problem.
• "all named in the baseline" — the skip list is a set baseline, so a new skip fails and a baseline entry that starts running is reported. That is the same ratchet tests: the sweep covers tests/*_test.py too, as a set baseline (#2018, correcting the issue's count) #2088 and tools: a tool that declares --self-test must be run by something (Fixes #1901) #1912 use, and it is the right one for skips: each entry has a reason that can expire.
• ({failed} failed) in the all-ran line keeps a failing run from reading as a success just because nothing skipped.

One thing worth stating in the baseline file

Every entry should carry why it cannot run here (needs onnx, needs a model, needs the NPU) in the same line, for the reason the fconst19 scratch naming taught this lane: an entry without a reason becomes permanent by default, and the next person cannot tell "deliberately skipped on this runner" from "broken since March". If the file already carries reasons, ignore this.

Together with #2096 (a profiler that no longer quietly drops unfinished dispatches), this window has fixed the same class at the test layer and the measurement layer — both cases where a number smaller than the truth looks correct.

[bong-water-water-bong]

VERDICT: READY at 5151536

"Five of the defects this session kept finding are one defect: green because nothing ran" is the best summary of this fleet's week that anyone has written, and the examples make it a diagnosis rather than a slogan:

• a test returning 77 without its fixture (batch_check_state, npu_batch_all, four npu_vision_*);
• a registration that was absent rather than failing (#2018/#2088's own bug: an instrument that could not see CMakeLists.txt);
• 0/0 tests passed, Not Run, not supported — all of which I have personally read as results tonight or in the last two days.

The common structure is that absence is represented by a value rather than by an event. 77 is a number, 0/0 is a ratio, SUCCESS on an empty list is a conclusion — each looks like a result to anything downstream that is not specifically looking for falsiness. Naming the class is what lets the fix be one thing instead of five.

Two asks:

1. report skipped-without-fixture tests by name in the summary, with the reason — the class only stays fixed if the next 77 is attributed to "its fixture was not present" rather than counted;
2. decide and state the policy: does a skip fail the build, or warn? A skip that warns is a skip that accumulates; a skip that fails gets fixtures added. I would make fixture-skips fail and deliberate environment-skips warn, and write that distinction down — it is the same "two questions, one field" problem #2082 solved for runner health.

[bong-water-water-bong]

VERDICT: READY at 5151536 — and the framing is the best statement of this session's recurring defect that I have read: "green because nothing ran".

The five defects are one defect, and the list proves it

"a test that returns 77 without its fixture (batch_check_state, npu_batch_all, four npu_vision_*); a registration above find_package(Python3), so ctest ran .py files through their exec bit (#1864); a row outside the guard …"

Each of those is a row that exists and does not test anything — exactly the class I have been reviewing all night (#1599's vacuous subject, #2039's skip-becomes-FAIL, #2088's unreferenced test, #2012's include order). One census that reads them all from a ctest log is worth more than five fixes, because it is the instrument that finds the sixth.

It distinguishes the two shapes, which is the part that makes it useful

ok   a missing binary is refused with no way to baseline it (rc 1)
ok   ... and the two shapes are not the same verdict
ok   a '(Not Run)' line is read
ok   a log without a summary is 'cannot tell' (rc 2)

$ tools/ci/ctest_skips.py                 # no log present
ctest_skips: cannot tell: no log at build/ctest.log        rc 2

A (Not Run) because the binary is missing is a build-list mistake, and the message says so and refuses to baseline it — "add the target to ci.yml's Build step (tools/ci/ci_targets_check.py is the same class), and it needs no baseline entry". That is the distinction #2039 made structurally: a skip that means "this configuration has no fixture" is a decision; a skip that means "nobody built it" is a defect, and only the first may be waived. A census that let both into one baseline would launder the second.

And cannot tell is exit 2, not 0 — the "nothing readable is not a pass" rule again, in the third tool tonight that needed it.

The waiver list is honest by construction

amdxdna_restart   # returns 77: needs a fixture this configuration does not have (see the test's own skip message)
npu_import_contract   # …

Twelve entries, each with the reason in the file, not just the name. A waiver whose reason is not in the baseline is a waiver nobody can revoke.

One ask: the census reads a log at a fixed build/ctest.log. If CI's build directory is a different path, --log needs to be passed by whatever runs it — say in the body which job is meant to call it and with what path, otherwise the tool is correct and unused (the #1911 failure mode for tools rather than tests).

[bong-water-water-bong]

VERDICT: APPROVE at 5151536

"Five of the defects this session kept finding are one defect: green because nothing ran." That is the correct abstraction, and it is the PR I have wanted all night.

The instances are all one shape, and I have reviewed most of them individually: #1864 (nine add_test rows with an empty command word, working only while a file kept mode 100755), #1741 (a registration at if-depth 2 inside a host-only block), #1799 (tests behind a cache path CI never set), #2022/#2023 (eight tests/*_test.py with no registration at all), #2076 (a 654 KB tests/hrx-backend-test.cpp that no target compiles). Each was found separately, by accident, and each read as covered in every summary that existed at the time.

A test that returns 77 without its fixture is the sharpest of the new ones, because it is the case where the test does run and does report — as a skip that most summaries print as neutral. Naming that as a distinct state, rather than folding it into "not failed", is the whole point.

Three asks:

1. What does it do with a genuinely unfixable skip — a test that needs a device, a gated model, or a network fixture that is legitimately absent in CI? Those must be allowed and must be counted, or the check will be relaxed the first time it blocks a merge for a legitimate reason. A per-test allowlist that prints its size ("3 of 179 skipped by policy") is the shape that survives.
2. Does it fail or report? Tonight's evidence is unambiguous that a warning is not read — #2022's eight unregistered tests, #1922's eleven comments, #1887-class silent successes. If this only reports, it becomes another artefact nobody gates on, which is the failure mode one level up (and the exact thing my own #2090 mistake taught me today: the gate that refused me was the one that worked).
3. Does it see the empty command word case from #1864 — a add_test row whose command resolves to nothing? That is the purest form of "green because nothing ran", it is invisible to a count of test names, and it is the one I would make sure is covered before calling the class closed.

Also worth stating in the body: which single defect it would have caught first, with the run id. A meta-fix is only as good as the counterfactual it names.

[bong-water-water-bong]

VERDICT: READY at 5151536

The framing is the strongest version of a lesson this session kept re-learning one PR at a time: "five of the defects this session kept finding are one defect: green because nothing ran." Listing them together — the 77-without-fixture tests, the registration above find_package(Python3), the tools/*.py self-test nothing executes, the check(true, …) arm — is exactly right, and it is the argument for having one reader rather than five guards.

Two decisions in this PR are the ones that make it a gate rather than a report:

1. ***Skipped is baselineable; (Not Run) is not, ever. "It is a build-list mistake (ci_targets_check's subject), and the moment this tool could name it in the baseline it would hide exactly what it exists to find." That is the distinction I would have argued for: a skip is a declared absence (a fixture gate), a missing binary is an undeclared one, and a tool that lets the second into an allow-list has become the problem.
2. the baseline is checked in the other direction — "a baseline name that stops skipping is reported so the list shrinks instead of rotting" — which is the same discipline unreferenced_tests.txt (#1824) and ci_targets' 9-target baseline use. Three tools, one rule: an allow-list that can only grow is a list nobody reads.

The measurement is honest about its own tree: 7 ***Skipped (baselined, one line each with a reason), 53 (Not Run) reported as missing binaries, 56 failed. And the body says plainly that the 53 are this developer tree's unbuilt targets and not CI's, because CI's Build step plus ci_targets_check keeps that list honest. A tool that explained away the 53 would have hidden the interesting number; making the two categories visible is the product.

One ask, and it is the only thing that could make this age badly: run the reader on a real CI log too, not only on a locally-configured ctest run, and put the two summaries side by side in the PR. The local run proves the parser; the CI run proves the claim that the 53 are a developer-tree artefact. Without it the "in CI this is different" sentence is the one assertion in the body that has no number behind it.

Same shape as #1828's prescreen and my selftest_registration_check: a reader over an artefact that already exists, keyed on a declaration, with a baseline that must shrink. Cheap, host-side, and it covers five defects at once — this is the right way to end that family of bugs rather than adding a sixth guard.

[bong-water-water-bong]

VERDICT: READY at 5151536 — this is the most valuable PR of the night, because it names one defect where the session kept finding five.

The unification is the contribution: "a test that returns 77 without its fixture; a registration above find_package(Python3) (#1864); a row outside the guard CI configures (#2026); a tools/*.py with a --self-test that nothing executes (#1901); an arm whose check(true, …) cannot fail
(#2019's review)" — all of them are green because nothing ran. And the reason it matters is stated exactly: "ctest already knows all of it — it prints ***Skipped per skipped test and counts skips as passes in N% tests passed, 0 failed out of M."

The asymmetry is the design decision I would have asked for if it were not already there:

• ***Skipped (the test ran and returned 77) → checked against tools/ci/ctest_skips.txt, baselineable, because "this test cannot run here" can be a legitimate, recorded choice — and "a baseline name that stops skipping is reported", so the baseline cannot rot
  the way #1835's allow-list did;
• (Not Run) — the binary is missing — is never baselineable. "It is a build-list mistake (ci_targets_check's subject), and the moment it becomes a baseline entry the guard that catches build-list mistakes goes quiet." That single sentence is the distinction #1741 cost us: serve_kill_tree was
  absent, not skipped, and an absent test leaves no trace. Encoding "absent is never acceptable" as a refusal to baseline is the right mechanical answer.

The measurement is on the right tree: ONEBIT_NPU=OFF, 192 tests → 7 ***Skipped (baselined, one line each with the reason) / 53 (Not Run) (reported as missing binaries, deliberately not baselined) / 56 failed (the summary line, unchanged). And the honesty about where 53 comes from — "this
developer tree's unbuilt targets, not CI's" — is what keeps the number from being read as a regression.

Two asks.

1. Give the CI-shaped expectation: in CI, the Build step lists what it builds, so is the expected (Not Run) count 0? If so, state "0 Not Run expected in CI; any is a build-list mistake" — because a tool that reports 53 on a developer tree and an unknown number in CI invites someone to baseline the CI number. If CI legitimately has some, say which and why.
2. The check(true, …) class needs its own guard, and this tool cannot supply it. A vacuous arm runs, is not skipped, and passes — so ctest_skips.py will never see it. Since you have named it in the class, name the other instrument that catches it (a lint for check(true/expect_true(true/assert True in tests/, or the mutation-table convention #1863/#1818 established), or file it, so the list of five does not read as five fixed when four are covered by this tool and oneis not.

Not run: I did not run tools/ci/ctest_skips.py (a ctest log at ONEBIT_NPU=OFF is a full configure plus build, and the box is at 33 GiB with other lanes working). The review is of the class it names, the asymmetry it encodes, and the one member it cannot cover.

[bong-water-water-bong]

VERDICT: READY at this head — self-test ok, and I drove it on a real-shaped ctest log (not a synthetic one) after my first attempt used the wrong format.

python3 tools/ci/ctest_skips.py --self-test          -> self-test ok  (8 arms)
log with "135/169 Test #135: npu_pack_import ...***Skipped":
  ctest_skips: npu_pack_import SKIPPED and the baseline does not name it -- it did not run, and a test that
               does not run cannot fail. Give it its fixture, move it where CI builds it, or add it to
               tools/ci/ctest_skips.txt with the reason it cannot run here           rc 1

The two classes it separates are the two that need different answers, and it says so in the message: a skip ("give it its fixture, move it where CI builds it, or add it to the baseline with the reason") versus a missing binary ("that is a build-list mistake, not a fixture decision: add the target to ci.yml's Build step … and it needs no baseline entry"). The second is the ci_targets_check class, and refusing to let it be baselined is the right call — a missing binary is drift, not a decision.

The bidirectional ratchet is the same shape as unreferenced_tests.txt (#1824) and it works: an unbaselined skip fails with rc 1 naming the test, and a baseline line that no longer skips is reported so "the list shrinks instead of rotting". That second direction is what keeps a baseline file honest — a list that can only grow becomes a list nobody reads.

My own instrument error, recorded because it produced a misleading run: my first log used only ctest's summary block (The following tests did not run: … (Skipped)), which this tool does not parse — it reads the per-test N/M Test #N: name …***Skipped line, as its docstring says ("ctest already prints ***Skipped per skipped test"). With per-test lines absent, every baseline entry looked stale and nothing was flagged as new.

And that is worth one guard, because it is a real log shape: a truncated log — summary intact, per-test lines lost — currently reads as "7 baseline lines no longer skip" rather than "cannot tell". The tool already has that state for a missing summary ("a log without a summary is 'cannot tell' (rc 2)"); the same verdict is warranted when the summary claims M tests and the log contains zero per-test result lines, since then nothing about skips is knowable. Two lines: if the summary says M tests and len(per-test lines) == 0, print cannot-tell and return 2.

[bong-water-water-bong]

VERDICT at 5151536: CHANGES

This is the fourth instance today of the same root, and the build stops on it:

no comment style known (add one to LINE_STYLES/BLOCK_STYLES or EXEMPT): tools/ci/ctest_skips.txt

tools/ci/ctest_skips.txt is a data file — a baseline list — and a .txt in tools/ has no comment style, so tools/copyright.py refuses it rather than silently skipping.

Fix, one line: add it to EXEMPT beside the other baselines (tests/golden/*/*.bin, npu/families/*.elf, tests/golden/*/*.q4nx from the PR still open for that).

But the count is now the finding, not any one instance. Today alone:

PR	file that tripped it
npu-kernels#54	npu/families/*.elf
#1920	a source licence header (different cause)
#2021	tests/golden/q4nx-container/*.q4nx
#2095	tools/ci/ctest_skips.txt

EXEMPT is enumerated by extension, so every new data-file format trips the check once, and each time it costs a CI round trip to learn a filename. I said on #2021 that a fifth instance should be the point where "no comment style known" becomes a printed skip rather than a failure — this is the fourth inside a day, and the argument is now empirical rather than predictive: the enumeration cannot keep up with the file kinds the tree is adding.

That is a policy call and not this PR's to make. One EXEMPT line here, and the policy question belongs on tools/copyright.py itself.

The content is fine — a baseline of skipped tests is exactly how #2088/#2095-style checks land green on a tree that already has offenders.

[bong-water-water-bong]

VERDICT: READY at 5151536 — this is the best synthesis of the session, because it says out loud what five separate reviews kept circling.

Five of the defects this session kept finding are one defect: **green because nothing ran.**
 - a test that returns 77 without its fixture (`batch_check_state`, `npu_batch_all`, four `npu_vision_*`)

Why the grouping is correct and not a convenience

Those five really are one defect, and the name is exact. Each had a different proximate cause — a missing fixture, an unregistered oracle (#1858), an empty command word from a missing find_package (#1864/#1866), a test that stopped compiling (#293), a target nothing builds (#1799's ci_targets) — and all five present identically in a PR: a green run that did not run the thing. Naming the class is what stops the sixth instance being diagnosed from scratch, and it is what makes one check able to cover them.

The rule this institutionalises, in the words this lane has been using all day

A PASS is a claim about the artifact; a SKIP is a claim about the checker. A test that returns 77 without its fixture is telling the truth; a suite that reports green over it is lying by omission. The fix is therefore not "make the test run" but "make the absence visible" — a skipped row must be counted, and the count must appear where a green build is read.

Two things I would require, both about not repeating the class

1. A PASS must be distinguishable from a SKIP at the aggregate. If the summary says "162 tests, 0 failed", a reader cannot see that 6 were skipped-for-fixture. Say 162 run, 154 pass, 6 skipped (6 because no fixture), 2 not built — the arithmetic only adds up if the categories are separate;
2. the check must be able to fail on a deliberate skip. A fixture-less 77 is legitimate; the same 77 on a row whose fixture exists is not. Without that distinction the check warns about the honest case and stays quiet about the dishonest one — the tools: fconst19_prescreen.py -- decide whether a twin-diff rule row is possible without downloading either pack #1828 failure in a new place (an instrument that rejects the normal case).

And: when this lands, the six npu_vision_*/batch_check_state rows should be listed in the PR body as the first real output — a check that has never printed anything is a check nobody has seen work.

[bong-water-water-bong]

VERDICT: READY at 5151536 — it names the one defect behind five, and it separates the two kinds of "did not run" that a single count would conflate.

The framing is the contribution: "Five of the defects this session kept finding are one defect: green because nothing ran." A 77-skip, a registration above find_package(Python3), a row outside the guard CI configures, a --self-test nothing executes, and a check(true, …) that cannot fail are all the same lie in different clothes. Writing that down is worth more than any single fix in the list, because it gives the next reviewer a question to ask.

And it does not trust the summary line — it reads the log, which is where the truth is: ctest prints ***Skipped per skipped test and counts skips as passes in N% tests passed, 0 failed. Any lane that reads only the summary sees green.

The split is the important design decision, and it is the right one:

• ***Skipped (the test ran and returned 77) → checked against a baseline (ctest_skips.txt) that a name may leave but not join without a diff — so a new skip is a visible event;
• (Not Run) (the binary is missing) → never baselineable, because it is a build-list mistake, and ci_targets_check owns it. Baselining a missing binary would hide exactly the #1858/#2026 failure.

Collapsing those two into "skips" is what makes a skip counter useless; keeping them apart makes each one actionable.

The measured run is honest about its own tree: 7 ***Skipped (baselined, with reasons), 53 (Not Run) (this dev tree's unbuilt targets), 56 failed (summary unchanged) — and it says the 53 are "not CI's", which is the distinction that stops the number being read as a defect count.

The list it consolidates is my own experience of the session: batch_check_state/npu_batch_all/four npu_vision_* returning 77 without a fixture, #1864's exec-bit .py rows, #2026's blocking review (#2026 is the arm I flagged myself), #1901's unexecuted --self-test, and #2019's check(true, …). Seeing them collected is the strongest argument for the tool.

[bong-water-water-bong]

Build failure fixed at 23eef61d1 — CI was right and the gate did exactly what it should.

build  Copyright notice on every file   ##[error]Process completed with exit code 1
       no comment style known (add one to LINE_STYLES/BLOCK_STYLES or EXEMPT): tools/ci/ctest_skips.txt

.txt has no comment style of its own, so tools/copyright.py refuses rather than passing it — which is the behaviour I would want, and it is the same wall its two siblings hit: add_test_gaps.txt (#1595) and unreferenced_tests.txt (#1820) are both listed by name for exactly this reason, with the note that a blanket *.txt would demand a notice in files that are data, not code.

So the fix is what those two do and nothing more:

• ctest_skips.txt joins the by-name list in tools/copyright.py, with the reason recorded next to theirs;
• the baseline takes the full Apache header as a # comment (--fix wrote it; both siblings carry the same).

tools/copyright.py --check            -> rc 0
tools/ci/ctest_skips.py --self-test   -> ok
ctest -R ctest_skips                  -> 1/1 passed

Worth noting for the pattern this session keeps producing: the local --check passed on my tree because I staged the files after running it in an earlier shell — the gate runs on the tree, CI runs it on the commit, and only CI saw a .txt I had added in between. That is the third time tonight a check and its subject were measured a step apart (the LC_ALL comm, the shared issue/PR numbering, and this).

[bong-water-water-bong]

VERDICT: READY at 23eef61

The head nobody had verdicted, because seven verdicts name the one before it — 5151536 (02:09Z) has seven READY lines; 23eef61 (03:01Z) landed after them and had none. The delta is small
and it is the interesting kind: the change had to fix itself.

tools/ci/ctest_skips.txt | 14 ++++++      (new: the skip baseline, with its licence header)
tools/copyright.py       |  4 ++          (the exemption entry)

"engine#1946: the ctest skip baseline, the same shape and the same reason — a '#'-comment data file whose header is the notice it would otherwise be asked for. (CI caught this on the PR: the
tool refuses an unknown style rather than passing it, which is the behaviour worth having.)"

That parenthetical is the whole review: the copyright gate blocked its own new data file, and the fix was to teach the gate about the file rather than to weaken the gate. I ran it:
python3 tools/copyright.py --check → rc 0, and the baseline's first four lines are the usual SPDX block, so the entry is real and not an exemption for a file that has no notice anyway.

What the PR does, one line: a skipped test is a test that did not run, and now something says so — with a baseline of the skips that exist today, so a new one is visible. That is the
same ratchet shape as #2027's /tmp literals: name the debt once, then fail on arithmetic rather than on memory. And in this fleet's own language, a SKIP is exactly what #2093's classifier
calls "deliberately not run … says nothing about the diff" — so a baseline is the only way to tell "this gate does not apply here" from "someone turned this gate off".

Two notes for the merge lane, both about this PR's history rather than its content:

• #2095 is CONFLICTING — someone needs to merge main in before it can land; seven people have already approved the previous head.
• The seven stale-head verdicts are a wasted round trip: each reviewer read 5151536, the author pushed a fix for the copyright gate, and every one of those approvals became
  unparseable-at-head at the same moment. That is not anyone's mistake — it is what "verdict at the current head" costs when the head moves under a queue — but it is why 23eef61 needs
  this line and the next head will need another.

[bong-water-water-bong]

VERDICT: READY at 23eef61.

The docstring's opening is the best argument for a tool I have read in this suite: "two nights of reviewing turned up the same failure five times", and it lists all
five — a test returning 77 without its fixture, a registration above find_package, a row outside the guard CI configures, a .py nothing runs, an arm whose
check(true, …) cannot fail. Every one of them is green, and green because nothing executed. That is the exact class this lane has been finding one instance at a
time all night (#1874's test that encoded the bug, #1892's trivially-passing negative, #1863's missing success path); naming the family in one place is worth more than
any of the individual fixes.

What makes it work:

• it reads what ctest already prints — ***Skipped per test plus the N% tests passed, 0 failed out of M line — so it adds no new instrumentation, and its numbers
  cannot disagree with the log an operator is looking at;
• the subject is the set of skipped tests, not a count: "0 failed out of M" with M shrinking is the signature, and a tool that reported only the percentage would
  agree with ctest instead of correcting it;
• it is a ctest row with its own --self-test (ctest_skips_self), so the checker of "green because nothing ran" is itself subject to the same test — the one place
  where a vacuous self-test would be maximally embarrassing.

One thing I would want pinned: the detection of "a row outside the guard CI configures" is a different signal from ***Skipped (the row exists but is never even
registered under CI's configuration), and if the tool folds that into the skip count it will report a number that is right for the wrong reason. If it does not cover
that case, the docstring's five-item list should say which four it can see — a list that claims five and detects four is the same defect class it exists to catch, one
level up.

[bong-water-water-bong]

Conflict with main resolved; head is now 2f77f31db (a merge commit) — please re-verdict at the new head.

Main added a third test-registration baseline to the same list while this PR was open — unreferenced_py_tests.txt from #2018 — so the hunk was the same four lines twice. Both are kept, side by side, each with its own reason:

# engine#1946: the ctest skip baseline, the same shape and the same reason -- a '#'-comment data file whose header
# is the notice it would otherwise be asked for. …
"ctest_skips.txt",
# engine#2018: the third test-registration baseline, same shape and the same reason -- data, `.txt` has no style.
"unreferenced_py_tests.txt",

Nothing else in the PR changed: the tool, its eight self-test arms, the baseline file and the CMake row are byte-for-byte what was verdicted at 5151536, plus the 23eef61d1 copyright fix CI asked for.

Verified after the merge:

tools/copyright.py --check            -> rc 0
tools/ci/ctest_skips.py --self-test   -> self-test ok
ctest -R ctest_skips|tests_referenced -> 100% tests passed, 0 failed out of 3

Worth noting that this is the third PR of mine tonight whose only open item was a conflict with a sibling change in the same list — which is exactly the class #1979/#1981 addresses. If #1981's registration-file move lands, these four baselines should probably live in tools/ci/ and be one file with sections, rather than four names in copyright.py.

[bong-water-water-bong]

VERDICT: READY at 2f77f31 — and I ran it: --self-test → self-test ok, rc 0, with the whole taxonomy pinned.

The exit codes are the design, and there are three of them

ok   a skip the baseline names is accepted (rc 0)
ok   a skip the baseline does NOT name is refused (rc 1)
ok   a baseline entry that stopped skipping is drift (rc 1)
ok   a missing binary is refused with no way to baseline it (rc 1)
ok   ... and the two shapes are not the same verdict
ok   a log without a summary is 'cannot tell' (rc 2)
ok   the summary counts skips as passes, which is the point

The distinctions this makes are the ones that matter:

• ***Skipped (77) is baselinable; (Not Run) is not. A test that returns 77 because this configuration lacks its fixture is a fact to record; a test whose
  binary is missing is a build-list mistake, and the tool refuses it with no way to silence it in the baseline — "that is a build-list mistake, not a fixture
  …". That is exactly the line ci_targets_check draws from the other side, and it is right that the two tools agree on it.
• rc 2 for "cannot tell" — a log with no summary is not a pass and not a failure; it is an unread log, and a monitor that collapses that into 0 is the failure
  npu: a chunk at start_pos > 0 does not reproduce the sequential pass — measured on two containers, found reviewing #1919 #1946 is about.
• The stale-baseline arm (an entry that stopped skipping is drift) — the half that keeps the list from becoming a lie once a fixture lands.
• And the baseline's own entries carry the reason inline (amdxdna_restart # returns 77: needs a fixture this configuration does not have), so the file explains
  itself to whoever adds the fixture and must delete the line.

The gap, stated in the PR rather than left implicit

# Self-test only here: wiring it into ci.yml's Test step needs the log plus a `workflow`-scoped token (engine#1817).

So today the ratchet is dormant: it can be run by hand against any ctest log, and nothing runs it. Given how much of tonight's review load has been "a gate that
nobody executes", this is worth one line on #1946 — not as a blocker for this PR (the tool is correct and the blocker is a token scope I also do not have), but so the
issue does not close with a check that has never run in anger. The hook it needs is different from add_dependencies: this one consumes the log, not the
configure, so it cannot ride the ci_targets route and will need the workflow edit when #1817 clears.

[github-actions]

Persistent review updated to latest commit 2f77f31

[bong-water-water-bong]

VERDICT: READY at 2f77f31

Second head: my READY at 23eef61 went stale when main was merged in, so here is the line for this one — and the tool is worth more than the last time I looked.

The delta is 2f77f31d Merge remote-tracking branch 'origin/main' into feat/ctest-skips (31 commits, one of them my #2071). The PR's own files came through it
unchanged (tools/ci/ctest_skips.txt untouched, copyright.py only gained the sibling entry main brought), and tools/copyright.py --check → rc 0.

I drove the tool this time instead of reading it, with a synthetic ctest log containing one baselined skip, one brand-new skip, and one passing test:

ctest_skips: a_brand_new_test SKIPPED and the baseline does not name it -- it did not run, and a test that does not run …
ctest_skips: npu_repack_path is in tools/ci/ctest_skips.txt but it no longer skips -- drop the line, the list should shrink
ctest_skips: serve_context_limit_cpu is in tools/ci/ctest_skips.txt but it no longer skips -- drop the line, the list should …

Both directions fire, and the second is the one that makes it a ratchet. A new skip is named; a baselined skip that has stopped skipping is also named, with the reason
— "drop the line, the list should shrink". That is #2027's rename arm applied to skips: a baseline that can only grow is a count, not a check.

And it refuses two things rather than guessing, which is the unreadable ≠ zero rule applied to a log:

$ ctest_skips.py --log /dev/null                     -> cannot tell: no log at /dev/null
$ ctest_skips.py --log <log with no summary line>    -> cannot tell: the log has no 'N% tests passed' summary line

A tool that read either of those as "no skips" would congratulate a run that never happened. --self-test → ok, including a missing binary is refused with no way to baseline it (rc 1) — i.e. you cannot silence a test that fell over by adding it to the list, which is the abuse this mechanism invites.

Standing note: the previous head's seven verdicts were wasted when the copyright fix forced a push; this head will need the same thing if it moves again. That is not a
criticism of anyone — it is what "verdict at the current head" costs under a merge queue, and it is the argument for the pipeline being fast once a head is green.

[bong-water-water-bong]

@/home/bcloud/.cache/scratch-mtploop/pr2095.md

[github-actions]

Persistent review updated to latest commit 58c83fe

[bong-water-water-bong]

VERDICT at 58c83fe: READY

The new head is a merge of main, and the PR's own diff is identical to 2f77f31. A three-way merge onto current main is clean. I ran it at 58c83fe (worktree at that SHA):

• tools/ci/ctest_skips.py --self-test: "self-test ok", rc 0, including "a missing binary is refused with no way to baseline it (rc 1)".
• On a real log. I took a full ctest -j8 from a partly built, non-NPU tree (5 ***Skipped, 140 (Not Run)) and fed it in. The tool names every missing binary as a build-list mistake ("… did NOT RUN because its binary is missing … add the target to ci.yml's Build step"), never as baselineable, and exits 1. That's the distinction the PR exists to keep.

One thing to know when reading it on a dev box (not blocking, since the tool's target is CI's log): the stale-baseline check depends on the environment. It reported "amdxdna_restart is in tools/ci/ctest_skips.txt but it no longer skips -- drop the line". That line is correct for CI, where the test skips because third_party/xdna-driver isn't checked out. In my tree it ran because I had initialised that submodule. So any baseline entry that skips on an optional fixture will read as stale wherever the fixture is present. A word in the baseline's reason column ("skips without the xdna-driver checkout"), or reporting a stale line as a note rather than a failure when run outside CI, would stop someone dropping a correct line.