Repository navigation
fix: make CI actually run the test suite (pytest collection import path + CLI subparser regression) - #841
Merged
Conversation
…I regression) CI's `pytest -q` step was failing at collection with `ModuleNotFoundError: No module named 'scripts'`, which aborted the entire run — no tests executed on any PR or on main. Two root causes, both fixed: 1. pyproject `[tool.pytest.ini_options].pythonpath` listed `["src","scripts"]` but not the repo root, so `scripts` (a real package, scripts/__init__.py) was not importable as a top-level package by the 26 tests that do `import scripts.<mod>`. It only worked under `python -m pytest` (which adds CWD); CI uses the `pytest` console script, which does not. Add "." to pythonpath so the repo root is on sys.path under both invocations. 2. With collection fixed, 148 tests failed on a CLI NameError: `src/openamp_foundry/cli/main.py` built the `experiment-priority-check` subparser via `subparsers.add_parser(...)`, but the subparsers object is named `sub` (92 other call sites use `sub`; this was the lone `subparsers`). A regression from #832 that CI never caught because collection was already broken. One-identifier fix. Verified with the CI-equivalent invocation (`pytest -q`): the suite goes from 0 tests executed (collection abort) to 4418 passed. Four pre-existing failures remain (2 distinct bugs — candidate-summary-card lowercase validation and negative-result informativeness scoring); both fail on unmodified main, are unrelated to this change, were hidden by the broken collection, and need product-level review. Filed separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0151yUvpHj6wAd6JraWyChEX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CI's
pytest -qstep has been failing at collection withModuleNotFoundError: No module named 'scripts', which aborts the entire run — no tests execute on any PR or onmain. This has masked real regressions. Two root causes, both fixed here.1. Test collection (pyproject).
[tool.pytest.ini_options].pythonpathwas["src", "scripts"]but did not include the repo root. Thescriptsdirectory is a real package (scripts/__init__.py) that 26 test modules import as a top-level package (import scripts.<mod>). With onlysrcandscriptson the path,import scriptsis unresolvable. It happened to work underpython -m pytest(which adds the CWD tosys.path) but not under thepytestconsole script that CI runs. Fix: add"."so the repo root is on the path under both invocations.2. CLI subparser regression (
src/openamp_foundry/cli/main.py). With collection fixed, 148 tests failed onNameError: name 'subparsers' is not definedfrom the CLI. Theexperiment-priority-checksubparser was registered viasubparsers.add_parser(...), but the subparsers object is namedsub(92 other call sites usesub; this was the lonesubparsers). A regression from #832 that CI never caught because collection was already broken. One-identifier fix.Change class
Type of change
Safety impact
Verification
Run with the CI-equivalent invocation (
pytest -q, console script) afterpip install -e ".[dev]":3 errors during collection, 0 tests executed.ruff check src/openamp_foundry/cli/main.py→ clean.Limitations — pre-existing failures this unmasks
Fixing collection reveals 4 failing tests (2 distinct pre-existing bugs) that fail on unmodified
main, are unrelated to this change, and were hidden by the broken collection. They are out of scope here because both require product-level judgment and one is an explicit Class-C stop condition (scoring). Filed as separate issues:tests/evidence/test_candidate_summary_card.py::test_lowercase_sequence_fails— the validator.upper()-normalizes before checking amino acids, so it accepts lowercase, but its own test demands rejection (andpresynth_check.pyalso normalizes). Code-vs-test conflict needing a spec decision.tests/evidence/test_negative_result_informativeness.py::TestCLI(×3) — the informativeness scorer classifies its own test fixture below the INFORMATIVE threshold (≥6.0/7.0). Changing scoring behavior is a Class-C / human-review item.This PR is deliberately the minimal, verified fix for the named bottleneck (CI cannot run the test suite) — one loop, one change. It is a strict improvement: from 0 tests executing to 4418 passing.
Agent disclosure
Reviewer focus
"."topythonpathis acceptable (repo root onsys.path; no test relies on the barebenchmarks.top-level — all 26 usescripts.).🤖 Generated with Claude Code
Generated by Claude Code