1 changed files with 210 additions and 0 deletions
|
|
@ -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)
|
||||
Loading…
Add table
Reference in a new issue