Merge pull request 'test(tradein/scraper-kit): prove yandex_valuation kit path safe for #2337 (#2336)' (#2345) from feat/tradein-migrate-yandex-valuation-kit into main

2026-07-03 23:04:13 +00:00 · 2026-07-03 23:04:13 +00:00 · f872f5636b
commit f872f5636b
parent aa2b33f264 7546868535
1 changed files with 210 additions and 0 deletions
--- a/tradein-mvp/backend/tests/scrapers/test_yandex_valuation_kit_migration.py
+++ b/tradein-mvp/backend/tests/scrapers/test_yandex_valuation_kit_migration.py
@ -0,0 +1,210 @@
 """Issue #2336 (Group E3 of scraper_kit migration epic #2277/#2308).
 `scraper_kit.providers.yandex.valuation.YandexValuationScraper` is already a
 strangler-copy (#2133) of `app.services.scrapers.yandex_valuation`, and already used
 via the kit path in `admin.py::scrape_yandex_valuation` (#2305, Group A) with the
 established DI convention: `config=RealScraperConfig()`,
 `delay_provider=get_scraper_delay`, `proxy_provider=_kit_proxy_provider()`.
 `app/services/estimator.py` (the real, paying-user-facing caller) still imports the
 LEGACY module directly and instantiates `YandexValuationScraper()` with NO arguments
 (`estimator.py:598`). Switching that call site to the kit class is #2337's job — out
 of scope here. This file exists to prove #2336's two confirmed risk patterns for
 whoever does that switch, so a regression is caught in CI rather than silently in prod:
 1. **Mandatory `config: ScraperConfig`** (same shape as the cian_valuation footgun,
   #2335) — omitting it raises `TypeError`. `estimator.py`'s call site sits inside a
   blanket `except Exception` (see `_get_or_fetch_yandex_valuation_cached`, lines
   ~596-606) that logs via `logger.warning(..., e)` — no `exc_info`/traceback, and at
   the SAME log severity as ordinary transient fetch failures. A `TypeError` from a
   forgotten `config=` would read like "Yandex source unavailable" in prod logs, not
   "wiring bug", even though the message text itself does end up in the log line.
 2. **Optional `delay_provider`** — if omitted, silently falls back to the hardcoded
   class-default `request_delay_sec = 5.0`, discarding whatever an admin configured via
   the DB-backed `scraper_settings` table (`get_scraper_delay`, cached 60s,
   `max(per_source, global)`). No exception, no log — just quietly weaker anti-ban
   throttling if an admin had raised it after ban trouble.
 Also extends `tests/test_scraper_kit_yandex_golden_parity.py::test_valuation_parse_parity`
 (which already proves `.parse()` output identity using a duck-typed `SimpleNamespace`
 config) to the REAL production construction path — `RealScraperConfig()` +
 `get_scraper_delay` — via `tests/support/parity.assert_parity` (issue #2304 harness),
 per the epic's parity-gate convention.
 """
 from __future__ import annotations
 import os
 from unittest.mock import patch
 import pytest
 # Old app.services.scrapers.yandex_valuation and app.services.scraper_adapters both
 # import app.core.config.settings=Settings(), which requires DATABASE_URL. Offline
 # parsing/construction never touches a real DB — a fake DSN is enough (same trick as
 # tests/test_scraper_kit_yandex_golden_parity.py).
 os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
 from scraper_kit.providers.yandex.valuation import (
    YandexValuationScraper as KitYandexValuationScraper,
 )
 from app.services import scraper_settings
 from app.services.scraper_adapters import RealScraperConfig
 from app.services.scraper_settings import get_scraper_delay
 from app.services.scrapers.yandex_valuation import (
    YandexValuationScraper as LegacyYandexValuationScraper,
 )
 from tests.support.parity import assert_parity
 _VALUATION_HTML = """
 <html><body>
 Дом 2016 года 25 этажей 2,7 м потолки Лифт 42 объекта Панорама
 48,5 м², 1-комнатная, 5 этаж 5,1 млн ₽ 105 155 ₽ за м² 23.10.2023
 В экспозиции 945 дней Снято 24.05.2024
 32 м², студия, 2 этаж 3,2 млн ₽ 100 000 ₽ за м² 01.02.2024 В продаже
 </body></html>
 """
 _PARSE_KWARGS = {
    "address": "Екатеринбург, ул. Ленина, 5",
    "offer_category": "APARTMENT",
    "offer_type": "SELL",
    "page": 1,
    "source_url": "https://realty.yandex.ru/otsenka-kvartiry-po-adresu-onlayn/?address=test",
 }
 # ---------------------------------------------------------------------------
 # Footgun #1 — mandatory `config`
 # ---------------------------------------------------------------------------
 def test_kit_valuation_requires_config() -> None:
    """Omitting `config=` raises TypeError — documents the mandatory-DI contract.
    Guards the exact risk flagged for #2336: if a future call site (#2337, switching
    estimator.py to the kit class) forgets `config=`, this must fail loud in CI, not
    silently in prod inside estimator.py's blanket except-block.
    """
    with pytest.raises(TypeError):
        KitYandexValuationScraper()  # type: ignore[call-arg]
 # ---------------------------------------------------------------------------
 # Footgun #2 — `delay_provider` silently falls back to hardcoded 5.0s
 # ---------------------------------------------------------------------------
 def test_kit_valuation_without_delay_provider_uses_hardcoded_default() -> None:
    """No `delay_provider=` -> `request_delay_sec` stays at the class-default 5.0s.
    This is the regression this test guards: a real call site that forgets to wire
    `delay_provider=get_scraper_delay` silently reverts an admin-tuned anti-ban delay
    (e.g. raised after a ban) to 5.0s -- no error, no log, just quietly weaker
    throttling.
    """
    scraper = KitYandexValuationScraper(config=RealScraperConfig())
    assert scraper.request_delay_sec == 5.0
    assert KitYandexValuationScraper.request_delay_sec == 5.0  # class default untouched
 def test_kit_valuation_with_delay_provider_uses_configured_value() -> None:
    """`delay_provider=get_scraper_delay` -> instance picks up whatever it returns.
    Controls the return value via patch so the assertion doesn't depend on a real DB
    row / DB availability -- this proves the WIRING (delay_provider is actually called
    and its return value actually lands in request_delay_sec), not
    `scraper_settings`'s own DB-fallback behaviour (covered elsewhere, e.g.
    `tests/test_yandex_scrapers_delay_wiring.py` for the legacy module).
    """
    with patch("app.services.scraper_settings.get_scraper_delay", return_value=12.5) as mock_delay:
        # Attribute lookup on the MODULE (not the bare name imported at file-top)
        # so this resolves to the patched mock, not the original function object
        # `from ... import get_scraper_delay` would have bound at import time.
        scraper = KitYandexValuationScraper(
            config=RealScraperConfig(), delay_provider=scraper_settings.get_scraper_delay
        )
    mock_delay.assert_called_once_with("yandex_valuation")
    assert scraper.request_delay_sec == 12.5
    assert scraper.request_delay_sec != 5.0  # explicitly NOT the hardcoded fallback
 def test_kit_valuation_delay_provider_receives_scraper_name() -> None:
    """delay_provider is invoked with `self.name` ('yandex_valuation'), not a typo'd string.
    `get_scraper_delay` aliases 'yandex_valuation' to the shared 'yandex' DB row today
    (`_KEY_ALIASES` in scraper_settings.py), but a hardcoded/mismatched source string
    passed at the call site would be a latent bug if that alias ever changes.
    """
    calls: list[str] = []
    def _spy_delay_provider(source: str) -> float:
        calls.append(source)
        return 9.0
    scraper = KitYandexValuationScraper(
        config=RealScraperConfig(), delay_provider=_spy_delay_provider
    )
    assert calls == ["yandex_valuation"]
    assert scraper.request_delay_sec == 9.0
 # ---------------------------------------------------------------------------
 # Extended parity: full instantiation/config-wiring path, not just parsing
 # ---------------------------------------------------------------------------
 def test_full_construction_and_parse_parity_with_real_config_wiring() -> None:
    """Legacy no-arg ctor vs kit ctor with the REAL production DI convention.
    `tests/test_scraper_kit_yandex_golden_parity.py::test_valuation_parse_parity`
    already proves `.parse()` output identity using a duck-typed `SimpleNamespace`
    config. This test additionally proves the REAL wiring convention used in
    production (`config=RealScraperConfig()`, `delay_provider=get_scraper_delay` --
    same as `admin.py::scrape_yandex_valuation`, #2305 Group A) constructs without
    error and does not change parsing output vs. the legacy no-arg constructor.
    """
    def _legacy_parse(html: str) -> object:
        return LegacyYandexValuationScraper().parse(html, **_PARSE_KWARGS)
    def _kit_parse(html: str) -> object:
        scraper = KitYandexValuationScraper(
            config=RealScraperConfig(), delay_provider=get_scraper_delay
        )
        return scraper.parse(html, **_PARSE_KWARGS)
    assert_parity(
        legacy_fn=_legacy_parse,
        kit_fn=_kit_parse,
        fixtures=[_VALUATION_HTML],
    )
 def test_parity_harness_catches_valuation_construction_divergence() -> None:
    """Sanity: assert_parity actually fails if kit construction changes parsed output.
    Proves the extended-coverage test above is not vacuously green -- mutates the
    kit-side house metadata and confirms `assert_parity` reports the exact field.
    """
    from tests.support.parity import ParityMismatchError
    scraper = KitYandexValuationScraper(
        config=RealScraperConfig(), delay_provider=get_scraper_delay
    )
    kit_result = scraper.parse(_VALUATION_HTML, **_PARSE_KWARGS)
    mutated = kit_result.model_copy(
        update={"house": kit_result.house.model_copy(update={"year_built": 1900})}
    )
    with pytest.raises(ParityMismatchError) as exc_info:
        assert_parity(
            legacy_fn=lambda: LegacyYandexValuationScraper().parse(
                _VALUATION_HTML, **_PARSE_KWARGS
            ),
            kit_fn=lambda: mutated,
            fixtures=[()],
        )
    assert "year_built" in str(exc_info.value)