fix(tests): remove broken sys.path.insert shadowing project packages - #101
Merged
Conversation
Eight test files under tests/<subdir>/ had:
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
which only ascends one level (to tests/), not to the project root. Since
tests/agent/, tests/hermes_cli/, and tests/cli/ are all real Python
packages (each has __init__.py), inserting tests/ at sys.path[0] causes
absolute imports like `from agent.model_metadata import ...` or `from
hermes_cli.auth import ...` to resolve to the test subpackage itself
instead of the real top-level package -- there's no such module in the
test subpackage, so it raises ModuleNotFoundError.
tests/agent/test_model_metadata_ssl.py and tests/hermes_cli/test_auth_ssl_macos.py
hit this every run (100% reproducible collection failure, not
environment-specific). The other six files (test_model_metadata_local_ctx.py,
test_voice_wrapper.py, test_tool_progress_scrollback.py, test_resume_display.py,
test_cli_user_message_preview.py, test_cli_init.py) have the identical bug but
it stayed dormant -- likely because the correctly-shadowed module was already
cached in sys.modules by the time these files were collected, depending on
collection order.
The line was always redundant: tests/conftest.py:32-34 already inserts the
real PROJECT_ROOT into sys.path unconditionally before any test file loads.
Deleted the line (and the now-unused os/sys imports where nothing else in
the file needed them) rather than fixing the ascent count, since conftest.py
already owns this responsibility.
Also installed psutil, ptyprocess, ruamel.yaml, croniter, and fire in this
environment -- all five are declared dependencies in pyproject.toml
(psutil/fire in the unconditional `dependencies` list, ruamel.yaml/croniter
also core, ptyprocess behind the `pty` extra) but were missing from this
session's Python environment, which was the root cause of ~65 of the 69
test failures seen in a fresh full-suite run on this branch. That's a
container-provisioning gap, not a code issue -- nothing to fix in the repo
for those. Full suite now passes 25370/25371; the one remaining failure
(test_auth_nous_provider.py::test_no_ca_bundle_returns_true) fails only
because this sandbox's outbound-proxy sets REQUESTS_CA_BUNDLE system-wide,
which _resolve_verify() correctly honors per its documented CA-bundle
priority order -- not reproducible in a normal CI/dev environment.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PTSqxb1h9MRkvGvVJqSuQh
🔎 Lint report:
|
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.
What does this PR do?
Fixes a real, 100%-reproducible test collection bug and cleans up six more files carrying the same latent defect, found while triaging a full test-suite run on
mainafter PR #93 merged (69 failing tests across 26 files, 4 files that failed to collect at all).tests/agent/test_model_metadata_ssl.pyandtests/hermes_cli/test_auth_ssl_macos.pyeach had:This only ascends one level — to
tests/, not the project root. Sincetests/agent/andtests/hermes_cli/are real Python packages (each has__init__.py), insertingtests/atsys.path[0]makesfrom agent.model_metadata import .../from hermes_cli.auth import ...resolve to the test subpackage itself instead of the real top-level package. Neither has amodel_metadata.py/auth.py, so collection fails withModuleNotFoundError.Six more files had the identical bug but it stayed dormant (likely because the correctly-resolved module was already cached in
sys.modulesby the time these files were collected, depending on collection order) — fixed for consistency since they're the same landmine waiting to trigger under a different collection order:test_model_metadata_local_ctx.py,test_voice_wrapper.py,test_tool_progress_scrollback.py,test_resume_display.py,test_cli_user_message_preview.py,test_cli_init.py.The line was always redundant —
tests/conftest.py:32-34already inserts the realPROJECT_ROOTintosys.pathunconditionally before any test file loads.The remaining ~65 of the 69 originally-failing tests turned out to be a container-provisioning gap in this specific session's environment, not a code issue:
psutil,fire,ruamel.yaml, andcroniterare all declared as unconditional core dependencies inpyproject.toml(plusptyprocessbehind theptyextra) but were missing from this session's Python env. Installed locally to confirm — no repo change needed for those. One test remains failing in this sandbox only (test_auth_nous_provider.py::test_no_ca_bundle_returns_true) because this environment's outbound-proxy config setsREQUESTS_CA_BUNDLEsystem-wide, which_resolve_verify()correctly honors per its documented CA-bundle priority order — not reproducible in a normal CI/dev environment, left as-is.Related Issue
Fixes #
Type of Change
Changes Made
tests/agent/test_model_metadata_ssl.py: removed the brokensys.path.insertline and the now-unusedos/sysimports — fixes a 100%-reproducible collection failure.tests/hermes_cli/test_auth_ssl_macos.py: same fix — fixes a 100%-reproducible collection failure. (sysimport kept; still used elsewhere in the file formonkeypatch.setattr(sys, "platform", ...).)tests/agent/test_model_metadata_local_ctx.py,tests/hermes_cli/test_voice_wrapper.py,tests/cli/test_resume_display.py: same fix, dormant bug (bothos/sysimports removed — unused elsewhere).tests/cli/test_tool_progress_scrollback.py,tests/cli/test_cli_user_message_preview.py: same fix, dormant bug (osimport removed,syskept — still used forsys.modules).tests/cli/test_cli_init.py: same fix, dormant bug (bothos/sysimports kept — both still used elsewhere in the file).How to Test
TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0 python3 -m pytest tests/agent/test_model_metadata_ssl.py tests/hermes_cli/test_auth_ssl_macos.py tests/agent/test_model_metadata_local_ctx.py tests/hermes_cli/test_voice_wrapper.py tests/cli/test_tool_progress_scrollback.py tests/cli/test_resume_display.py tests/cli/test_cli_user_message_preview.py tests/cli/test_cli_init.py -q— 176 passedscripts/run_tests_parallel.py(withpsutil/fire/ruamel.yaml/croniter/ptyprocessinstalled to match this session's environment topyproject.toml's declared dependencies): 25370/25371 passed — the 1 remaining failure is the sandbox-onlyREQUESTS_CA_BUNDLEartifact described above.ruff checkon all 8 changed files — clean.Checklist
Code
fix(scope):)Documentation & Housekeeping
Generated by Claude Code