bug(ci): 56 of 74 test files under autobot-infrastructure are collected by nothing, and one kills the pytest session #14917

Activity

1. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   infrastructure

   on Aug 24, 2026
2. mrveiss commented on Aug 24, 2026

   

   mrveiss

   on Aug 24, 2026

   OwnerAuthor

   More actions

   Partially addressed by #14929 — AC1 and AC4 delivered, AC2/AC3/AC5 not. This issue stays open.

   AC1 — the session-killer. Delivered, and there were TWO.

   This issue names shared/scripts/test_alertmanager.py. Sweeping every file matching pytest.ini's python_files for an import-time exit found a second module with the identical defect: shared/scripts/test_grafana_integration.py, also ending in a bare sys.exit(0) at module scope. Both are pure print() status summaries that assert nothing.

   Both now run behind main() + if __name__ == "__main__":, so import is inert and the scripts still work. Their module-scope sys.path.insert moved inside too — that was a second import-time side effect polluting sys.path for the whole session.

   The generalisation is in repo_tests/collected_modules_inert_on_import_test.py: derives the file patterns from pytest.ini rather than hard-coding them, bans import-time sys.exit / exit / quit / os._exit / raise SystemExit repo-wide, and allows the __main__-guarded form — 46 modules use it correctly, and a guard that failed them would be disabled within a week. A floor of 30 asserts that exemption branch still has live subjects, so it cannot quietly stop meaning anything.

   That the population was never derived is exactly why the second file was unknown. 1973 modules scanned, 2 fatal, 46 correctly guarded.

   AC4 — the invocation drift. Delivered, and it was hiding a correctness hole.

   tools, scripts and pipeline-scripts are now in all four invocations, not just ci.yml. While deriving the root sets I found something this issue records only as a reporting problem:

   scripts/check_script_exec_bits_test.py's @pytest.mark.integration case was selected by no workflow at all. ci.yml, coverage.yml and test-durations.yml all deselect that marker; marker-tests.yml, the one that selects it, did not name the tree it lives in. So that test ran nowhere — the same "listed nowhere, so collected by nothing" defect this issue is about, a fifth time.

   Rather than patching three lists, all four invocations that name repo_tests now share one root set, so marker selection is the only axis that varies. Two tests appended to repo_tests/hook_suites_run_in_ci_test.py:

   • the four root lists must agree with each other, never with a hard-coded list — so a fifth invocation is checked on arrival and no entry goes stale on a rename;
   • the reason that matters is re-derived rather than asserted: a marked test must still exist outside the core trees, or the floor is guarding nothing and says so.

   AC2, AC3, AC5 — not delivered

   • AC2 (23 collection errors) — triaging these requires running collection, which imports each module. That is beyond what a static sweep can settle, and each error needs its own decision (fix and collect, or rename out of the test_* namespace).
   • AC3 (the remaining 56 files) — wiring them today would redden CI outright. Measured on this branch, those files carry 134 return-value offenders (see bug(test): 262 tests return a value instead of asserting, so they cannot fail #14920) on top of the 23 import errors. The sys.exit(0) blocker is now gone, so this is unblocked, but it is a triage job per file, not a testpaths edit.
   • AC5 — mutation evidence for a newly-collected suite has to wait on AC3.

   Suggested split

   The remaining work is three separable pieces, and bundling them is what made this issue large enough to stall:

   1. the 23 collection errors, triaged individually;
   2. the 30 files under shared/scripts/analysis — the largest block, and by shape (heavy print(), __main__ drivers, no assertions) most of them look like scripts that should leave the test_* namespace rather than tests to wire in;
   3. the remaining 26, wired in with mutation evidence.

   Related

   • fix(test): tests, suites and hooks that cannot report what they were written to report (#14923, #14920, #14917) #14929 — the PR
   • bug(test): 262 tests return a value instead of asserting, so they cannot fail #14920 — the 134 return-value offenders inside the uncollected files
   • bug(test): 120 test_* methods sit in non-Test* classes, so pytest collects none of them #14927 — 120 test_* methods in non-Test* classes, 4 of those files in this tree
3. mrveiss

   added 2 commits that reference this issue on Aug 24, 2026

   fix(ci): stop two test-named scripts killing the pytest session, and …

   ...

   018b68c

   fix(test): tests, suites and hooks that cannot report what they were …

   ...

   10d7be2
4. mrveiss commented on Aug 24, 2026

   

   mrveiss

   on Aug 24, 2026

   OwnerAuthor

   More actions

   Delivered and merged as 10d7be234 (PR #14929).

   • CI: 51 success / 9 skipped / 0 failures, and — checked separately — all 10 required contexts present, not merely non-failing. A required check that never runs is absent from the rollup entirely, so a tally of present checks reads it as green; this PR was gated against that.
   • Review: two independent code-reviewer passes, plus a focused re-review of the delta I authored myself.

   The population was measured, not taken from the issue. #14920 filed 262; the real count is 327 returns across 174 functions in 53 files. The naive walk figure was 1870, not 739. autobot-frontend's 22 turned out to be methods of a class that does not match python_classes — pytest never collects them, a different defect, filed separately as #14927.

   49 functions fixed, and that number is stated rather than rounded up to the headline. Exclusions with basis: 152 returns in non-Test* classes (dead code — converting would edit what nothing executes), 11 fixtures, and 6 whose return True a caller genuinely consumes.

   Review caught 9 conversions that broke their own drivers. Those drivers dispatch indirectly — for name, fn in tests: result = await fn() — and sum truthiness, so a bare return made result None and every passing test reported failure. The consumed-check had matched literal funcname( syntax and could not see a function referenced only as a bare name inside a tuple, so all 9 landed in the "uncalled, therefore safe" bucket. They now carry both a real assertion and a truthy return: the assertion so the test can fail, the return so the driver's verdict stays honest. Either alone is insufficient.

   That fix then exposed a flaw in the ratchet itself. offending_returns counted every value-return with no exemption for functions that also assert — so a correct fix was rejected by the guard the PR added. Refined to skip functions with their own assert or raise, since the defect is returning instead of asserting. Driven on synthetic cases: return-only is an offence, assert-and-return and raise-and-return are not.

   The budget drop is documented honestly, because the obvious reading is wrong. autobot-backend fell 136 → 78 while none of the nine fixes are in that tree. The entire movement is a tree-wide side effect of the exemption, which un-flags 39 pre-existing functions there and 4 in autobot-infrastructure. All 43 were checked individually — every one carries real assertions (1-24 apiece), so nothing vacuous is hidden. The comment in base now says exactly that.

   The ratchet refuses a collapsed sweep, failing with "Fix the sweep; do NOT lower these numbers" rather than inviting someone to lower the floor, and offers no route to raise a budget — verified in review to match the docstring's claim, unlike #14919 where a message promised a route the code never implemented.

   filterwarnings=error was deliberately deferred, and review judged that correct: it only sees tests that run, and 141 of the 277 remaining returns are in trees pytest never collects. Enabling it would have reported clean with over half the population out of reach. The AST ratchet is the broader prevention.

   Residuals filed: #14927, #14928, #14931, #14941.
5. mrveiss

   closed this as completedon Aug 24, 2026