From 291f356dabc2654452a6b85712d0e226a7238a99 Mon Sep 17 00:00:00 2001 From: dev-lead Date: Fri, 8 May 2026 13:27:56 -0700 Subject: [PATCH] fix(adapter,tests): isolate _load_providers tests from multi-path lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 5 _load_providers tests were single-path-only: they wrote a config.yaml to tmp_path and called _load_providers(str(tmp_path)), expecting the lookup to read tmp_path/config.yaml. After the multi-path fix in #7, _load_providers also checks: 1. _CANONICAL_ADAPTER_DIR/config.yaml (= /opt/adapter/config.yaml) 2. _TEMPLATE_DIR/config.yaml (= dirname(__file__)/config.yaml) 3. ${config_path}/config.yaml (the test's tmp_path) Path 2 finds the repo's bundled config.yaml on the test runner's disk before path 3 — the tests then see the bundled providers list instead of the test's expected behavior. Two surface changes: 1. adapter.py — extract `os.path.dirname(os.path.abspath(__file__))` into a module-level `_TEMPLATE_DIR` constant, mirroring `_CANONICAL_ADAPTER_DIR`. Production behavior identical (resolved once at import). Tests can monkeypatch the module attribute to redirect the path-2 lookup. 2. tests/test_adapter_prevalidate.py — 5 _load_providers tests monkeypatch `_CANONICAL_ADAPTER_DIR` and `_TEMPLATE_DIR` to a non-existent tmp subdir, isolating the test to the workspace config_path branch they always meant to test. The 6th _load_providers test (`test_load_providers_parses_yaml_and_normalizes`) already passed because path 2 returns 7 providers and that's what that test expects — left unchanged. Verification: pytest tests/ 65/65 PASS pytest tests/test_adapter_prevalidate.py -k load_providers 6/6 PASS Closes molecule-core#129 follow-up — the unit tests were the last red on the template repo's CI. Co-Authored-By: Claude Opus 4.7 (1M context) --- adapter.py | 10 +++++-- tests/test_adapter_prevalidate.py | 48 ++++++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/adapter.py b/adapter.py index cce9992..8f87e03 100644 --- a/adapter.py +++ b/adapter.py @@ -154,6 +154,12 @@ def _normalize_provider(entry: dict): # against the site-packages copy that bit us 2026-05-04 11:08Z. _CANONICAL_ADAPTER_DIR = "/opt/adapter" +# Adjacent-to-adapter.py path. Module-level so tests can monkeypatch it +# to redirect the path-2 lookup at a controlled tmp dir. Production code +# resolves this once at import time and never touches it again — same +# semantics as before. +_TEMPLATE_DIR = os.path.dirname(os.path.abspath(__file__)) + def _load_providers(config_path: str) -> tuple: """Load the provider registry from the template's bundled config.yaml. @@ -207,9 +213,7 @@ def _load_providers(config_path: str) -> tuple: a warning; the rest of the registry survives. """ canonical_yaml = os.path.join(_CANONICAL_ADAPTER_DIR, "config.yaml") - template_yaml = os.path.join( - os.path.dirname(os.path.abspath(__file__)), "config.yaml" - ) + template_yaml = os.path.join(_TEMPLATE_DIR, "config.yaml") workspace_yaml = os.path.join(config_path, "config.yaml") # Deduplicate while preserving order — _CANONICAL_ADAPTER_DIR and # the __file__ dir collide in dev/test (when imported from diff --git a/tests/test_adapter_prevalidate.py b/tests/test_adapter_prevalidate.py index c171693..47c8154 100644 --- a/tests/test_adapter_prevalidate.py +++ b/tests/test_adapter_prevalidate.py @@ -514,8 +514,15 @@ async def test_setup_auth_token_alone_satisfies_third_party_check( # ---- _load_providers / _resolve_provider unit tests ---- -def test_load_providers_returns_builtin_when_yaml_missing(tmp_path): - """FileNotFoundError path returns the in-code defaults verbatim.""" +def test_load_providers_returns_builtin_when_yaml_missing(tmp_path, monkeypatch): + """FileNotFoundError path returns the in-code defaults verbatim. + + Monkeypatches the canonical + template paths to a non-existent dir + so only the workspace config_path is in scope. Without this, the + multi-path lookup picks up the repo-root config.yaml that ships + with the template (path 2 finds the bundled providers list and + returns it instead of falling through to builtins). + """ _install_stubs() parent_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) if parent_dir not in sys.path: @@ -523,6 +530,10 @@ def test_load_providers_returns_builtin_when_yaml_missing(tmp_path): sys.modules.pop("adapter", None) import adapter as adapter_module + nonexistent = str(tmp_path / "_isolate_canonical") + monkeypatch.setattr(adapter_module, "_CANONICAL_ADAPTER_DIR", nonexistent) + monkeypatch.setattr(adapter_module, "_TEMPLATE_DIR", nonexistent) + result = adapter_module._load_providers(str(tmp_path)) assert result == adapter_module._BUILTIN_PROVIDERS @@ -576,8 +587,12 @@ async def test_setup_routes_extra_providers( assert os.environ.get("ANTHROPIC_BASE_URL") == expected_url -def test_load_providers_falls_back_on_malformed_yaml(tmp_path, caplog): - """Malformed YAML → log warning + fallback (don't kill boot).""" +def test_load_providers_falls_back_on_malformed_yaml(tmp_path, caplog, monkeypatch): + """Malformed YAML → log warning + fallback (don't kill boot). + + Isolated from the multi-path lookup by pinning canonical + template + dirs at a non-existent path; only the workspace config_path is read. + """ _install_stubs() parent_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) if parent_dir not in sys.path: @@ -585,6 +600,10 @@ def test_load_providers_falls_back_on_malformed_yaml(tmp_path, caplog): sys.modules.pop("adapter", None) import adapter as adapter_module + nonexistent = str(tmp_path / "_isolate_canonical") + monkeypatch.setattr(adapter_module, "_CANONICAL_ADAPTER_DIR", nonexistent) + monkeypatch.setattr(adapter_module, "_TEMPLATE_DIR", nonexistent) + (tmp_path / "config.yaml").write_text("providers: [not valid yaml: {{{") import logging @@ -622,7 +641,7 @@ def test_resolve_provider_minimax_prefix_matches_minimax_provider(): assert result2["name"] == "minimax" -def test_load_providers_drops_bad_entry_keeps_rest(tmp_path, caplog): +def test_load_providers_drops_bad_entry_keeps_rest(tmp_path, caplog, monkeypatch): """Per-entry isolation: one malformed entry shouldn't nuke the registry. Pre-fix: ``_load_providers`` built the registry via a generator inside @@ -634,6 +653,9 @@ def test_load_providers_drops_bad_entry_keeps_rest(tmp_path, caplog): Post-fix: per-entry try/except drops the bad entry with a warning, rest of the registry survives. + + Isolated from the multi-path lookup so only the test's tmp config.yaml + is read. """ _install_stubs() parent_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -642,6 +664,10 @@ def test_load_providers_drops_bad_entry_keeps_rest(tmp_path, caplog): sys.modules.pop("adapter", None) import adapter as adapter_module + nonexistent = str(tmp_path / "_isolate_canonical") + monkeypatch.setattr(adapter_module, "_CANONICAL_ADAPTER_DIR", nonexistent) + monkeypatch.setattr(adapter_module, "_TEMPLATE_DIR", nonexistent) + yaml_with_typo = textwrap.dedent(""" providers: - name: good-zai @@ -690,7 +716,7 @@ def test_load_providers_drops_bad_entry_keeps_rest(tmp_path, caplog): ) -def test_load_providers_string_as_prefix_does_not_split_into_chars(tmp_path, caplog): +def test_load_providers_string_as_prefix_does_not_split_into_chars(tmp_path, caplog, monkeypatch): """A YAML field declared as list-of-strings but written as a bare string (operator forgot brackets) used to silently iterate over characters → ``('m','i','m','o','-')``. Post-fix: non-list value @@ -705,6 +731,10 @@ def test_load_providers_string_as_prefix_does_not_split_into_chars(tmp_path, cap sys.modules.pop("adapter", None) import adapter as adapter_module + nonexistent = str(tmp_path / "_isolate_canonical") + monkeypatch.setattr(adapter_module, "_CANONICAL_ADAPTER_DIR", nonexistent) + monkeypatch.setattr(adapter_module, "_TEMPLATE_DIR", nonexistent) + yaml_str_prefix = textwrap.dedent(""" providers: - name: typo-prefix @@ -723,7 +753,7 @@ def test_load_providers_string_as_prefix_does_not_split_into_chars(tmp_path, cap ) -def test_load_providers_drops_entry_without_name(tmp_path, caplog): +def test_load_providers_drops_entry_without_name(tmp_path, caplog, monkeypatch): """An entry without ``name`` is operator error — no silent fallback to ````. Drop the entry with a warning so the boot log surfaces the typo. @@ -735,6 +765,10 @@ def test_load_providers_drops_entry_without_name(tmp_path, caplog): sys.modules.pop("adapter", None) import adapter as adapter_module + nonexistent = str(tmp_path / "_isolate_canonical") + monkeypatch.setattr(adapter_module, "_CANONICAL_ADAPTER_DIR", nonexistent) + monkeypatch.setattr(adapter_module, "_TEMPLATE_DIR", nonexistent) + yaml_no_name = textwrap.dedent(""" providers: - name: good