test-guard(repo_tests): count the tests that run nothing, and unwrap four that could not fail (#15189)

[mrveiss]

Thinking Path

#15189 asked for three sweeps. Running them was only possible after #15195 landed as c5306fe85, because until then the AST guard treated an assert that was present as one that could fire — so nine of the ten functions that started this issue read as defended.

The merged detector was read first and called, not re-implemented. repo_tests/collected_test_model already owns "what pytest collects" and "can this assertion fire"; the third sweep is a third question asked of the same model, so it is a new function in that module rather than a second AST walk. Re-implementing the walk is how #14927 and #15018 happened.

The third shape needed a definition narrow enough to survive contact with 27,000 tests. The rule chosen is the strictest one that still catches test_migrated_files_import: every statement in the body is a no-op — pass, ..., a bare string, or a bare-name print(...). One statement that calls anything and the function is out of scope, because "it does not raise" is a thin assertion but a real one. A test declaring @pytest.mark.skip/skipif/xfail is honest about not running and is not the subject. An over-flagging guard gets switched off, which is worse than the blindness.

Counts were taken before anything was touched, and only the cheap, unambiguous conversions were made. What was left is named below with the reason.

What Changed

Sweep counts, measured on this branch before any fix:

Sweep	Detector	Before	After
1 — assert neutralised by a swallowing handler	swallowed_assertions_in (#15195)	9 functions / 5 files / 2 trees	5
2 — test returns a value instead of asserting	_offending_returns (#14920)	220 returns / 3 trees	214
3 — test body executes nothing	new, empty_bodies_in	11 functions / 3 files / 1 tree	11 (ratcheted)

Per tree — sweep 1 before: autobot-backend 6, autobot-infrastructure 3. Sweep 2 before: autobot-backend 86, autobot-infrastructure 127, autobot-npu-worker 7. Sweep 3: autobot-backend 11, every other tree zero.

New detector. repo_tests/collected_test_model.py gains does_nothing, is_declared_not_running, empty_bodies and empty_bodies_in. No ratchet and no budget, matching the module's existing contract.

New guard. repo_tests/tests_that_execute_nothing_test.py — a per-tree down-only ratchet pinned at the measured 11, every other tree pinned at zero by derivation, a population floor so a collapsed walk cannot read as a drained tree, and a self-test that exercises every flagging branch and every sparing branch.

Four tests converted, each of which now holds an assertion that reaches pytest:

• autobot-backend/config/config_migration_integration_test.py:87 test_secrets_service_config_migration — assertions moved out from under except Exception: pass.
• autobot-infrastructure/shared/scripts/test_configuration.py — test_config_values, test_config_manager, test_config_validation lost their try: … except Exception: return False wrappers. The driver contract is unchanged: main() already wraps every call in its own handler and records a crash as a failed test, so the verdict is identical and the traceback is no longer discarded.

Ratchets lowered, never raised. _SWALLOWED_ASSERTIONS: autobot-backend 6 → 5, and autobot-infrastructure deleted rather than left at zero. _KNOWN_OFFENDERS["autobot-infrastructure"]: 127 → 121 — those six returns stopped being counted because the assertions beside them became live, which is the guard's own exemption rule, not a weakening of it. tests_that_return_instead_of_asserting_test.py finishes at 599 lines, under MAX_LINES = 600, with no KNOWN_LARGE entry added.

Left deliberately, and why. Five swallowing sites remain: autobot-backend/config/config_consolidation_p2_test.py:202 (one 200-line function with ten try/assert/except sections — the same restructuring #15166 did for its sibling cache_consolidation_p4_test.py, and its own piece of work), and four *_full_pipeline methods at autobot-backend/tests/integration/test_causal_framework_integration.py:152,394,707,937 that catch AssertionError into a scenario dataclass a report aggregator consumes. All eleven empty-bodied tests remain: each needs a real integration test written against a live endpoint or a real KnowledgeBase. Half-converting a population is how a ratchet gets stuck, so all sixteen are recorded under a down-only ceiling instead.

Verification

Guards, after the change — repo_tests/tests_that_return_instead_of_asserting_test.py, repo_tests/tests_that_execute_nothing_test.py, repo_tests/collected_test_model_test.py: 21 passed. repo_tests/python_file_size_ratchet_test.py: 31 passed.

Contrast mutation 1 — the converted test now fails when its subject breaks. ConfigManager.set was mutated to store the wrong value, then reverted from a pristine copy confirmed with diff -q:

Subject	Test	Result
unmutated	converted	1 passed
mutated	converted	1 failed — AssertionError: assert 'MUTATED' == 'test_secrets_key'
mutated	original (swallowing)	1 passed

The third row is the point: the same broken subject, and the pre-fix test reports green.

Contrast mutation 2 — the driver's swallow was hiding a real break. Same file, run directly:

• before: 7 passed, 7 warnings (every one a PytestReturnNotNoneWarning)
• after: 3 failed, 4 passed

The three failures are pre-existing and are recorded as a finding below, not introduced here.

Contrast mutation 3 — the new guard catches the shape. A planted def test_planted_empty_body(): pass + print("✅ subject verified") in autobot_shared/ (a tree pinned at zero by derivation) fails the guard by name:

E   autobot_shared/planted_probe_15189_test.py:1 test_planted_empty_body
E   assert not {'autobot_shared': [...]}
1 failed, 4 passed

Counter-mutation — the guards spare legitimate code. A probe file in the same clean tree holding a try/except ValueError that pytest.skips, a try/except Exception that re-raises, a thin smoke test that only calls its subject, and a @pytest.mark.skip test with an empty body: 13 passed, nothing flagged by either guard. The same shapes are pinned as synthetic cases in the new guard's self-test, so the sparing branches are proven rather than described.

AC 6 — was cache_consolidation_p4_test.py ever exempted? No, and nothing suppressed the warning either. It appears in no skip, ignore or exemption list anywhere in the repository; its only two mentions outside itself are a path mapping in scripts/migrate_tests.py:161 and a historical note in the ratchet. pytest.ini has no filterwarnings section at all, its norecursedirs covers only VCS and build directories, and addopts ignores only autobot-slm-backend/ansible and autobot-npu-worker/resources. The warning was emitted into a green run for its whole life and nothing was configured to catch it.

What could not be verified. Every run above is local, on an interpreter 90 of 206 declared dependency floors below CI — the suite prints that banner itself. These runs demonstrate the shape of each guard and each fix; CI is the authority on whether they pass where they ship. The AC asking to "show CI fails" on a reintroduced swallowing test is met only to the extent that the planted probe fails the guard locally; no deliberately-red commit was pushed to prove it in CI. filterwarnings = error::pytest.PytestReturnNotNoneWarning remains off, correctly: 214 returns are still in the tree, and 121 of them are in a tree no pytest invocation collects, so the config would report clean while more than half the population sat outside its reach.

Feasibility of detecting the third shape: confirmed and cheap. 74 lines in the shared model, driven off the collection model that already existed, and the population is 11 in a single tree. The reason it was never measured is that nobody had asked the question, not that it was hard.

Model Used

Claude Opus 5 (1M context)

Refs #15189 — not Closes. Two criteria are short: the contrast mutation is demonstrated locally rather than as an observed CI failure, and the sixteen remaining sites (5 swallowing + 11 empty-bodied) are measured and ratcheted down-only rather than converted.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

Review pass complete — no blocking findings

The reviewer independently re-derived every number in the PR body rather than taking it on trust: the population (patterns read live from pytest.ini, not a bare unanchored git ls-files pattern — the failure mode that made an earlier guard match zero files), the 11 counted sites read individually at head, and the ratchet arithmetic in the sibling file (9→5, 127→121) matching the claimed deltas exactly.

The ratchet is bidirectional, which is the thing that most often goes wrong here: it fails on over, on spent (count below budget without lowering it), and on drained — the last instructing that the entry be deleted rather than left sitting at 0. That is the correct handling of the down-only anti-pattern, not an instance of it.

AC5 (the contrast mutation demonstrated red in CI) is honestly disclosed as not met — mutations were run locally and no red commit was pushed. The PR correctly says Refs #15189 rather than Closes for exactly that reason, so #15189 stays open on that criterion.

Two detector gaps found, both minor, both false-positive directions that cannot hide a vacuous test, and neither triggered by any of the 11 sites today. Filed separately rather than held against this PR.

[mrveiss]

Detector gaps filed as #15263 — print(subject()) classified as a no-op, and class-level skip marks not seen by is_declared_not_running(). Neither is triggered by any of the 11 sites this PR records.