7546868535
All checks were successful
CI Trade-In / changes (pull_request) Successful in 17s
CI / changes (pull_request) Successful in 16s
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 1m50s
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.
210 lines
9.5 KiB
Python
210 lines
9.5 KiB
Python
"""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)
|