From 24b4275d6eb6680238d1bdea1e9f23bf57a28ee8 Mon Sep 17 00:00:00 2001 From: Stephen Zhou Date: Tue, 3 Nov 2026 04:21:34 +0000 Subject: [PATCH] perf(api): speed up API tests by lazy IM imports and smaller contract docs (#45381) --- .../im_integration/adapters/factory.py | 24 +++++++--- .../unit/controllers/error_contract.py | 47 +++++++++++++++---- .../controllers/common/test_schema.py | 24 ++++++++++ 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/api/core/human_input_v2/im_integration/adapters/factory.py b/api/core/human_input_v2/im_integration/adapters/factory.py index b4336f55ca8..2a3ff86746d 100644 --- a/api/core/human_input_v2/im_integration/adapters/factory.py +++ b/api/core/human_input_v2/im_integration/adapters/factory.py @@ -1,4 +1,9 @@ -"""Construction of complete IM provider adapters from canonical credentials.""" +"""Construction of complete IM provider adapters from canonical credentials. + +Provider adapters are imported inside each branch because their vendor SDKs +(notably ``lark_oapi``) take seconds to import, and this module is reachable +from ``create_app()``. +""" from .credentials import ( DingTalkCredentials, @@ -9,27 +14,34 @@ from .credentials import ( SlackCredentials, WeComCredentials, ) -from .dingtalk import DingTalkIMProviderAdapter -from .feishu_lark import FeishuIMProviderAdapter, LarkIMProviderAdapter -from .ms_teams import MSTeamsIMProviderAdapter from .protocols import IMProviderAdapter -from .slack import SlackIMProviderAdapter -from .wecom import WeComIMProviderAdapter def build_im_provider_adapter(credentials: IMProviderCredentials) -> IMProviderAdapter: match credentials: case SlackCredentials(): + from .slack import SlackIMProviderAdapter + return SlackIMProviderAdapter(credentials) case FeishuCredentials(): + from .feishu_lark import FeishuIMProviderAdapter + return FeishuIMProviderAdapter(credentials) case LarkCredentials(): + from .feishu_lark import LarkIMProviderAdapter + return LarkIMProviderAdapter(credentials) case DingTalkCredentials(): + from .dingtalk import DingTalkIMProviderAdapter + return DingTalkIMProviderAdapter(credentials) case MSTeamsCredentials(): + from .ms_teams import MSTeamsIMProviderAdapter + return MSTeamsIMProviderAdapter(credentials) case WeComCredentials(): + from .wecom import WeComIMProviderAdapter + return WeComIMProviderAdapter(credentials) case _: raise TypeError("unsupported IM provider credentials") diff --git a/api/tests/helpers/unit/controllers/error_contract.py b/api/tests/helpers/unit/controllers/error_contract.py index a8fa5320840..cf263feefdb 100644 --- a/api/tests/helpers/unit/controllers/error_contract.py +++ b/api/tests/helpers/unit/controllers/error_contract.py @@ -13,7 +13,7 @@ any code. On APIs that do not document business errors import sys from collections.abc import Iterator, Mapping -from typing import cast +from typing import cast, override from weakref import WeakKeyDictionary, WeakSet from flask import Flask, Response, current_app, request @@ -74,15 +74,46 @@ def _documents_business_errors(api: ExternalApi) -> bool: return any(api is surface for surface in _module_apis(_DOCUMENTED_APIS)) +# Flask-RESTX types ``Api.models`` as ``dict``, so the fallback mapping must be one. +class _BorrowedModels(dict[str, object]): # noqa: FURB189 + """An API's own models whose lookups by name fall back to the real surfaces' models. + + Swagger resolves models only through ``in`` and ``[]``, while document finalization + iterates every registered model. Iterating only the API's own models keeps a test API's + document to the borrowed models its operations reference, instead of every surface model. + """ + + def __init__(self, own: Mapping[str, object], fallbacks: tuple[Mapping[str, object], ...]) -> None: + super().__init__(own) + self._fallbacks = fallbacks + + @override + def __contains__(self, key: object, /) -> bool: + return super().__contains__(key) or any(key in fallback for fallback in self._fallbacks) + + def __missing__(self, key: str) -> object: + found = [fallback[key] for fallback in self._fallbacks if key in fallback] + if not found: + raise KeyError(key) + return found[0] + + def _document(api: ExternalApi) -> dict[str, object]: + """The OpenAPI document of ``api``, built once per API and cached. + + Test apps often mount a few resources on a fresh ExternalApi without the namespace that + registers their models; those models are borrowed from the real surfaces for the build, + without being copied into ``api.models``. ``api.models`` is the same mapping afterwards, + whether the build succeeds or raises. + """ if api not in _documents: - # Test apps often mount a few resources on a fresh ExternalApi without the namespace - # that registers their models; borrow those from the real surfaces so the document builds. - for surface in _module_apis(): - if surface is not api: - for name, model in surface.models.items(): - api.models.setdefault(name, model) - _documents[api] = cast(dict[str, object], Swagger(api).as_dict()) + surfaces = tuple(surface.models for surface in _module_apis() if surface is not api) + own_models = api.models + api.models = _BorrowedModels(own_models, surfaces) + try: + _documents[api] = cast(dict[str, object], Swagger(api).as_dict()) + finally: + api.models = own_models return _documents[api] diff --git a/api/tests/unit_tests/controllers/common/test_schema.py b/api/tests/unit_tests/controllers/common/test_schema.py index a626cc61641..aec7dafa16a 100644 --- a/api/tests/unit_tests/controllers/common/test_schema.py +++ b/api/tests/unit_tests/controllers/common/test_schema.py @@ -933,6 +933,30 @@ class TestErrorResponseContractCheckCases: assert "does not match ErrorResponse" in found assert f"{field}:" in found + @staticmethod + def test_documents_only_the_surface_models_a_test_api_references(monkeypatch: pytest.MonkeyPatch) -> None: + # Copying every surface model into each test API made every document carry thousands of schemas. + surface = ExternalApi(Flask("surface")) + surface.model("BorrowedItem", {"value": fields.String}) + surface.model("UnrelatedItem", {"value": fields.String}) + monkeypatch.setattr(error_contract, "_module_apis", lambda *_names: iter([surface])) + app = Flask(__name__) + api = ExternalApi(app) + + @api.route("/items") + class Items(Resource): + @api.response(200, "OK", "BorrowedItem") + def get(self) -> dict[str, str]: + return {} + + own_models = dict(api.models) + with app.test_request_context("/items"): + schemas = error_contract._document(api)["components"]["schemas"] # type: ignore[index] + + assert "BorrowedItem" in schemas + assert "UnrelatedItem" not in schemas + assert dict(api.models) == own_models + @staticmethod @pytest.mark.allow_error_contract_violations def test_checks_responses_served_by_an_api_mounted_on_the_app() -> None: