Repository navigation
fix(test): write the 11 vacuous tests for real and fix silent-failure helpers - #15351
Conversation
… helpers (#15256, #15251, #15258) Eleven test functions that executed nothing while their names asserted a behaviour now hit the real endpoints/timeout paths through TestClient and dependency_overrides; _import_func now actually imports api/agent_org.py and raises instead of returning None; two test_data_dir fixtures pointing at missing, unconsumed directories are removed; trivial_config_test.py's sys.path anchor is fixed; and the with_error_handling guard now scans non-test helpers living inside tests/ directories instead of pruning the whole directory.
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
…rom the drained baseline (#15256)
|
Review pass — one blocking defect found and fixed on the branch ( Draining return {
tree: (populations.get(tree, 0), floor)
for tree, (_, floor) in _EMPTY_BODIED.items() # <- now empty
if populations.get(tree, 0) < floor
}With the dict empty it iterates nothing and returns That is the exact defect class this PR exists to remove, reproduced in the guard that enforces it. Reaching zero is what made it possible: the floor was a side effect of the baseline being non-empty, so eliminating the last entry silently removed the protection. Fix: Proven to fire rather than assumed:
|
Thinking Path
All three issues are the same defect shape: a test or helper that reports success while executing nothing. Ground-truthed every named location against the actual files before touching them (all eleven #15256 sites, all four #15251 sites, and the #15258 directory-pruning behaviour matched their issue descriptions exactly — no stale claims found). For each of the 11 vacuous tests, the mandate was "implement the behaviour the function name asserts," which meant finding the real endpoint or code path each name promised and exercising it through the same TestClient/dependency_overrides pattern already used elsewhere in this codebase (
tests/api/test_onboarding_auth_regression.py). For the two broken helpers, the fix was to make failure loud (raise, not returnNone/a missing path) rather than paper over it. For the guard, the fix was narrowing an exclusion from directory-name to file-name so a non-test helper inside atests/directory is no longer invisible to it.What Changed
#15256 — 11 vacuous tests, all implemented (none deleted):
autobot-backend/config/timeout_configuration_test.py:241,247(test_knowledge_base_uses_timeouts,test_timeout_changes_affect_runtime) — implemented against the realknowledge/documents.py:66-69asyncio.wait_for(..., timeout=kb_timeouts.document_add). ADocumentsMixinhost with a controllable internal delay proves the accessor is actually read (1ms budget vs 50ms op → timeout) and that changing the value changes the outcome (1ms → timeout, 5s → success).autobot-backend/services/rag_integration_test.py:536,542,548,554,560(TestAPIEndpoints, 5 tests) — moved to newautobot-backend/services/rag_integration_api_test.py(kept the parent file underMAX_LINES) and rewritten to hit the realapi.knowledge_ragrouter at its production prefix (/knowledge_base/rag) throughTestClient, withget_current_userandget_rag_service_dependencyoverridden. Real assertions:enable_rerankingthreads through toRAGService.advanced_search, search results/metrics round-trip,rerank_resultsreranks, and GET/PUT/config/ragread and mutate the realRAGConfigsingleton.autobot-backend/tests/api/test_knowledge_grounding.py:614,620,626,632(4 endpoint tests) — rewritten to hit the realapi.knowledge_groundingrouter throughTestClient, withget_current_user/check_admin_permissionoverridden andget_grounded_agent/get_async_redis_clientpatched per test. Real assertions onverify-claim,kb-conflictslist/resolve, andkb-stats.repo_tests/tests_that_execute_nothing_test.py, added by prior PR test-guard(repo_tests): count the tests that run nothing, and unwrap four that could not fail (#15189) #15254) — the_EMPTY_BODIED["autobot-backend"] = (11, 18000)ratchet is now an empty dict, soautobot-backendis pinned at zero by derivation like every other tree. This guard already had the "vacuity floor" the task asked for (test_functions_by_tree()floor, checked before the ratchet count), so no new guard was written — the existing one was fixed in place.#15251 — four helpers:
autobot-backend/tests/unit/test_agents_status_pg_optional.py:44-73—_import_funcnow takes anattr_name, anchors atparents[2](notparents[3]), actually callsspec.loader.exec_module(...), and raisesImportError/AttributeErrorinstead of returningNone. Addedtest_import_func_resolves_the_real_module, which imports the real_fallback_agents_from_registry/_ORCH_TYPE_MAPfromapi/agent_org.py(the class's other tests reimplement this logic inline; nothing else consumed_import_func's oldNone).autobot-infrastructure/shared/tests/conftest.py:106andautobot-backend/conftest.py:1387— bothtest_data_dirfixtures removed. Confirmed zero consumers repo-wide (grep -rn "(test_data_dir"in both trees returns nothing) before removing, per the issue's own AC ("a fixture nobody uses is a different fix from one whose consumers have been quietly getting nothing").repo_tests/test_module_path_anchors_15181_test.py— removed theKNOWN_UNRESOLVEDgrandfather entry now that the anchor resolves; replacedtest_the_grandfathered_set_is_down_onlywithtest_the_previously_grandfathered_anchor_now_resolves, which pins that the site resolves toagent_org.pygoing forward.repo_tests/trivial_config_test.py:18— anchor wasPath(__file__).parent / "autobot-backend"(resolves to the non-existentrepo_tests/autobot-backend); fixed toPath(__file__).resolve().parent.parent / "autobot-backend". Also added the repo root tosys.pathsoautobot_sharedresolves too — the file's ownmain()was still unrunnable standalone with only the backend-path fix (pytest already provides both viapytest.ini'spythonpath), so "confirmed to work" meant fixing both. Verified:python3 repo_tests/trivial_config_test.pynow runs end-to-end and prints "All tests passed!".#15258 — directory-name pruning:
repo_tests/with_error_handling_single_definition_test.py— removed"tests"from_SKIP_PARTS. The per-file filename check (test_*.py/*_test.py) already excludes actual test files regardless of directory, so this narrows the exclusion from "everything under atests/directory" to "test files, wherever they live" — the AC's own suggested resolution. Recorded the decision in the docstring. Re-measured the tracked population (3,099 → 3,199) and moved_SOURCE_FLOORfrom 2950 to 3050 (same 149-line margin). Addedtest_a_non_test_helper_inside_a_tests_directory_is_still_scanned, the contrast mutation: a fork placed inllc/tests/_e2e_harness.py-shaped path is now caught, while a real test file in the same directory stays excluded.repo_tests/workflow_planner_deprecation_test.py:65— a different file with a different guard, so filed separately as test(guard): workflow_planner_deprecation_test.py prunes tests/ by directory name, same blind spot as #15258 #15350 rather than folding into this PR.Collateral (
scripts/python_file_size_known_large.py/repo_tests/python_file_size_ratchet_baseline.py):Writing real tests grew
test_knowledge_grounding.py(835 → 934) and shrankautobot-backend/conftest.py(1676 → 1670, from removing the dead fixture). Both are on the CHECKED PAIR ratchet; both entries updated in both files in this commit, exact match to the newwc -l-equivalent line counts.False/stale claims found
None. Every location named in all three issues matched the code as committed: the two
pass/comment bodies intimeout_configuration_test.py, all fiveTestAPIEndpointsplaceholders inrag_integration_test.py, all four endpoint placeholders intest_knowledge_grounding.py,_import_func'sparents[3]/return None, bothtest_data_dirfixtures pointing at missing directories,trivial_config_test.py's off-by-one anchor, and_SKIP_PARTScontaining a bare"tests"entry all confirmed exactly as described.Verification
Ran locally with the repo's Python 3.10.12 interpreter (many declared floors unmet per
pytest.ini's dependency-floor banner — a pass here is not CI-equivalent, but nothing skipped):autobot-backend/config/timeout_configuration_test.py— 22 passed (incl. both new tests)autobot-backend/services/rag_integration_test.py— 22 passedautobot-backend/services/rag_integration_api_test.py(new) — 5 passedautobot-backend/tests/api/test_knowledge_grounding.py— 31 passed (incl. all 4 new)autobot-backend/tests/unit/test_agents_status_pg_optional.py— 8 passed (incl. new_import_functest)repo_tests/test_module_path_anchors_15181_test.py— 17 passedrepo_tests/trivial_config_test.py— 4 passed via pytest; standalonepython3 repo_tests/trivial_config_test.pyalso verified end-to-endrepo_tests/with_error_handling_single_definition_test.py— 20 passed (incl. new contrast mutation)repo_tests/tests_that_execute_nothing_test.py— 5 passed (ratchet now zero forautobot-backend)repo_tests/python_file_size_ratchet_test.py— 31 passed (both ratchet entries confirmed exact)All of the above ran and executed (none skipped locally) — every assertion in this PR was exercised, not merely collected. The pre-push hook independently ran pytest on the seven
_test.pyfiles it detected as changed and reported[pre-push OK] pytest: all relevant tests passbefore the push completed.Model Used
Claude Opus 5 (1M context)
Closes #15256, #15251, #15258 — all criteria delivered; see the per-site table above.