From 7049717782529340a1c77e89b455c990dcad36fa Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 4 Jul 2026 01:48:00 +0300 Subject: [PATCH] test(tradein/scraper-kit): golden-parity + config-footgun proof for avito_imv (#2334) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group E1 of the scraper_kit migration epic (#2277 -> #2308 -> #2334): prepares the kit side of app/services/scrapers/avito_imv.py -> scraper_kit.providers.avito.imv WITHOUT switching any real call site (estimator.py is #2337, a separate final step; house_imv_backfill.py is deliberately deferred too, see report). - Golden-parity: full evaluate_via_imv() async flow (geocode A/B + evaluate C) proven byte-identical between legacy and kit on REAL live-captured Avito API responses (tests/fixtures/avito_imv_geo_position.json + avito_imv_getdata.json), routed through the browser_fetcher transport to stay offline/deterministic. Previously this function had zero parity coverage (only _parse_price was covered, via admin.py Group A). Added cheap pure-function parity for compute_imv_cache_key/_parse_geo_position/ _parse_placement_history/_parse_suggestions too. - Config-footgun regression: kit evaluate_via_imv only reads config.scraper_proxy_url when config= is explicitly passed; called bare it silently builds a proxy-less curl_cffi session even with a real proxy configured. Two tests lock this down (without config= -> proxies=None, with config=RealScraperConfig() -> proxies wired) — manually verified by breaking scraper_kit/providers/avito/imv.py's config threading, watching the "with config" test fail on the exact proxies assertion, then reverting. No production import switched in this commit — see PR/issue report for the full caller audit and the house_imv_backfill.py deferral rationale. --- .../scrapers/test_avito_imv_kit_parity.py | 309 ++++++++++++++++++ 1 file changed, 309 insertions(+) create mode 100644 tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py diff --git a/tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py b/tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py new file mode 100644 index 00000000..b0fbdabe --- /dev/null +++ b/tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py @@ -0,0 +1,309 @@ +"""Golden-parity + config-footgun proof for `evaluate_via_imv` — issue #2334. + +Group E1 of the scraper_kit migration epic (#2277 → #2308 → #2334): migrates +`app/services/scrapers/avito_imv.py` readiness onto its +`scraper_kit.providers.avito.imv` equivalent. Unlike prior groups (#2305/#2306), +`evaluate_via_imv` had ZERO parity coverage before this file — `_parse_price` alone +is covered by `tests/scrapers/test_admin_avito_kit_parity.py` (Group A, admin debug +route), but the full 3-request async flow (geocode A/B + evaluate C) that +`app/services/estimator.py` and `app/services/house_imv_backfill.py` depend on was +never proven byte-for-byte identical between legacy and kit. + +Two independent things are proven here: + +1. **Golden parity** — legacy `evaluate_via_imv` and kit `evaluate_via_imv` give the + IDENTICAL `IMVEvaluation` on the SAME input, using REAL Avito API responses + recorded live (`tests/fixtures/avito_imv_geo_position.json` + + `avito_imv_getdata.json`, see `tests/test_avito_imv_parse.py` for provenance — + issue #565, 2026-05 capture, ЕКБ ул. Малышева 125). Both sides run through the + `browser_fetcher=` transport (see `tests/scrapers/test_avito_imv_browser_transport.py`) + so the test is offline/deterministic and never touches `curl_cffi`/proxy config — + that's a SEPARATE axis, covered below. + +2. **Config-footgun regression** — CONFIRMED by recon before this issue started: + kit `evaluate_via_imv(config: ScraperConfig | None = None, ...)` only reads + `config.scraper_proxy_url` when `config is not None`; legacy unconditionally reads + `settings.scraper_proxy_url` itself. Call the kit fn WITHOUT `config=` → own + curl_cffi session gets built with NO proxy (`proxies=None`) even though a real + proxy is configured — a silent behaviour change vs legacy. WITH + `config=RealScraperConfig()` (the established DI pattern, see + `app/services/cian_price_history.py` / `tests/test_scraper_kit_pricehistory_session_parity.py:: + test_fetch_detail_own_session_proxy_wiring_parity` for precedent) the proxy is + used identically to legacy. NOTE: neither `estimator.py` nor + `house_imv_backfill.py` is switched to the kit import in this issue (see report in + PR/issue #2334) — these tests exist to prove the KIT SIDE is safe to wire up + whenever that switch happens (#2337 for estimator.py). +""" + +from __future__ import annotations + +import asyncio +import json +import os +from pathlib import Path +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +# app.core.config.Settings() requires DATABASE_URL at import time; app.services. +# scraper_adapters (RealScraperConfig) imports app.core.config transitively. +# Mirror tests/test_scraper_kit_pricehistory_session_parity.py. +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") + +from scraper_kit.providers.avito.imv import ( + IMVAddressNotFoundError as KitIMVAddressNotFoundError, +) +from scraper_kit.providers.avito.imv import _parse_geo_position as kit_parse_geo_position +from scraper_kit.providers.avito.imv import _parse_placement_history as kit_parse_history +from scraper_kit.providers.avito.imv import _parse_suggestions as kit_parse_suggestions +from scraper_kit.providers.avito.imv import compute_imv_cache_key as kit_cache_key +from scraper_kit.providers.avito.imv import evaluate_via_imv as kit_evaluate_via_imv + +from app.core.config import settings +from app.services.scraper_adapters import RealScraperConfig +from app.services.scrapers.avito_imv import _parse_geo_position as legacy_parse_geo_position +from app.services.scrapers.avito_imv import _parse_placement_history as legacy_parse_history +from app.services.scrapers.avito_imv import _parse_suggestions as legacy_parse_suggestions +from app.services.scrapers.avito_imv import compute_imv_cache_key as legacy_cache_key +from app.services.scrapers.avito_imv import evaluate_via_imv as legacy_evaluate_via_imv +from tests.support.parity import ParityMismatchError, assert_parity + +_FIXTURES = Path(__file__).parent.parent / "fixtures" + + +def _load_fixture(name: str) -> dict[str, Any]: + with (_FIXTURES / name).open(encoding="utf-8") as f: + return json.load(f) + + +# --------------------------------------------------------------------------- +# Cheap pure-function parity (closes the gap beyond `_parse_price`, already +# covered by tests/scrapers/test_admin_avito_kit_parity.py Group A). +# --------------------------------------------------------------------------- + + +def test_compute_imv_cache_key_parity() -> None: + assert_parity( + legacy_fn=legacy_cache_key, + kit_fn=kit_cache_key, + fixtures=[ + ("ЕКБ ул. Малышева, 125", 2, 54.0, 4, 9, "panel", "cosmetic", True, False), + ], + ) + + +def test_parse_geo_position_parity_real_fixture() -> None: + data_b = _load_fixture("avito_imv_geo_position.json") + assert_parity( + legacy_fn=lambda: legacy_parse_geo_position(data_b, lat=56.840872, lon=60.654077), + kit_fn=lambda: kit_parse_geo_position(data_b, lat=56.840872, lon=60.654077), + fixtures=[()], + ) + + +def test_parse_placement_history_parity_real_fixture() -> None: + data_c = _load_fixture("avito_imv_getdata.json") + assert_parity( + legacy_fn=legacy_parse_history, + kit_fn=kit_parse_history, + fixtures=[(data_c,)], + ) + + +def test_parse_suggestions_parity_real_fixture() -> None: + data_c = _load_fixture("avito_imv_getdata.json") + assert_parity( + legacy_fn=legacy_parse_suggestions, + kit_fn=kit_parse_suggestions, + fixtures=[(data_c,)], + ) + + +# --------------------------------------------------------------------------- +# Golden parity — full evaluate_via_imv() flow on REAL captured API fixtures +# --------------------------------------------------------------------------- + +# Same address/lat/lon/lot-params as tests/test_avito_imv_parse.py:: +# test_fixtures_feed_full_evaluation_shape — keeps this test consistent with the +# established "reference realistic case" derived from the live 2026-05 capture +# (issue #565), rather than inventing a new ad-hoc fixture. +_ADDRESS = "Свердловская область, Екатеринбург, улица Малышева, 125" +_LAT = 56.840872 +_LON = 60.654077 +_LOT_PARAMS: dict[str, Any] = { + "rooms": 2, + "area_m2": 54.0, + "floor": 4, + "floor_at_home": 9, + "house_type": "panel", + "renovation_type": "cosmetic", + "has_balcony": True, + "has_loggia": False, +} + + +def _canned_browser_responses() -> list[dict[str, Any]]: + """4 responses in call order: warm-up GET, coords A, position B, get-data C. + + B and C bodies are the REAL live-captured Avito responses (fixtures/*.json); + A is synthesized with the SAME lat/lon as the fixture B (both refer to the same + ул. Малышева, 125 lookup) so the flow is internally consistent end-to-end. + """ + warmup = {"status": 200, "body": "warm"} + coords_a_body = { + "normalizedAddress": _ADDRESS, + "point": {"latitude": _LAT, "longitude": _LON}, + } + coords_a = {"status": 200, "body": json.dumps(coords_a_body)} + position_b = {"status": 200, "body": json.dumps(_load_fixture("avito_imv_geo_position.json"))} + getdata_c = {"status": 200, "body": json.dumps(_load_fixture("avito_imv_getdata.json"))} + return [warmup, coords_a, position_b, getdata_c] + + +def _run_legacy_evaluation() -> Any: + bf = AsyncMock() + bf.fetch_json = AsyncMock(side_effect=_canned_browser_responses()) + return asyncio.run(legacy_evaluate_via_imv(address=_ADDRESS, browser_fetcher=bf, **_LOT_PARAMS)) + + +def _run_kit_evaluation() -> Any: + bf = AsyncMock() + bf.fetch_json = AsyncMock(side_effect=_canned_browser_responses()) + return asyncio.run(kit_evaluate_via_imv(address=_ADDRESS, browser_fetcher=bf, **_LOT_PARAMS)) + + +def test_legacy_and_kit_imv_evaluation_are_class_distinct() -> None: + """Sanity-check ДО harness'а — same reason as test_avito_detail_kit_parity.py: + legacy/kit IMVEvaluation are different classes (different modules), so plain + `==` is always False even on identical field values. + """ + legacy_result = _run_legacy_evaluation() + kit_result = _run_kit_evaluation() + + assert type(legacy_result) is not type(kit_result) + assert legacy_result != kit_result + assert legacy_result.recommended_price == kit_result.recommended_price + assert legacy_result.geo.geo_hash == kit_result.geo.geo_hash + + +def test_evaluate_via_imv_golden_parity_real_fixtures() -> None: + """Proves kit evaluate_via_imv gives BYTE-IDENTICAL IMVEvaluation to legacy on a + real recorded Avito API response (no synthetic мокы для price/history/suggestions — + только step A синтетика для координат, зеркалящих ту же самую фикстуру B). + """ + assert_parity( + legacy_fn=_run_legacy_evaluation, + kit_fn=_run_kit_evaluation, + fixtures=[()], + ) + + +def test_parity_harness_catches_imv_field_divergence() -> None: + """Доказывает, что harness реально ЛОВИТ расхождение на этой dataclass-форме — + мутируем kit-результат (recommended_price) и убеждаемся в ParityMismatchError, + называющем именно это поле (mirrors test_avito_detail_kit_parity.py pattern). + """ + import dataclasses + + legacy_result = _run_legacy_evaluation() + kit_result = _run_kit_evaluation() + mutated_kit_result = dataclasses.replace( + kit_result, recommended_price=kit_result.recommended_price + 1 + ) + + with pytest.raises(ParityMismatchError) as exc_info: + assert_parity( + legacy_fn=lambda: legacy_result, + kit_fn=lambda: mutated_kit_result, + fixtures=[()], + ) + + assert "recommended_price" in str(exc_info.value) + + +# --------------------------------------------------------------------------- +# Config-footgun regression (#2334 CONFIRMED recon finding) +# --------------------------------------------------------------------------- +# +# evaluate_via_imv's own-curl_cffi-session path (browser_fetcher=None, the default) +# is the one that reads proxy config. Mirrors +# tests/test_scraper_proxy.py::test_avito_imv_own_session_receives_proxies (legacy +# reference) and tests/test_scraper_kit_pricehistory_session_parity.py:: +# test_fetch_detail_own_session_proxy_wiring_parity (kit config= precedent for a +# sibling provider, #2306). + + +def _mock_own_session_flow() -> MagicMock: + """Session double that lets warm-up + geocode-A pass, then fails at geocode-B + (missing geoFieldsHash) — raises IMVAddressNotFoundError AFTER the curl_cffi + session constructor already ran, which is all these tests need to capture. + """ + mock_session = MagicMock() + mock_warmup_resp = MagicMock(status_code=200) + mock_session.get = AsyncMock(return_value=mock_warmup_resp) # warm-up + geocode A + mock_geo_resp = MagicMock(status_code=200) + mock_geo_resp.json = MagicMock(return_value={"result": {"point": None}}) # no geoFieldsHash + mock_session.post = AsyncMock(return_value=mock_geo_resp) # geocode B + mock_session.close = AsyncMock() + return mock_session + + +async def _call_kit_own_session(*, config: Any = None) -> None: + with pytest.raises(KitIMVAddressNotFoundError): + await kit_evaluate_via_imv( + address="ул. Тургенева, 4", + rooms=2, + area_m2=50.0, + floor=3, + floor_at_home=9, + house_type="panel", + renovation_type="cosmetic", + has_balcony=False, + has_loggia=False, + config=config, + ) + + +def test_kit_evaluate_via_imv_without_config_drops_proxy(monkeypatch: pytest.MonkeyPatch) -> None: + """CONFIRMED footgun: kit evaluate_via_imv called WITHOUT config= builds its own + curl_cffi session with proxies=None — even though a real proxy IS configured in + settings — because `_env = config.scraper_proxy_url if config is not None else None` + never reads settings when config is absent (no fallback to env/settings inside kit). + """ + monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080") + mock_session = _mock_own_session_flow() + captured: dict[str, Any] = {} + + def _fake_session(*args: Any, **kwargs: Any) -> MagicMock: + captured.update(kwargs) + return mock_session + + with patch("curl_cffi.requests.AsyncSession", _fake_session): + asyncio.run(_call_kit_own_session(config=None)) + + assert captured.get("proxies") is None + + +def test_kit_evaluate_via_imv_with_config_uses_proxy(monkeypatch: pytest.MonkeyPatch) -> None: + """With config=RealScraperConfig() (the established DI pattern for this epic) the + kit path reads the SAME settings.scraper_proxy_url as legacy and wires it into the + curl_cffi session identically — this is the regression guard: if someone later + "simplifies away" the config kwarg threading in evaluate_via_imv, this test fails + on `captured.get("proxies") is None` instead of the expected proxy dict. + """ + monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080") + mock_session = _mock_own_session_flow() + captured: dict[str, Any] = {} + + def _fake_session(*args: Any, **kwargs: Any) -> MagicMock: + captured.update(kwargs) + return mock_session + + with patch("curl_cffi.requests.AsyncSession", _fake_session): + asyncio.run(_call_kit_own_session(config=RealScraperConfig())) + + assert captured.get("proxies") == { + "http": "http://test-proxy.local:8080", + "https": "http://test-proxy.local:8080", + } -- 2.45.3