test(tradein/scraper-kit): prove cian_valuation kit path safe for #2337 (#2335)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 6s
CI / changes (pull_request) Successful in 7s
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 1m35s

scraper_kit.providers.cian.valuation.estimate_via_cian_valuation (and
session.load_session) already existed with mandatory config: ScraperConfig
(no default) — this was ground prepared ahead of Group E2, not new work here.
This change proves it's safe to switch estimator.py's import to it (#2337):

- Extends the existing _parse_valuation_state golden-parity test with a
  full-pipeline parity test (cache-miss -> load_session -> curl_cffi fetch ->
  extract_state -> auth-check -> parse -> sanity-check), wired with
  config=RealScraperConfig() per the #2306 convention
  (app/services/cian_price_history.py).
- Adds a regression guard proving estimate_via_cian_valuation raises TypeError
  when config= is omitted, and documents why that must stay mandatory:
  estimator.py's call site (line ~3143) sits inside a blanket
  `except Exception` that only does `logger.warning(..., exc)` (no exc_info,
  no logger.exception) -- below GlitchTip's ERROR-level capture threshold
  (see LoggingIntegration(event_level=logging.ERROR) in app/main.py), so a
  TypeError from a missing config= would be logged but invisible in
  GlitchTip/Sentry, indistinguishable from ordinary graceful degradation.
- Documents an asymmetry vs. the task's initial assumption: kit's
  load_session requires config= mandatorily, but mark_session_invalid does
  NOT take a config parameter at all (confirmed by inspection, not a
  footgun -- just noting the actual contract).
- Confirms app/services/estimator.py is not the only real caller:
  app/tasks/cian_history_backfill.py also imports estimate_via_cian_valuation
  directly and deliberately stays on the legacy import for now (see its own
  comment referencing issue #2308) -- out of scope for this change, flagged
  for whoever picks up cian_history_backfill's migration.

Full backend test suite: 3211 passed, 6 skipped, 1 pre-existing unrelated
failure (test_search_api.py::test_search_cache_hit, 401 vs 200 auth issue,
reproduces identically on main without this change).
This commit is contained in:
bot-backend 2026-07-04 01:45:46 +03:00
parent 1d9e25ce07
commit 53287da886

View file

@ -21,6 +21,7 @@ Strangler-инвариант (#2133 cian): новая scraper_kit-копия cia
from __future__ import annotations
import asyncio
import dataclasses
import json
import os
@ -52,6 +53,14 @@ from scraper_kit.providers.cian.serp import _format_address as new_format_addres
from scraper_kit.providers.cian.valuation import (
_parse_valuation_state as new_parse_valuation,
)
from scraper_kit.providers.cian.valuation import (
estimate_via_cian_valuation as new_estimate_via_cian_valuation,
)
# #2335 (Group E2): estimate_via_cian_valuation full-pipeline parity + mandatory-config
# regression guard. RealScraperConfig — тот же read-only адаптер над settings, что и
# #2306's cian_price_history.py (см. app/services/scraper_adapters.py).
from app.services.scraper_adapters import RealScraperConfig
# СТАРЫЙ (app.services.scrapers) cian-провайдер.
from app.services.scrapers.cian import CianScraper as OldCianScraper
@ -71,6 +80,10 @@ from app.services.scrapers.cian_state_parser import extract_state as old_extract
from app.services.scrapers.cian_valuation import (
_parse_valuation_state as old_parse_valuation,
)
from app.services.scrapers.cian_valuation import (
estimate_via_cian_valuation as old_estimate_via_cian_valuation,
)
from tests.support.parity import assert_parity
# ── Fixtures: SERP offers → _cianConfig push HTML ────────────────────────────
@ -435,6 +448,153 @@ def test_valuation_parse_state_parity() -> None:
assert dataclasses.asdict(old_res) == dataclasses.asdict(new_res)
# ── estimate_via_cian_valuation full-pipeline parity (#2335, Group E2) ───────
#
# test_valuation_parse_state_parity above only covers the pure parsing helper.
# These tests exercise the FULL async call path (cache lookup → load_session →
# curl_cffi fetch → extract_state → auth-check → parse → sanity-check → cache
# write), with config=RealScraperConfig() wired per the #2306 convention
# (app/services/cian_price_history.py), proving the kit path is safe to switch
# to whenever #2337 wires estimator.py's call site.
def _valuation_mfe_html(state: dict[str, Any]) -> str:
"""Wrap a valuation state dict as a valid _cianConfig push() HTML page."""
state_json = json.dumps(state, ensure_ascii=False)
return (
"<html><body><script>\n"
"window._cianConfig['valuation-for-agent-frontend'] = "
"window._cianConfig['valuation-for-agent-frontend'] || [];\n"
"window._cianConfig['valuation-for-agent-frontend'].push({"
f"key: 'initialState', value: {state_json}, priority: 10, filter: null"
"});\n</script></body></html>"
)
def _make_fake_async_session_cls(html: str) -> type:
"""Stand-in for curl_cffi.requests.AsyncSession — returns canned HTML on .get()."""
class _FakeAsyncSession:
def __init__(self, *args: Any, **kwargs: Any) -> None:
pass
async def __aenter__(self) -> _FakeAsyncSession:
return self
async def __aexit__(self, *exc: object) -> bool:
return False
async def get(self, url: str, allow_redirects: bool = True) -> SimpleNamespace:
return SimpleNamespace(status_code=200, text=html)
return _FakeAsyncSession
def test_estimate_via_cian_valuation_full_pipeline_parity(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Full estimate_via_cian_valuation() pipeline parity — not just the pure parser.
Mocks curl_cffi.AsyncSession (canned MFE HTML) + load_session (fixed cookies) on
BOTH legacy and kit modules, cache-miss on a MagicMock db, and drives both async
call paths with identical inputs (kit wired with config=RealScraperConfig()).
"""
html = _valuation_mfe_html(_valuation_state())
fake_session_cls = _make_fake_async_session_cls(html)
def _run_legacy() -> Any:
monkeypatch.setattr("app.services.scrapers.cian_valuation.AsyncSession", fake_session_cls)
monkeypatch.setattr(
"app.services.scrapers.cian_valuation.load_session",
lambda db: {"DMIR_AUTH": "token"},
)
db = MagicMock()
db.execute.return_value.mappings.return_value.first.return_value = None # cache miss
return asyncio.run(
old_estimate_via_cian_valuation(
db,
address="Свердловская обл, Екатеринбург",
total_area=50.0,
rooms_count=2,
floor=5,
total_floors=16,
use_cache=False,
)
)
def _run_kit() -> Any:
monkeypatch.setattr("scraper_kit.providers.cian.valuation.AsyncSession", fake_session_cls)
monkeypatch.setattr(
"scraper_kit.providers.cian.valuation.load_session",
lambda db, *, config: {"DMIR_AUTH": "token"},
)
db = MagicMock()
db.execute.return_value.mappings.return_value.first.return_value = None # cache miss
return asyncio.run(
new_estimate_via_cian_valuation(
db,
config=RealScraperConfig(),
address="Свердловская обл, Екатеринбург",
total_area=50.0,
rooms_count=2,
floor=5,
total_floors=16,
use_cache=False,
)
)
assert_parity(
legacy_fn=_run_legacy,
kit_fn=_run_kit,
fixtures=[()],
)
def test_kit_estimate_via_cian_valuation_requires_config() -> None:
"""Kit's estimate_via_cian_valuation MUST require config= (no default) — mandatory.
Regression guard (#2335): config: ScraperConfig has NO default in the kit
signature, so calling it without config= raises TypeError immediately (at
argument-binding time, before any coroutine runs). This is intentional and
must stay this way: estimator.py's call site (#2337) sits inside a blanket
`except Exception` used for graceful multi-source degradation a future
change that gave config= a default would let a wiring mistake silently
degrade into "source unavailable" instead of raising loudly at call-time.
See #2306 for the precedent this guards against (silent-degradation footgun
from an optional dependency parameter).
"""
db = MagicMock()
with pytest.raises(TypeError, match="config"):
new_estimate_via_cian_valuation( # not awaited — TypeError raises at call time
db,
address="Свердловская обл, Екатеринбург",
total_area=50.0,
rooms_count=2,
floor=5,
)
def test_kit_cian_session_config_requirement_asymmetry() -> None:
"""scraper_kit.providers.cian.session: load_session requires config=, but
mark_session_invalid does NOT.
Confirmed by direct inspection for #2335 — the two helpers estimate_via_cian_valuation
calls internally are NOT symmetric re: config, unlike the assumption in the original
task brief ("the kit version of those also requires config mandatorily"). Documented
here as a regression guard so a future refactor that changes either signature is a
deliberate, visible decision rather than an accidental drift.
"""
from scraper_kit.providers.cian.session import load_session, mark_session_invalid
db = MagicMock()
with pytest.raises(TypeError, match="config"):
load_session(db) # missing mandatory config=
db2 = MagicMock()
mark_session_invalid(db2, 42) # no config parameter at all — must NOT raise
db2.execute.assert_called_once()
# ── newbuilding pure-экстракторы parity ──────────────────────────────────────