All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m36s
Удалены (0 runtime importers в app/, scripts/, packages/; проверено grep'ом, включая intra-legacy импорты в оставшихся 21 legacy-модуле scrapers/): - backend/app/services/scrapers/avito_imv.py - backend/app/services/scrapers/domclick_detail.py - backend/app/services/scrapers/ekb_geoportal_client.py - backend/app/services/scrapers/yandex_detail.py - backend/app/services/scrapers/yandex_valuation.py Тесты — удалены (pure legacy-vs-kit parity, kit-only coverage уже есть в другом месте): - tests/scrapers/test_ekb_geoportal_client_kit_parity.py - tests/scrapers/test_domclick_detail_kit_parity.py Тесты — конвертированы в kit-only (импорт легаси заменён на scraper_kit.*, покрытие сохранено без потерь): - tests/scrapers/test_avito_imv_kit_parity.py (parity-тесты убраны, config-footgun regression на kit-стороне оставлен) - tests/scrapers/test_avito_imv_browser_transport.py - tests/scrapers/test_yandex_valuation_kit_migration.py (parity убрана, footgun- регрессия mandatory config/delay_provider оставлена) - tests/scrapers/test_domclick_detail.py (+ exceptions переведены на scraper_kit.domclick_exceptions, т.к. kit parse_detail_html поднимает их) - tests/test_avito_imv_parse.py - tests/test_ekb_geoportal_ingest.py (client-часть; ingest/geocoder-часть не трогали — не зависят от удалённого модуля) - tests/test_yandex_detail.py - tests/test_yandex_detail_structural.py - tests/test_yandex_valuation.py (YandexValuationScraper() → config=_KIT_CONFIG, kit конструктор требует обязательный config) - tests/test_yandex_valuation_save.py - tests/test_extval_house_id_write_path.py (только yandex_valuation-часть; cian_valuation остался нетронутым — живой легаси-модуль) - tests/test_yandex_history_area_filter.py Тесты — частично отредактированы (убраны только части про удалённые модули, живые легаси-модули/их parity не тронуты): - tests/scrapers/test_admin_domclick_ingest_kit_parity.py (base.py/ScrapedLot parity остался) - tests/scrapers/test_admin_yandex_kit_parity.py (yandex_realty parity остался) - tests/scrapers/test_admin_avito_kit_parity.py (avito.py/avito_houses.py parity остался; _parse_price parity убран) - tests/scrapers/test_avito_unix_date_tz_consistency.py (IMV-ветка переведена на kit avito.imv, легаси avito/avito_houses/avito_shared/avito_detail не тронуты) - tests/tasks/test_yandex_detail_backfill.py (subject — kit-задача, уже вызывает scraper_kit.providers.yandex.detail; save_detail_enrichment coverage-тесты внизу файла переведены на kit) - tests/test_scraper_kit_yandex_golden_parity.py (detail/valuation parity убраны; serp/newbuilding/house_type_normalizer/build_url для yandex_realty/ yandex_newbuilding не тронуты) - tests/test_scraper_proxy.py (YandexValuation proxy-тесты переведены на kit config= DI; test_avito_imv_own_session_receives_proxies удалён — дублирует test_avito_imv_kit_parity.py footgun-тесты) - tests/test_yandex_scrapers_delay_wiring.py (yandex_detail/yandex_valuation → kit delay_provider=; yandex_realty/yandex_newbuilding не тронуты) - tests/test_scraper_kit_group_c_backfill_kit_parity.py (docstring уточнение, без функциональных изменений) Полный backend-suite зелёный (3253 passed, 6 skipped) кроме известного pre-existing flake test_search_api.py::test_search_cache_hit (#2208, не регрессия этого PR). ruff check + ruff format — чисто на всех изменённых файлах.
132 lines
6 KiB
Python
132 lines
6 KiB
Python
"""Config-footgun regression for kit `evaluate_via_imv` — issue #2334.
|
|
|
|
Group E1 of the scraper_kit migration epic (#2277 → #2308 → #2334 → final deletion
|
|
step). Legacy `app/services/scrapers/avito_imv.py` was deleted once the kit provider
|
|
(`scraper_kit.providers.avito.imv`) reached golden parity and no runtime caller
|
|
referenced the legacy module anymore — the legacy-vs-kit comparison tests that used
|
|
to live here were removed together with the legacy import.
|
|
|
|
What remains is a standalone kit-only regression, still worth keeping on its own:
|
|
|
|
**Config-footgun regression** — CONFIRMED by recon before issue #2334 started:
|
|
kit `evaluate_via_imv(config: ScraperConfig | None = None, ...)` only reads
|
|
`config.scraper_proxy_url` when `config is not None`; the deleted legacy module
|
|
unconditionally read `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 the old code. 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 wired
|
|
correctly. NOTE: neither `estimator.py` nor `house_imv_backfill.py` needs this
|
|
config= kwarg wired for `evaluate_via_imv` specifically — these tests exist to prove
|
|
the KIT SIDE stays safe against a "simplify away the config kwarg" regression.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import os
|
|
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 evaluate_via_imv as kit_evaluate_via_imv
|
|
|
|
from app.core.config import settings
|
|
from app.services.scraper_adapters import RealScraperConfig
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 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, now gone with the legacy module) 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 used to 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",
|
|
}
|