diff --git a/tests/_contract_support.py b/tests/_contract_support.py index d336b07..2e8ca44 100644 --- a/tests/_contract_support.py +++ b/tests/_contract_support.py @@ -1,13 +1,27 @@ -"""Request-side contract support: SDK method → OpenAPI endpoint mapping and helpers. +"""Contract test infrastructure: SDK ↔ spec mapping + the EXCLUSIONS allowlist. -This module is test infrastructure. It lives in ``tests/`` (not ``kalshi/``) so that -users importing the SDK don't get spec-parsing code shipped in the PyPI wheel. +This module is test infrastructure. It lives in ``tests/`` (not ``kalshi/``) so +that users importing the SDK don't get spec-parsing code shipped in the PyPI +wheel. -The map covers ``path``, ``query``, and ``requestBody`` surface. Body schemas -for POST/PUT endpoints are referenced via ``MethodEndpointEntry.request_body_schema`` -(a spec ``$ref`` string) and resolved via ``_resolve_request_body_schema``. Drift -tests that consume this infrastructure (``TestRequestParamDrift``, -``TestRequestBodyDrift``) land in subsequent v0.8.0 tasks. +``METHOD_ENDPOINT_MAP`` covers the request side: ``path``, ``query``, and +``requestBody`` surface. Body schemas for POST/PUT/DELETE-with-body endpoints +are referenced via ``MethodEndpointEntry.request_body_schema`` (a spec ``$ref`` +string) and resolved via ``_resolve_request_body_schema``. + +``EXCLUSIONS`` is consulted by FOUR drift checks: + + - ``TestRequestParamDrift`` — query/path kwargs vs. spec parameters + - ``TestRequestBodyDrift`` — request body model properties vs. spec + - ``TestSpecDrift`` — REST response model fields vs. OpenAPI + (``test_additive_drift`` + ``test_required_drift``) + - ``TestWsSpecDrift`` — WS payload model fields vs. AsyncAPI + (``test_ws_additive_drift`` + ``test_ws_required_drift``) + +A single ``(sdk_fqn, field_name)`` key may serve more than one check — e.g. +``CreateOrderRequest`` and ``CreateRFQRequest`` are both request bodies AND +response schemas in the spec, so a field exclusion is reused by both sides. +Add a new entry once at the model+field coordinate; do NOT duplicate it. Async siblings are derived at test time via ``Async`` substitution with identical method names. Do NOT add separate async entries to the map. @@ -737,24 +751,25 @@ class Exclusion: # --------------------------------------------------------------------------- -# EXCLUSIONS allowlist for request-side drift tests +# EXCLUSIONS allowlist for drift tests (request + response) # --------------------------------------------------------------------------- # Keys are ``(sdk_fqn, param_or_field_name)`` tuples. Values are ``Exclusion`` -# dataclasses with a required ``reason`` string. An entry declares "this is -# not drift; here's why." Tests that fail to find the corresponding drift -# should also fail (see ``test_exclusion_map_is_current``), so stale entries -# don't accumulate. +# dataclasses with required ``reason`` and ``kind`` fields. An entry declares +# "this is not drift; here's why." # -# BOOTSTRAP (v0.8.0) — two classes of entries: -# 1. ``CreateOrderRequest`` spec fields the SDK deliberately omits (cent-form -# redundant with ``_dollars`` variants; deprecated-in-spec). -# 2. ``cursor`` on every ``list_all`` method — paginator-handled internally; -# the kwarg is absent from the method signature by design. +# Consumers (see this module's docstring for full coverage): +# - request side : ``TestRequestParamDrift``, ``TestRequestBodyDrift`` +# - response side: ``TestSpecDrift`` (additive + required), +# ``TestWsSpecDrift`` (additive + required) # -# Additional entries for ``AmendOrderRequest`` get appended in Task 3 (after -# that model is created). Do NOT preload them here — forward references to -# a model that doesn't exist yet will trip mypy on import. - +# A single key may serve more than one check — e.g. ``CreateOrderRequest`` is +# both a request body and a response schema in the spec, so a field-level +# exclusion is reused by both sides. Add a new entry once at the model+field +# coordinate; do NOT duplicate it per drift test. +# +# Tests that fail to find the corresponding drift should also fail (see +# ``test_exclusion_map_is_current``), so stale entries can't silently +# accumulate. EXCLUSIONS: dict[tuple[str, str], Exclusion] = { # --- CreateOrderRequest spec fields deliberately not on the model --- ("kalshi.models.orders.CreateOrderRequest", "yes_price"): Exclusion( @@ -1111,6 +1126,31 @@ class Exclusion: reason="paginator-handled; not a caller-facing kwarg on list_all", kind="paginator_handled", ), + # --- Response-side additive drift: spec fields the SDK intentionally omits --- + # Consumed by TestSpecDrift / TestWsSpecDrift in addition to any request-side + # body/param checks above that happen to share the same model+field key. + # Each entry is a spec-documented deviation; flipping it to a hard fail + # would surface as a real bug (see issue #157 stack for the v2.1.0 + # ``Balance.balance_dollars`` regression that motivated this PR). + ("kalshi.models.markets.Market", "response_price_units"): Exclusion( + reason="spec marks DEPRECATED; superseded by price_level_structure / price_ranges", + kind="spec_deprecated", + ), + ("kalshi.models.orders.Order", "action"): Exclusion( + reason="spec marks Deprecated; superseded by outcome_side / book_side", + kind="spec_deprecated", + ), + ("kalshi.models.markets.Orderbook", "orderbook_fp"): Exclusion( + reason=( + "SDK reshapes the orderbook_fp nested object into yes/no level lists " + "at the resource layer; the model itself holds the unwrapped lists" + ), + kind="wire_normalization", + ), + ("kalshi.ws.models.ticker.TickerPayload", "time"): Exclusion( + reason="spec marks deprecated; superseded by ts_ms", + kind="spec_deprecated", + ), } diff --git a/tests/test_contracts.py b/tests/test_contracts.py index 87822c6..94c2f95 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -375,6 +375,11 @@ def _classify_drift( Returns (additive_issues, required_issues). Both are warnings, not failures (SDK is intentionally permissive). + + Per-field skips are honored in this order: + 1. ``entry.ignored_fields`` — per-contract-entry allowlist (legacy). + 2. ``EXCLUSIONS[(entry.sdk_model, field_name)]`` — typed, reasoned + allowlist shared with the request-side drift checks (#157). """ additive: list[str] = [] required_issues: list[str] = [] @@ -385,6 +390,8 @@ def _classify_drift( for spec_field_name in spec_fields: if spec_field_name in entry.ignored_fields: continue + if (entry.sdk_model, spec_field_name) in EXCLUSIONS: + continue if spec_field_name not in reverse_map: additive.append(f"Spec field '{spec_field_name}' has no SDK mapping") @@ -393,6 +400,8 @@ def _classify_drift( for req_field in required_fields: if req_field in entry.ignored_fields: continue + if (entry.sdk_model, req_field) in EXCLUSIONS: + continue sdk_name = reverse_map.get(req_field) if sdk_name and sdk_name in model_class.model_fields: field_info = model_class.model_fields[sdk_name] @@ -758,6 +767,8 @@ def test_ws_required_drift(self, entry: ContractEntry) -> None: for req_field in ws_required: if req_field in entry.ignored_fields: continue + if (entry.sdk_model, req_field) in EXCLUSIONS: + continue sdk_name = reverse_map.get(req_field) if sdk_name and sdk_name in model_class.model_fields: field_info = model_class.model_fields[sdk_name] @@ -1338,40 +1349,89 @@ def test_exclusion_map_is_current() -> None: intentional — prevent that here. """ spec = _load_spec() + # Lazy-loaded on first WS-keyed exclusion (avoids parsing the AsyncAPI + # YAML when no WS entry needs it; reused across all WS entries when one + # does — currently exercised live by ``TickerPayload.time``). + ws_spec: dict[str, Any] | None = None stale: list[str] = [] for (fqn, name), excl in EXCLUSIONS.items(): - # Case 1: spec-side exclusion keyed on a model FQN (kalshi.models.*) - if fqn.startswith("kalshi.models."): + # Case 1: model-keyed exclusion. The FQN may belong to a request body + # (``BODY_MODEL_MAP``), a REST response model (``CONTRACT_MAP``), a WS + # payload (``WS_CONTRACT_MAP``), or several of these at once — e.g. + # ``CreateOrderRequest`` is both a request body and a response schema. + # ``name`` must appear in at least one of the spec sources, and the + # SDK model must NOT emit it as a wire alias. + if fqn.startswith("kalshi.models.") or fqn.startswith("kalshi.ws.models."): + spec_sources: list[tuple[str, dict[str, Any]]] = [] + + # Source 1: request body via BODY_MODEL_MAP + METHOD_ENDPOINT_MAP spec_ref = next( (ref for ref, m in BODY_MODEL_MAP.items() if m == fqn), None, ) - if spec_ref is None: - stale.append( - f"EXCLUSIONS[{(fqn, name)}] references unknown model " - f"{fqn} (not in BODY_MODEL_MAP); reason={excl.reason!r}" - ) - continue - schema = None - for e in METHOD_ENDPOINT_MAP: - if e.request_body_schema == spec_ref: - schema = _resolve_request_body_schema( - spec, e.path_template, e.http_method, + if spec_ref is not None: + body_schema = None + for e in METHOD_ENDPOINT_MAP: + if e.request_body_schema == spec_ref: + body_schema = _resolve_request_body_schema( + spec, e.path_template, e.http_method, + ) + break + if body_schema is None: + # METHOD_ENDPOINT_MAP / BODY_MODEL_MAP inconsistency is a + # real bug; surface it, but still try CONTRACT_MAP and + # WS_CONTRACT_MAP so a shared model isn't falsely flagged + # as unknown when one of its other sources would resolve. + stale.append( + f"EXCLUSIONS[{(fqn, name)}] references schema {spec_ref} " + f"not reachable via METHOD_ENDPOINT_MAP" + ) + else: + spec_sources.append( + (f"request body {spec_ref}", body_schema.get("properties", {})) + ) + + # Source 2: REST response model via CONTRACT_MAP + for c_entry in CONTRACT_MAP: + if c_entry.sdk_model == fqn: + spec_sources.append( + ( + f"response schema {c_entry.spec_schema}", + _get_schema_fields(spec, c_entry.spec_schema), + ) ) break - if schema is None: + + # Source 3: WS payload model via WS_CONTRACT_MAP + for w_entry in WS_CONTRACT_MAP: + if w_entry.sdk_model == fqn: + if ws_spec is None: + ws_spec = _load_asyncapi_spec() + spec_sources.append( + ( + f"WS schema {w_entry.spec_schema}", + _get_ws_msg_fields(ws_spec, w_entry.spec_schema), + ) + ) + break + + if not spec_sources: stale.append( - f"EXCLUSIONS[{(fqn, name)}] references schema {spec_ref} " - f"not reachable via METHOD_ENDPOINT_MAP" + f"EXCLUSIONS[{(fqn, name)}] references unknown model {fqn} " + f"(not in BODY_MODEL_MAP, CONTRACT_MAP, or WS_CONTRACT_MAP); " + f"reason={excl.reason!r}" ) continue - if name not in schema.get("properties", {}): + + if not any(name in props for _label, props in spec_sources): + source_list = ", ".join(label for label, _ in spec_sources) stale.append( f"EXCLUSIONS[{(fqn, name)}] claims spec has {name!r} on " - f"{spec_ref}, but spec does NOT — entry is stale. " - f"reason={excl.reason!r}" + f"{fqn}, but it is absent from every spec source " + f"({source_list}) — entry is stale. reason={excl.reason!r}" ) + model_cls = _get_model_class_from_fqn(fqn) if name in _model_aliases(model_cls): stale.append( @@ -1410,7 +1470,8 @@ def test_exclusion_map_is_current() -> None: else: stale.append( f"EXCLUSIONS[{(fqn, name)}] has unexpected FQN prefix; " - f"expected kalshi.models.* or kalshi.resources.*" + f"expected kalshi.models.*, kalshi.ws.models.*, or " + f"kalshi.resources.*" ) if stale: