From 7546868535a95b1a3d05bba7c0a3ea81545dc833 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 4 Jul 2026 01:49:51 +0300 Subject: [PATCH] test(tradein/scraper-kit): prove yandex_valuation kit-wiring safety (#2336) Group E3 recon confirmed `scraper_kit.providers.yandex.valuation` is already a strangler-copy (#2133) and already used via admin.py's debug route (#2305, Group A) with the established config=RealScraperConfig()/delay_provider= get_scraper_delay convention. estimator.py (the real caller, #2337's scope) still imports the legacy module directly with no arguments -- not touched here. Adds regression coverage for the two confirmed footguns before #2337 switches that call site: - mandatory `config: ScraperConfig` raises TypeError when omitted (same discipline as the cian_valuation footgun, #2335) - optional `delay_provider` silently falls back to hardcoded 5.0s when not wired, discarding admin-tuned anti-ban throttling (scraper_settings. get_scraper_delay) Also extends the existing `.parse()` golden-parity test (test_scraper_kit_ yandex_golden_parity.py, SimpleNamespace config) with an assert_parity-based proof over the REAL production construction path (RealScraperConfig() + get_scraper_delay), plus a harness-catches-divergence sanity check. No production code changed -- app/services/scrapers/yandex_valuation.py and scraper_kit/providers/yandex/valuation.py are both pre-existing. --- .../test_yandex_valuation_kit_migration.py | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 tradein-mvp/backend/tests/scrapers/test_yandex_valuation_kit_migration.py diff --git a/tradein-mvp/backend/tests/scrapers/test_yandex_valuation_kit_migration.py b/tradein-mvp/backend/tests/scrapers/test_yandex_valuation_kit_migration.py new file mode 100644 index 00000000..59c0d1f5 --- /dev/null +++ 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 = """ + +Дом 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 В продаже + +""" + +_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) -- 2.45.3