You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up from #688 review (PR for #687). Two non-blocking nits flagged by code-reviewer:
Autouse fixture for _brand_manifest_path_warned reset. Every existing test for the brand_manifest non-standard-path warning does the same monkeypatch.setattr(_url_mod, "_brand_manifest_path_warned", set()) boilerplate. A future test that forgets the reset would silently inherit state from prior tests and flake intermittently. Centralizing as an autouse fixture eliminates the boilerplate and removes the failure mode.
Cross-adapter dedup test. After Extend brand_manifest non-standard-path warning to inline-object branch #687 centralized the dedup state in _url.py, the dedup set is shared across get_products and create_media_buy. That's the intended behaviour (same offending URL through any adapter only warns once per process), but no test documents the semantic. If someone refactors back to per-module state, nothing catches it.
Scope
tests/conftest.py: add scoped autouse fixture that resets adcp.compat.legacy.v2_5._url._brand_manifest_path_warned between tests.
Remove the now-redundant monkeypatch.setattr(_url_mod, "_brand_manifest_path_warned", set()) calls from the existing 10 tests in tests/test_legacy_adapter_v2_5_get_products.py and tests/test_legacy_adapter_v2_5_media_buy.py.
Add one cross-adapter test asserting that the same CDN URL through get_products.adapt_request and create_media_buy.adapt_request warns exactly once.
Out of scope
Bounded dedup cache (separate concern; per-deployment cardinality is small so unlikely to matter in practice).
Background
Follow-up from #688 review (PR for #687). Two non-blocking nits flagged by
code-reviewer:Autouse fixture for
_brand_manifest_path_warnedreset. Every existing test for the brand_manifest non-standard-path warning does the samemonkeypatch.setattr(_url_mod, "_brand_manifest_path_warned", set())boilerplate. A future test that forgets the reset would silently inherit state from prior tests and flake intermittently. Centralizing as an autouse fixture eliminates the boilerplate and removes the failure mode.Cross-adapter dedup test. After Extend brand_manifest non-standard-path warning to inline-object branch #687 centralized the dedup state in
_url.py, the dedup set is shared acrossget_productsandcreate_media_buy. That's the intended behaviour (same offending URL through any adapter only warns once per process), but no test documents the semantic. If someone refactors back to per-module state, nothing catches it.Scope
tests/conftest.py: add scoped autouse fixture that resetsadcp.compat.legacy.v2_5._url._brand_manifest_path_warnedbetween tests.monkeypatch.setattr(_url_mod, "_brand_manifest_path_warned", set())calls from the existing 10 tests intests/test_legacy_adapter_v2_5_get_products.pyandtests/test_legacy_adapter_v2_5_media_buy.py.get_products.adapt_requestandcreate_media_buy.adapt_requestwarns exactly once.Out of scope
strip_url_schemerename + IPv6 silent failure (nits filed during fix(compat): extract hostname from brand_manifest URLs with paths #679 review; cosmetic at this point).