Skip to content

fix(test): write the 11 vacuous tests for real and fix silent-failure helpers - #15351

Merged
mrveiss merged 5 commits into
Dev_new_guifrom
issue-15256-vacuous-tests
Aug 31, 2026
Merged

mrveiss merged 5 commits into
Dev_new_guifrom
issue-15256-vacuous-tests

Conversation

@mrveiss

@mrveiss mrveiss commented Aug 31, 2026 •

Copy link
Copy Markdown
Owner

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 return None/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 a tests/ 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 real knowledge/documents.py:66-69 asyncio.wait_for(..., timeout=kb_timeouts.document_add). A DocumentsMixin host 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 new autobot-backend/services/rag_integration_api_test.py (kept the parent file under MAX_LINES) and rewritten to hit the real api.knowledge_rag router at its production prefix (/knowledge_base/rag) through TestClient, with get_current_user and get_rag_service_dependency overridden. Real assertions: enable_reranking threads through to RAGService.advanced_search, search results/metrics round-trip, rerank_results reranks, and GET/PUT /config/rag read and mutate the real RAGConfig singleton.
  • autobot-backend/tests/api/test_knowledge_grounding.py:614,620,626,632 (4 endpoint tests) — rewritten to hit the real api.knowledge_grounding router through TestClient, with get_current_user/check_admin_permission overridden and get_grounded_agent/get_async_redis_client patched per test. Real assertions on verify-claim, kb-conflicts list/resolve, and kb-stats.
  • Updated the existing empty-body guard (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, so autobot-backend is 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_func now takes an attr_name, anchors at parents[2] (not parents[3]), actually calls spec.loader.exec_module(...), and raises ImportError/AttributeError instead of returning None. Added test_import_func_resolves_the_real_module, which imports the real _fallback_agents_from_registry/_ORCH_TYPE_MAP from api/agent_org.py (the class's other tests reimplement this logic inline; nothing else consumed _import_func's old None).
  • autobot-infrastructure/shared/tests/conftest.py:106 and autobot-backend/conftest.py:1387 — both test_data_dir fixtures 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 the KNOWN_UNRESOLVED grandfather entry now that the anchor resolves; replaced test_the_grandfathered_set_is_down_only with test_the_previously_grandfathered_anchor_now_resolves, which pins that the site resolves to agent_org.py going forward.
  • repo_tests/trivial_config_test.py:18 — anchor was Path(__file__).parent / "autobot-backend" (resolves to the non-existent repo_tests/autobot-backend); fixed to Path(__file__).resolve().parent.parent / "autobot-backend". Also added the repo root to sys.path so autobot_shared resolves too — the file's own main() was still unrunnable standalone with only the backend-path fix (pytest already provides both via pytest.ini's pythonpath), so "confirmed to work" meant fixing both. Verified: python3 repo_tests/trivial_config_test.py now 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 a tests/ 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_FLOOR from 2950 to 3050 (same 149-line margin). Added test_a_non_test_helper_inside_a_tests_directory_is_still_scanned, the contrast mutation: a fork placed in llc/tests/_e2e_harness.py-shaped path is now caught, while a real test file in the same directory stays excluded.
  • Swept for the same blind spot elsewhere (AC4) and found one more instance, 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 shrank autobot-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 new wc -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 in timeout_configuration_test.py, all five TestAPIEndpoints placeholders in rag_integration_test.py, all four endpoint placeholders in test_knowledge_grounding.py, _import_func's parents[3]/return None, both test_data_dir fixtures pointing at missing directories, trivial_config_test.py's off-by-one anchor, and _SKIP_PARTS containing 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 passed
  • autobot-backend/services/rag_integration_api_test.py (new) — 5 passed
  • autobot-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_func test)
  • repo_tests/test_module_path_anchors_15181_test.py — 17 passed
  • repo_tests/trivial_config_test.py — 4 passed via pytest; standalone python3 repo_tests/trivial_config_test.py also verified end-to-end
  • repo_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 for autobot-backend)
  • repo_tests/python_file_size_ratchet_test.py — 31 passed (both ratchet entries confirmed exact)
  • Combined run across all ten files above: 165 passed

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.py files it detected as changed and reported [pre-push OK] pytest: all relevant tests pass before the push completed.

Model Used

Claude Opus 5 (1M context)

Closes #15256, #15251, #15258 — all criteria delivered; see the per-site table above.

… 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.
@github-actions

Copy link
Copy Markdown
Contributor

✅ 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

mrveiss commented Aug 31, 2026

Copy link
Copy Markdown
Owner Author

Review pass — one blocking defect found and fixed on the branch (a3785a1cf1).

Draining _EMPTY_BODIED to {} disabled this guard's own anti-vacuity floor. _collapsed() derives entirely from that dict:

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 {} unconditionally, so the assertion guarding against a broken sweep — "a scan that finds nothing reports every tree clean" — could no longer fire, at either of its two call sites. If test_modules() ever stopped resolving, the population and the empty-body count would collapse to zero together and the guard would report clean.

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: _sweep_shortfalls(), with floors that do not derive from the baseline — measured 28708 test functions across 13 trees on Dev_new_gui, floors set at 25000 / 10, only ever rising. It also collects every shortfall rather than asserting on the first, so one red assertion cannot mask the others.

Proven to fire rather than assumed:

collapsed sweep -> ['only 0 test functions collected, floor is 25000', 'only 0 trees reached, floor is 10']
one tree only   -> ['only 22354 test functions collected, floor is 25000', 'only 1 trees reached, floor is 10']
healthy         -> []

pytest repo_tests/tests_that_execute_nothing_test.py → 5 passed. The rest of the PR verified sound: all 11 vacuous tests implemented rather than deleted or skipped, and the pre-existing #15254 guard was extended rather than forked into a second one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant