[Kartikey1306]

Stacked on #127. Review the last commit. Closes the other half of the gap @srpatcha identified reviewing that PR.

test_cmake_test_registration.py answers "is this file compiled?". It cannot answer "is this test called?" — because a test function nothing invokes is valid C. It compiles, it links, ctest runs its suite, and the suite passes having skipped it. No build error, no linker error, no ctest failure, and no registration check can see it.

What it finds

Run against master, test_every_test_function_is_called() reports exactly one instance across every registered suite, with no false positives:

AssertionError: these test functions are defined in a suite that is built and
registered, but nothing calls them, so they do not run and cannot fail:
['test_kernel.c::test_queue_send_waiter_overflow']

That is the suite guarding the eos_queue_send / eos_queue_receive waiter table — "a full send/recv waiter table now returns EOS_KERN_NO_MEMORY instead of blocking a task that can never be woken". It has been defined and uncalled since it landed, so that fix has had no executing test.

Calling it, it passes. The fix was right; nothing was checking it. That is the good outcome — but it was luck, not evidence, and the point of this PR is that the difference was invisible.

The same class had already bitten eBoot, where a merge kept a test function and dropped its run_ call (eBoot#94).

Avoiding false positives

Two declaration styles are in use and both are handled:

style	called as
static void test_x(void)	test_x()
TEST(x)	run_x()

Occurrences are counted by bare identifier, not by name(, so a function passed by address — a task entry point, a callback — is not reported as dead. test_entry in this same file is exactly that case, and is correctly ignored. Comments are stripped first, so a name appearing only in prose does not read as a call.

There is an UNCALLED_OK escape hatch with the same discipline as NOT_BUILT, plus a staleness check so an exemption cannot outlive its reason.

The hardcoded count, again

test_kernel.c's summary printed === ALL KERNEL TESTS PASSED (12/12) === while thirteen tests sat in the file. The literal agreed with the omission instead of exposing it. It is counted now, by a RUN() macro that increments as it calls — the same shape as the tests_run fix eBoot#94 applied for the identical reason.

Validation

guard against master's tree   ->  1 failure, naming the right function
guard after the fix           ->  7 passed
test_kernel                   ->  13/13   (was 12 called, 13 defined)
pytest tests/                 ->  17 passed
ctest                         ->  39/39 passed

Follow-up, not in scope here

eBoot needs the same guard — it has no equivalent of this file at all, and it is the repo where the class has already caused a merge to silently drop a test. Happy to port it once this shape is agreed.

[srpatcha]

Review — eos#134 "test: catch tests that are compiled but never called"

head: 9361769 author: Kartikey1306 ci: fail (2 of 26 — both cross-repo, not this PR)

Verdict: Good change, and every claim in the Validation block reproduces exactly. The guard finds precisely one instance on master — test_kernel.c::test_queue_send_waiter_overflow, the suite guarding the eos_queue_send waiter-table overflow fix, which has had no executing test since it landed — with no false positives, and the tree is clean afterwards. The gap it closes is real and the framing ("the fix was right; nothing was checking it — that was luck, not evidence") is the correct one. One defect: the escape hatch cannot be used as its own error message instructs, and using it that way produces a second, actively false failure.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	tests/unit/test_cmake_test_registration.py:178-185 (with :139, :196)	UNCALLED_OK is keyed on the bare function name, but the failure message tells you to key it on file::function — and following that instruction fails twice. findings.append(f"{name}::{fn}") at :179 reports test_kernel.c::test_queue_send_waiter_overflow, and the message at :185 says "add each to UNCALLED_OK with a reason". But the lookup at :178 is if fn not in UNCALLED_OK — the bare fn. Executed against master's tree with this PR's guard, following the message literally: adding "test_kernel.c::test_queue_send_waiter_overflow": "known gap, tracked" → • test_every_test_function_is_called still fails — the exemption is inert; • test_uncalled_ok_has_no_stale_entries also fails, asserting these are called now and should leave UNCALLED_OK — which is false, the function is not called. With the key the code actually looks up ("test_queue_send_waiter_overflow") both pass. So a maintainer who does exactly what the tool tells them gets an exemption that does nothing plus a staleness error that states the opposite of the truth. Secondary consequence of the bare key: an exemption is global across suites, so exempting test_init in one file exempts it in every file. The docstring at :138 says UNCALLED_OK has "the same discipline as NOT_BUILT", but NOT_BUILT is keyed by file name with a prose reason and is exercised by real entries, whereas UNCALLED_OK is dict = {} — empty, untyped, and never exercised, which is why neither problem showed up.	Key on the same string you report, and make the staleness check agree. At :178-179: key = f"{name}::{fn}" if key not in UNCALLED_OK: findings.append(key) and at :190-196 build still_dead from the same composite key: still_dead.update(f"{name}::{fn}" for fn in _uncalled_in(path)) Then seed UNCALLED_OK with one worked entry (or a commented-out example in the documented format) so the format is exercised rather than assumed — the same reason NOT_BUILT's five real entries kept it honest. Add dict[str, str] to the annotation.
2	Low	tests/test_kernel.c:330-331	The replacement count still cannot expose a dropped test. printf("=== ALL KERNEL TESTS PASSED (%d/%d) ===\n", kernel_tests_run, kernel_tests_run) prints one counter twice, so the ratio is N/N unconditionally. The stated defect was that "the literal said 12/12 while a thirteenth test sat in this file uncalled, so the number agreed with the omission instead of exposing it" — a self-referential ratio has exactly that property. Delete a RUN() line and it prints 12/12 and still says ALL KERNEL TESTS PASSED. The counter does remove the stale-literal failure mode, which is a real improvement; but the thing that actually catches a dropped RUN() is the new Python guard, not this line, and the x/y shape implies a denominator the C code does not have.	Print what it knows: printf("=== ALL KERNEL TESTS PASSED (%d tests) ===\n", kernel_tests_run);. If a meaningful ratio is wanted it has to come from the defined-test count, which is what the Python guard supplies — so say so in the comment and let the guard own it. (Trivially, the file still ends without a newline — \ No newline at end of file — if it is being touched anyway.)
3	Low	tests/unit/test_cmake_test_registration.py:142	TEST_FUNC_RE = r"^static\s+void\s+(test_\w+)\s*\(\s*void\s*\)" matches only that one spelling, so a definition that is non-static, indented, returns anything other than void, or writes () instead of (void) is invisible to the guard — it is not reported as dead because it is never seen as a test at all. Not active today: of 291 test definitions under tests/, 289 are static void test_x( and the only 2 non-static void test_x( are in test_emulation_simulation.c and test_performance_benchmarks.c, both listed in NOT_BUILT and therefore never passed to _uncalled_in (which only walks _registered_sources()). It matters because the file's stated philosophy is to "fail on the class of mistake rather than on one instance of it", and a new suite written void test_x(void) would be silently exempt.	Widen to ^\s*(?:static\s+)?(?:void|int)\s+(test_\w+)\s*\(\s*(?:void)?\s*\) and add a fixture asserting a non-static definition is detected. Cheap, and it closes the gap before someone writes the 292nd test in a different style.
4	Low	tests/unit/test_cmake_test_registration.py:151-166	Transitive deadness is not detected. _uncalled_in counts bare identifier occurrences, so a call from inside another uncalled function counts as a call: given static void test_a(void){…} and static void test_b(void){ test_a(); } where nothing calls test_b, only test_b is reported and test_a is treated as live. The identifier-counting choice is deliberate and correct for its stated purpose — not flagging a function passed by address, and test_entry in this same file is exactly that case and is correctly ignored — so this is a documented-limitation issue, not a wrong design.	State it in the module comment so the next reader does not assume reachability analysis. Closing it properly needs a call graph, which is more machinery than the problem warrants; the honest note is enough.
5	Low	(CI)	2 of 26 checks fail, neither from this diff: Cross-compile ARM64 kernel fails building eBoot (eBoot/include/eos_image.h:135:23: error: 'eos_image_header_t' has no member named 'reserved', also :142), and Full-stack integration summary as its dependent. Confirmed pre-existing — the same workflow fails on origin/master at feee2726, and eos#121, #126 and #129 report the identical 2 failures.	Nothing for this author.

Architecture conformance

Conforms. tests/ is the repository's own test tree — master design §21 Tier 1 — Foundation for eos, and .ai/architect.md's reference layout lists tests/ as "unit, functional, fuzz, performance, simulation, mocks". The new guard is a static parse of tests/CMakeLists.txt and tests/*.c, needs no cmake or compiler, and introduces no import, link line or manifest entry — nothing points up a tier and §21.1 is not in play.

It serves §28 squarely, and this is the section that makes the PR worth landing rather than just tidy. §28's evidence table grants Implemented on "Code and functional tests" and Validated on "Hardware/CI/test reports". A test function that is compiled, linked, and never invoked satisfies neither while appearing to satisfy both — the suite goes green having skipped it, and ctest reports one more passing test. That is the exact shape the existing §28.2 proposal ("The evidence policy is silent on checks that verify nothing" in .ai/autoreview/proposals/2026-09.md) is about, one level below CI: not a workflow that ran nothing, but a binary that ran less than it contains. The proposal already covers the principle, so I am not appending a duplicate — but it is worth noting that this PR is the first instance in this org where the vacuous check was inside a test binary rather than in a workflow file, and the proposal's current wording is about CI jobs.

No check was weakened — this is the opposite. The diff adds two tests and removes none, converts twelve bare calls into RUN() invocations without dropping any, and adds the thirteenth that was missing. Nothing is skipped, xfailed or deleted. UNCALLED_OK is an escape hatch, which would normally warrant scrutiny, but it ships empty, so nothing is currently exempted — the concern with it is finding 1's usability defect, not a loosening.

The tests/CMakeLists.txt hunk is disclosed. The body opens "Stacked on #127. Review the last commit." The 5-line block is the same test_crypto_ed25519_loworder registration that #118, #119, #121, #126 and #127 all add at the same anchor, so the standing merge-order note applies: whichever lands second drops the hunk.

The finding itself is the most valuable part of this PR and deserves recording independently of the tooling: test_queue_send_waiter_overflow guards the change described as "a full send/recv waiter table now returns EOS_KERN_NO_MEMORY instead of blocking a task that can never be woken". That is a kernel-level liveness fix (§6.1, "interrupt, timer and synchronization semantics must be documented and testable") and it has been shipping with no executing test. It passes when called — I confirmed the suite reports 13/13 — so the fix is correct, but the PR is right that the difference between "correct" and "verified correct" was invisible here.

Proposed changes

1. Finding 1 — the key mismatch, both halves. This is the one I would hold the PR for; it is ~4 lines and it is the difference between an escape hatch that works and one that misleads. Seed one entry so the format is exercised.
2. Finding 3 — widen the definition regex and add a non-static fixture.
3. Finding 2 — (%d tests) instead of (%d/%d), and a trailing newline.
4. Finding 4 — one sentence in the module comment.

On the follow-up the body proposes: porting this guard to eBoot is worth doing and the case for it is stronger than the body states — eBoot is where the class has already cost a real test (a merge keeping a test function and dropping its run_ call), and it has no equivalent file at all. Fix finding 1 first so the port does not carry the defect.

Not checked

• master's pre-existing test_every_c_suite_is_built_or_listed failure (the unregistered test_crypto_ed25519_loworder.c) appears in my master-tree runs and is #127's subject, not this PR's. I did not investigate it further; it is unrelated to the new guard and disappears on this head because the stacked hunk registers the suite.
• The other 290 test definitions. The guard reports one uncalled function across every registered suite and I confirmed that number, but I did not independently audit the suites to confirm there is no second uncalled test that the regex in finding 3 fails to see. The two non-static definitions I found are both in NOT_BUILT files, so at minimum nothing registered is missed by that particular narrowness — but I checked the declaration style census, not each of the 289 functions.
• Whether the exempted NOT_BUILT suites contain uncalled tests — by construction they are never scanned, so five suites' worth of test functions are outside this guard entirely. That is defensible (they are not built) but it means "every test function is called" holds only over built suites.
• test_entry's pass-by-address case I confirmed is not reported, which is the stated no-false-positive claim; I did not construct further callback shapes to probe the identifier-counting heuristic.
• eBoot I did not look at, so the claims about tests/unit/test_ed25519.c and eBoot#94 are the author's, not verified here.
• What I did run, and what it showed — all four rows of the body's Validation block reproduce:
  • Guard against master's tree (PR's guard copied onto a clean origin/master extraction) → test_every_test_function_is_called fails with assert not ['test_kernel.c::test_queue_send_waiter_overflow'] — exactly one instance, the right one, no false positives.
  • Guard on this head → pytest tests/unit/test_cmake_test_registration.py -q → 7 passed.
  • ./b/tests/test_kernel → === ALL KERNEL TESTS PASSED (13/13) === (was 12/12 with 13 defined).
  • ctest --no-tests=error → 39/39 passed; cmake --build -j4 → exit 0, no errors.
  • pytest tests/ -q → 17 passed.
  • Plus the finding-1 probe: injecting the documented file::function key into UNCALLED_OK on master's tree → 3 failed / 4 passed, including the false staleness assertion; injecting the bare-name key → the exemption works and only the unrelated ed25519 failure remains.
  • Single configuration: GCC 15.2.0, x86_64 Linux, Python 3.12. No cross target, no hardware.

---

Automated architecture review of 93617696fb3d — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.