From 1cad12bc8791ca0a4b4c31a76c38e54a87500261 Mon Sep 17 00:00:00 2001 From: Petr Date: Sun, 27 Sep 2026 07:13:59 +0200 Subject: [PATCH] test: add ratcheting API call-count tests for hot commands (#802) Pin the exact (METHOD, path) HTTP calls of the hot read commands (project list/status, config list/detail, job list single + fan-out, job detail, storage tables/table-detail, flow list/detail) end-to-end through CliRunner against pytest-httpx, with 1-vs-10-item cases proving list call counts do not grow per item. Shared recording/assert helpers live in tests/helpers.py; CONTRIBUTING.md asks hot-path command changes to add or update a case. --- CONTRIBUTING.md | 2 + tests/helpers.py | 75 +++++++ tests/test_api_call_counts.py | 375 ++++++++++++++++++++++++++++++++++ 3 files changed, 452 insertions(+) create mode 100644 tests/test_api_call_counts.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 909cec8ac..9faffab7b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -402,6 +402,8 @@ before the PR is mergeable. > **Running locally without exporting a token:** if the target project is already registered in a kbagent `config.json`, use config-dir mode -- `make test-e2e-local CONFIG_DIR=/path/to/.kbagent ALIAS=my-proj`. The harness reads the token from `config.json` at import time and promotes it into `E2E_API_TOKEN` / `E2E_URL`; an explicit `E2E_API_TOKEN` still wins. +- [ ] **API call-count test for hot read paths** -- a new or changed list/detail command that users and agents run often (the `project`/`config`/`job`/`storage`/`flow` read commands and their peers) adds or updates a case in `tests/test_api_call_counts.py`. It pins the exact `(METHOD, path)` calls via `helpers.assert_api_calls`; list commands also get a 1-vs-10-items case proving the count does not grow per item. The expected lists are a ratchet: raising one is a deliberate, reviewed change -- say why in the PR + - [ ] **Run `make check`** before committing (lint + format + full test suite) - [ ] **Run `make typecheck`** -- `ty` must pass clean (0 diagnostics; the backlog was cleared in 0.45.0, so the gate is blocking, not warning-only) - [ ] **No new `tuple[...]` returns** -- multi-value returns use a `@dataclass` ([Code Quality Patterns](#code-quality-patterns)) diff --git a/tests/helpers.py b/tests/helpers.py index cb1f22c97..34e481165 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -4,9 +4,13 @@ ConfigStore instances. Used across multiple test files to avoid duplication. """ +from collections.abc import Mapping, Sequence from pathlib import Path +from typing import Any from unittest.mock import MagicMock +import httpx + from keboola_agent_cli.config_store import ConfigStore from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.models import ProjectConfig, TokenVerifyResponse @@ -126,3 +130,74 @@ def setup_two_projects(tmp_config_dir: Path) -> ConfigStore: ), ) return store + + +# --------------------------------------------------------------------------- +# API call-count recording (issue #802) +# --------------------------------------------------------------------------- + +# One recorded HTTP call: (METHOD, path) -- host stripped, query optional. +ApiCall = tuple[str, str] + +# Status returned for a request no route matches. Deliberately NOT a +# retryable status (429/5xx): an unmatched call must fail fast, never sleep +# through the client's backoff. +UNMATCHED_ROUTE_STATUS = 418 + + +def _call_of(request: httpx.Request, *, include_query: bool) -> ApiCall: + path = request.url.path + if include_query and request.url.query: + path = f"{path}?{request.url.query.decode()}" + return (request.method, path) + + +def mock_api_routes(httpx_mock: Any, routes: Mapping[ApiCall, Any]) -> None: + """Answer every HTTP request from a ``{(METHOD, path): json_body}`` table. + + Routing ignores host and query string, so one table serves the Storage + and Queue hosts alike. The callback is reusable and optional: the same + route may be hit any number of times (that count is what the call-count + tests assert), and a command that makes zero calls does not trip + pytest-httpx's "response never requested" teardown check. A request no + route matches gets HTTP ``UNMATCHED_ROUTE_STATUS`` so it surfaces as a + command error and as an unexpected entry in the recorded calls. + """ + + def _respond(request: httpx.Request) -> httpx.Response: + key = _call_of(request, include_query=False) + if key not in routes: + return httpx.Response( + UNMATCHED_ROUTE_STATUS, + json={"error": f"no mocked route for {key[0]} {key[1]}"}, + ) + return httpx.Response(200, json=routes[key]) + + httpx_mock.add_callback(_respond, is_reusable=True, is_optional=True) + + +def recorded_api_calls(httpx_mock: Any, *, include_query: bool = False) -> list[ApiCall]: + """Every HTTP request the mock saw, as ``(METHOD, path)`` in send order.""" + return [_call_of(r, include_query=include_query) for r in httpx_mock.get_requests()] + + +def assert_api_calls( + httpx_mock: Any, + expected: Sequence[ApiCall], + *, + include_query: bool = False, + ordered: bool = True, +) -> None: + """Assert the exact number AND sequence of HTTP calls a command made. + + ``ordered=False`` compares sorted multisets -- use it where calls are + issued from a thread pool (multi-project fan-out) and the send order is + not deterministic. The count is exact either way: a duplicate call fails. + """ + actual = recorded_api_calls(httpx_mock, include_query=include_query) + if not ordered: + actual, expected = sorted(actual), sorted(expected) + assert actual == list(expected), ( + f"API calls changed ({len(actual)} made, {len(expected)} expected).\n" + f"actual: {actual}\nexpected: {list(expected)}" + ) diff --git a/tests/test_api_call_counts.py b/tests/test_api_call_counts.py new file mode 100644 index 000000000..f0cd91204 --- /dev/null +++ b/tests/test_api_call_counts.py @@ -0,0 +1,375 @@ +"""Ratcheting API call-count tests for hot read commands (issue #802). + +Each test runs a real command end-to-end -- Typer ``CliRunner`` -> command -> +service -> real HTTP client -- against pytest-httpx, then asserts the EXACT +number and sequence of ``(METHOD, path)`` calls it made. The point is to catch +N+1 regressions (a per-item lookup sneaking into a list loop) and duplicate +calls before they reach users as slow commands and rate-limit pressure. + +THE EXPECTED CALL LISTS ARE A RATCHET. They are committed literals, not +computed from the fixture data. A change that makes a command issue MORE +calls must edit the literal here, which puts the increase in front of a +reviewer on purpose -- justify it in the PR. A change that makes a command +issue FEWER calls should lower the literal in the same PR so the win cannot +silently regress. + +Size-parametrized tests (1 vs 10 items) pin the design property that a list +command's call count does NOT grow with the number of items it returns. + +Routing ignores host and query (see ``helpers.mock_api_routes``); where the +query string is part of the contract (job list sorting/paging) the assertion +uses ``include_query=True``. Multi-project fan-out runs on a thread pool, so +those assertions compare sorted multisets (``ordered=False``). +""" + +import json +from pathlib import Path +from typing import Any + +import pytest +from typer.testing import CliRunner, Result + +from helpers import ( + assert_api_calls, + mock_api_routes, + setup_single_project, + setup_two_projects, +) +from keboola_agent_cli.cli import app + +runner = CliRunner() + +DEFAULT_BRANCH_ID = 100 +SIZES = [1, 10] + +# Shared route bodies --------------------------------------------------------- + +TOKEN_VERIFY = { + "id": "12345", + "description": "My Token", + "owner": {"id": 258, "name": "Production", "defaultBackend": "snowflake"}, +} +DEV_BRANCHES = [{"id": DEFAULT_BRANCH_ID, "name": "Main", "isDefault": True}] + + +def _invoke(config_dir: Path, *args: str) -> Result: + result = runner.invoke(app, ["--config-dir", str(config_dir), "--json", *args]) + assert result.exit_code == 0, result.output + return result + + +def _data(result: Result) -> Any: + return json.loads(result.output)["data"] + + +def _configs(n: int) -> list[dict[str, Any]]: + return [ + { + "id": str(i), + "name": f"Config {i}", + "description": "", + "configuration": {"parameters": {}}, + "rows": [], + "currentVersion": { + "created": "2026-09-01T00:00:00+0000", + "creatorToken": {"description": "me"}, + "changeDescription": "", + }, + } + for i in range(n) + ] + + +# project --------------------------------------------------------------------- + + +class TestProjectCallCounts: + def test_project_list_is_offline(self, tmp_config_dir: Path, httpx_mock) -> None: + """`project list` reads config.json only -- zero API calls.""" + setup_single_project(tmp_config_dir) + mock_api_routes(httpx_mock, {}) + result = _invoke(tmp_config_dir, "project", "list") + assert [p["alias"] for p in _data(result)] == ["prod"] + assert_api_calls(httpx_mock, []) + + def test_project_status_single(self, tmp_config_dir: Path, httpx_mock) -> None: + setup_single_project(tmp_config_dir) + mock_api_routes(httpx_mock, {("GET", "/v2/storage/tokens/verify"): TOKEN_VERIFY}) + _invoke(tmp_config_dir, "project", "status", "--project", "prod") + assert_api_calls(httpx_mock, [("GET", "/v2/storage/tokens/verify")]) + + def test_project_status_fan_out(self, tmp_config_dir: Path, httpx_mock) -> None: + """One token verify per project, nothing else.""" + setup_two_projects(tmp_config_dir) + mock_api_routes(httpx_mock, {("GET", "/v2/storage/tokens/verify"): TOKEN_VERIFY}) + result = _invoke(tmp_config_dir, "project", "status") + assert len(_data(result)) == 2 + assert_api_calls( + httpx_mock, + [("GET", "/v2/storage/tokens/verify")] * 2, + ordered=False, + ) + + +# config ---------------------------------------------------------------------- + + +class TestConfigCallCounts: + @pytest.mark.parametrize("n", SIZES) + def test_config_list_constant_in_config_count( + self, tmp_config_dir: Path, httpx_mock, n: int + ) -> None: + """Components (with configs inlined) + default-branch lookup + folder metadata. + + The count must not grow with the number of configurations. + """ + setup_single_project(tmp_config_dir) + mock_api_routes( + httpx_mock, + { + ("GET", "/v2/storage/components"): [ + { + "id": "keboola.ex-db", + "name": "DB", + "type": "extractor", + "configurations": _configs(n), + } + ], + ("GET", "/v2/storage/dev-branches"): DEV_BRANCHES, + ( + "GET", + f"/v2/storage/branch/{DEFAULT_BRANCH_ID}/search/component-configurations", + ): [], + }, + ) + result = _invoke(tmp_config_dir, "config", "list", "--project", "prod") + assert len(_data(result)["configs"]) == n + assert_api_calls( + httpx_mock, + [ + ("GET", "/v2/storage/components"), + ("GET", "/v2/storage/dev-branches"), + ( + "GET", + f"/v2/storage/branch/{DEFAULT_BRANCH_ID}/search/component-configurations", + ), + ], + ) + + @pytest.mark.parametrize("n_rows", SIZES) + def test_config_detail_single_call(self, tmp_config_dir: Path, httpx_mock, n_rows: int) -> None: + """Rows come inline with the config -- no per-row fetch.""" + setup_single_project(tmp_config_dir) + body = _configs(1)[0] + body["rows"] = [ + {"id": f"r{i}", "name": f"Row {i}", "configuration": {}} for i in range(n_rows) + ] + mock_api_routes( + httpx_mock, + {("GET", "/v2/storage/components/keboola.ex-db/configs/0"): body}, + ) + result = _invoke( + tmp_config_dir, + "config", + "detail", + "--project", + "prod", + "--component-id", + "keboola.ex-db", + "--config-id", + "0", + ) + assert len(_data(result)["rows"]) == n_rows + assert_api_calls( + httpx_mock, + [("GET", "/v2/storage/components/keboola.ex-db/configs/0")], + ) + + +# job ------------------------------------------------------------------------- + + +def _jobs(n: int) -> list[dict[str, Any]]: + return [ + { + "id": str(1000 + i), + "status": "error" if i % 2 else "success", + "component": "keboola.ex-db", + "config": "0", + "startTime": f"2026-09-01T00:{i:02d}:00+00:00", + } + for i in range(n) + ] + + +JOB_SEARCH_CALL = ( + "GET", + "/search/jobs?limit=50&offset=0&sortBy=startTime&sortOrder=desc", +) + + +class TestJobCallCounts: + @pytest.mark.parametrize("n", SIZES) + def test_job_list_single_project(self, tmp_config_dir: Path, httpx_mock, n: int) -> None: + """One search call regardless of job count (failed jobs included).""" + setup_single_project(tmp_config_dir) + mock_api_routes(httpx_mock, {("GET", "/search/jobs"): _jobs(n)}) + result = _invoke(tmp_config_dir, "job", "list", "--project", "prod") + assert len(_data(result)["jobs"]) == n + assert_api_calls(httpx_mock, [JOB_SEARCH_CALL], include_query=True) + + def test_job_list_multi_project_fan_out(self, tmp_config_dir: Path, httpx_mock) -> None: + """Exactly one search per registered project; merged client-side.""" + setup_two_projects(tmp_config_dir) + mock_api_routes(httpx_mock, {("GET", "/search/jobs"): _jobs(3)}) + result = _invoke(tmp_config_dir, "job", "list") + assert len(_data(result)["jobs"]) == 6 + assert_api_calls( + httpx_mock, + [JOB_SEARCH_CALL] * 2, + include_query=True, + ordered=False, + ) + + def test_job_detail_single_call(self, tmp_config_dir: Path, httpx_mock) -> None: + """No log tail by default -> just the job itself (flow job: hint is static).""" + setup_single_project(tmp_config_dir) + mock_api_routes( + httpx_mock, + { + ("GET", "/jobs/1001"): { + "id": "1001", + "status": "error", + "component": "keboola.flow", + "config": "9", + } + }, + ) + result = _invoke(tmp_config_dir, "job", "detail", "--project", "prod", "--job-id", "1001") + assert "trigger_hint" in _data(result) + assert_api_calls(httpx_mock, [("GET", "/jobs/1001")]) + + +# storage --------------------------------------------------------------------- + + +def _tables(n: int) -> list[dict[str, Any]]: + return [ + { + "id": f"in.c-main.t{i}", + "name": f"t{i}", + "displayName": f"t{i}", + "bucket": {"id": "in.c-main", "backendPath": ["KBC_DB", "in.c-main"]}, + "columns": ["id", "value"], + "primaryKey": ["id"], + "rowsCount": i, + "dataSizeBytes": 1024 * i, + } + for i in range(n) + ] + + +class TestStorageCallCounts: + @pytest.mark.parametrize("n", SIZES) + def test_storage_tables_constant_in_table_count( + self, tmp_config_dir: Path, httpx_mock, n: int + ) -> None: + """One list call regardless of table count -- no per-table detail fetch.""" + setup_single_project(tmp_config_dir) + mock_api_routes(httpx_mock, {("GET", "/v2/storage/tables"): _tables(n)}) + result = _invoke(tmp_config_dir, "storage", "tables", "--project", "prod") + assert len(_data(result)["tables"]) == n + assert_api_calls(httpx_mock, [("GET", "/v2/storage/tables")]) + + def test_storage_table_detail_single_call(self, tmp_config_dir: Path, httpx_mock) -> None: + """Bucket backendPath comes inline with the table -- no bucket fetch.""" + setup_single_project(tmp_config_dir) + mock_api_routes( + httpx_mock, + {("GET", "/v2/storage/tables/in.c-main.t0"): _tables(1)[0]}, + ) + result = _invoke( + tmp_config_dir, + "storage", + "table-detail", + "--project", + "prod", + "--table-id", + "in.c-main.t0", + ) + assert _data(result)["table_id"] == "in.c-main.t0" + assert_api_calls(httpx_mock, [("GET", "/v2/storage/tables/in.c-main.t0")]) + + +# flow ------------------------------------------------------------------------ + + +def _flow(flow_id: str, n_tasks: int) -> dict[str, Any]: + return { + "id": flow_id, + "name": f"Flow {flow_id}", + "description": "", + "isDisabled": False, + "configuration": { + "phases": [{"id": "1", "name": "Extract", "next": []}], + "tasks": [ + { + "id": str(t), + "name": f"Task {t}", + "phase": "1", + "enabled": True, + "task": { + "type": "job", + "componentId": "keboola.ex-db", + "configId": str(t), + "mode": "run", + }, + } + for t in range(n_tasks) + ], + }, + } + + +class TestFlowCallCounts: + @pytest.mark.parametrize("n", SIZES) + def test_flow_list_constant_in_flow_count( + self, tmp_config_dir: Path, httpx_mock, n: int + ) -> None: + """keboola.flow configs + the legacy keboola.orchestrator count probe.""" + setup_single_project(tmp_config_dir) + mock_api_routes( + httpx_mock, + { + ("GET", "/v2/storage/components/keboola.flow/configs"): [ + _flow(str(i), 3) for i in range(n) + ], + ("GET", "/v2/storage/components/keboola.orchestrator/configs"): [], + }, + ) + result = _invoke(tmp_config_dir, "flow", "list", "--project", "prod") + assert len(_data(result)["flows"]) == n + assert_api_calls( + httpx_mock, + [ + ("GET", "/v2/storage/components/keboola.flow/configs"), + ("GET", "/v2/storage/components/keboola.orchestrator/configs"), + ], + ) + + @pytest.mark.parametrize("n_tasks", SIZES) + def test_flow_detail_constant_in_task_count( + self, tmp_config_dir: Path, httpx_mock, n_tasks: int + ) -> None: + """No per-task component/config lookup.""" + setup_single_project(tmp_config_dir) + mock_api_routes( + httpx_mock, + {("GET", "/v2/storage/components/keboola.flow/configs/9"): _flow("9", n_tasks)}, + ) + result = _invoke(tmp_config_dir, "flow", "detail", "--project", "prod", "--flow-id", "9") + assert _data(result)["task_count"] == n_tasks + assert_api_calls( + httpx_mock, + [("GET", "/v2/storage/components/keboola.flow/configs/9")], + )