test(tradein/scraper-kit): golden-parity + config-footgun proof for avito_imv (#2334)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 11s
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m56s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 11s
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m56s
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.
This commit is contained in:
parent
1d9e25ce07
commit
7049717782
1 changed files with 309 additions and 0 deletions
309
tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py
Normal file
309
tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py
Normal file
|
|
@ -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": "<html>warm</html>"}
|
||||
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",
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue