From 18f2595a0b22269844cb6d58955bb6f18faf380c Mon Sep 17 00:00:00 2001 From: soustruh Date: Wed, 30 Sep 2026 14:06:38 +0200 Subject: [PATCH] test: close gaps found in the post-merge reviews of #797, #803 and #804 --- CONTRIBUTING.md | 2 +- src/keboola_agent_cli/data_science_client.py | 5 +- tests/helpers.py | 56 ++++- tests/test_api_call_counts.py | 238 ++++++++++++++++--- tests/test_data_app_service.py | 61 ++++- tests/test_integration.py | 46 +++- tests/test_permissions_cli.py | 1 + 7 files changed, 357 insertions(+), 52 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 677a1d68..ea416d62 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -404,7 +404,7 @@ 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 +- [ ] **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 calls via `helpers.assert_api_calls`, which compares the method and path and, where the test pins them, the parsed query (`include_query=True`) and the token (`include_token=True`, for multi-project cases). List commands also run with 1 and 10 items and expect the same calls at both sizes: this catches a call made once per item, but not a call made once per batch or page of more than 10 items. Fixtures must look like real API payloads (several component types, a transformation with rows and storage mappings, Queue jobs with `runId`, alias and shared tables), because a per-item call that depends on a field the fixture lacks is not caught. 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) diff --git a/src/keboola_agent_cli/data_science_client.py b/src/keboola_agent_cli/data_science_client.py index c4f22263..f214226d 100644 --- a/src/keboola_agent_cli/data_science_client.py +++ b/src/keboola_agent_cli/data_science_client.py @@ -82,8 +82,9 @@ def __exit__(self, *args: object) -> None: def list_apps(self) -> list[dict[str, Any]]: """Return the thin index of ALL deployments in the project (no body filter). - The Data Science API scopes responses by the token's project; there - is no ``branchId`` query parameter on the list endpoint. + The Data Science API scopes responses by the token's project. The list + endpoint also accepts ``componentId``, ``type`` and ``branchId`` + filters; this method sends none of them. ``GET /apps`` is paginated: without ``limit``/``offset`` it returns only a default first page (100 items) that mixes workspace diff --git a/tests/helpers.py b/tests/helpers.py index 34e48116..b8bf1070 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -8,6 +8,7 @@ from pathlib import Path from typing import Any from unittest.mock import MagicMock +from urllib.parse import parse_qsl, urlencode import httpx @@ -136,8 +137,9 @@ def setup_two_projects(tmp_config_dir: Path) -> ConfigStore: # API call-count recording (issue #802) # --------------------------------------------------------------------------- -# One recorded HTTP call: (METHOD, path) -- host stripped, query optional. -ApiCall = tuple[str, str] +# One recorded HTTP call: (METHOD, path) -- host stripped, query optional -- +# plus the X-StorageApi-Token value when the caller asks for it. +ApiCall = tuple[str, ...] # Status returned for a request no route matches. Deliberately NOT a # retryable status (429/5xx): an unmatched call must fail fast, never sleep @@ -145,10 +147,27 @@ def setup_two_projects(tmp_config_dir: Path) -> ConfigStore: UNMATCHED_ROUTE_STATUS = 418 -def _call_of(request: httpx.Request, *, include_query: bool) -> ApiCall: +def _sorted_query(target: str) -> str: + """Rebuild ``path?query`` with the query parsed and its pairs sorted. + + Parameter order and percent-encoding then do not matter: two targets are + equal only when they carry the same parameters with the same values. + """ + path, _, query = target.partition("?") + if not query: + return path + pairs = sorted(parse_qsl(query, keep_blank_values=True)) + return f"{path}?{urlencode(pairs, safe='[],')}" + + +def _call_of( + request: httpx.Request, *, include_query: bool, include_token: bool = False +) -> ApiCall: path = request.url.path if include_query and request.url.query: - path = f"{path}?{request.url.query.decode()}" + path = _sorted_query(f"{path}?{request.url.query.decode()}") + if include_token: + return (request.method, path, request.headers.get("X-StorageApi-Token", "")) return (request.method, path) @@ -176,9 +195,14 @@ def _respond(request: httpx.Request) -> httpx.Response: 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 recorded_api_calls( + httpx_mock: Any, *, include_query: bool = False, include_token: bool = False +) -> list[ApiCall]: + """Every HTTP request the mock saw, as ``(METHOD, path[, token])`` in send order.""" + return [ + _call_of(r, include_query=include_query, include_token=include_token) + for r in httpx_mock.get_requests() + ] def assert_api_calls( @@ -186,15 +210,25 @@ def assert_api_calls( expected: Sequence[ApiCall], *, include_query: bool = False, + include_token: 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. + ``include_query=True`` compares the query too, parsed and sorted on both + sides, so the expected literal may list parameters in any order. + ``include_token=True`` adds each call's ``X-StorageApi-Token`` as a third + element -- use it in multi-project tests, where the path alone cannot show + which project a call was for. ``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) + actual = recorded_api_calls( + httpx_mock, include_query=include_query, include_token=include_token + ) + if include_query: + expected = [(call[0], _sorted_query(call[1]), *call[2:]) for call in expected] if not ordered: actual, expected = sorted(actual), sorted(expected) assert actual == list(expected), ( diff --git a/tests/test_api_call_counts.py b/tests/test_api_call_counts.py index f0cd9120..ab9e51ca 100644 --- a/tests/test_api_call_counts.py +++ b/tests/test_api_call_counts.py @@ -13,13 +13,24 @@ 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. +Size-parametrized tests (1 vs 10 items) expect the same calls at both sizes. +That catches a call made once per item, but not one made once per batch or +page of more than 10 items. The fixtures look like real API payloads (several component types, a +transformation with rows and storage mappings, Queue jobs with ``runId``, +alias and shared tables), so a per-item call gated on one of those fields +is caught too. 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``). +query string is part of the contract (config list and storage tables +includes, job list sorting/paging) the assertion uses ``include_query=True``, +which compares the parsed, sorted query. Multi-project fan-out runs on a +thread pool, so those assertions compare sorted multisets (``ordered=False``) +and record each call's token (``include_token=True``) to show that every +project got its own call. + +The tests invoke ``app``, not the ``run()`` console-script wrapper, so the +best-effort telemetry ``POST /v2/storage/events`` that ``run()`` sends after a +command is not counted here. """ import json @@ -36,11 +47,27 @@ setup_two_projects, ) from keboola_agent_cli.cli import app +from keboola_agent_cli.constants import ENV_AUTO_UPDATE runner = CliRunner() DEFAULT_BRANCH_ID = 100 SIZES = [1, 10] +# The tokens `helpers.setup_two_projects` gives the `prod` and `dev` projects. +PROD_TOKEN = "901-xxx" +DEV_TOKEN = "7012-yyy" + + +@pytest.fixture(autouse=True) +def _no_auto_update(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the startup auto-update check out of the recorded calls. + + Without it the exact counts hold only because an editable install skips the + check (``_is_dev_install``); an installed wheel would add a GitHub + ``releases/latest`` call to the first invocation. + """ + monkeypatch.setenv(ENV_AUTO_UPDATE, "false") + # Shared route bodies --------------------------------------------------------- @@ -55,6 +82,11 @@ 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 + # A list command exits 0 even when a project call fails -- the failure goes + # into `errors` -- so the exit code alone proves little. + data = _data(result) + if isinstance(data, dict) and "errors" in data: + assert data["errors"] == [], data["errors"] return result @@ -68,11 +100,18 @@ def _configs(n: int) -> list[dict[str, Any]]: "id": str(i), "name": f"Config {i}", "description": "", + "created": "2026-09-01T00:00:00+0000", + "creatorToken": {"id": 7, "description": "me"}, + "version": 3, + "changeDescription": "", + "isDeleted": False, + "isDisabled": False, "configuration": {"parameters": {}}, "rows": [], + "state": {}, "currentVersion": { "created": "2026-09-01T00:00:00+0000", - "creatorToken": {"description": "me"}, + "creatorToken": {"id": 7, "description": "me"}, "changeDescription": "", }, } @@ -80,6 +119,64 @@ def _configs(n: int) -> list[dict[str, Any]]: ] +def _transformation_configs(n: int) -> list[dict[str, Any]]: + """SQL transformation configs: a code block, storage input/output mappings and a row.""" + return [ + { + **cfg, + "configuration": { + "parameters": { + "blocks": [ + { + "name": "Block 1", + "codes": [ + { + "name": "Code", + "script": [f'CREATE TABLE "out{i}" AS SELECT * FROM "t{i}";'], + } + ], + } + ] + }, + "storage": { + "input": {"tables": [{"source": f"in.c-main.t{i}", "destination": f"t{i}"}]}, + "output": { + "tables": [{"source": f"out{i}", "destination": f"out.c-main.out{i}"}] + }, + }, + }, + "rows": [ + { + "id": f"{i}01", + "name": "Row 1", + "isDisabled": False, + "configuration": {"parameters": {}}, + } + ], + } + for i, cfg in enumerate(_configs(n)) + ] + + +def _components(n: int) -> list[dict[str, Any]]: + """An extractor, a SQL transformation and a writer, each with ``n`` configs.""" + return [ + {"id": "keboola.ex-db", "name": "DB", "type": "extractor", "configurations": _configs(n)}, + { + "id": "keboola.snowflake-transformation", + "name": "Snowflake SQL", + "type": "transformation", + "configurations": _transformation_configs(n), + }, + { + "id": "keboola.wr-db-snowflake", + "name": "Snowflake", + "type": "writer", + "configurations": _configs(n), + }, + ] + + # project --------------------------------------------------------------------- @@ -95,7 +192,8 @@ def test_project_list_is_offline(self, tmp_config_dir: Path, httpx_mock) -> None 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") + result = _invoke(tmp_config_dir, "project", "status", "--project", "prod") + assert [(p["alias"], p["status"]) for p in _data(result)] == [("prod", "ok")] assert_api_calls(httpx_mock, [("GET", "/v2/storage/tokens/verify")]) def test_project_status_fan_out(self, tmp_config_dir: Path, httpx_mock) -> None: @@ -103,10 +201,17 @@ def test_project_status_fan_out(self, tmp_config_dir: Path, httpx_mock) -> None: 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 sorted((p["alias"], p["status"]) for p in _data(result)) == [ + ("dev", "ok"), + ("prod", "ok"), + ] assert_api_calls( httpx_mock, - [("GET", "/v2/storage/tokens/verify")] * 2, + [ + ("GET", "/v2/storage/tokens/verify", PROD_TOKEN), + ("GET", "/v2/storage/tokens/verify", DEV_TOKEN), + ], + include_token=True, ordered=False, ) @@ -127,14 +232,7 @@ def test_config_list_constant_in_config_count( mock_api_routes( httpx_mock, { - ("GET", "/v2/storage/components"): [ - { - "id": "keboola.ex-db", - "name": "DB", - "type": "extractor", - "configurations": _configs(n), - } - ], + ("GET", "/v2/storage/components"): _components(n), ("GET", "/v2/storage/dev-branches"): DEV_BRANCHES, ( "GET", @@ -143,17 +241,21 @@ def test_config_list_constant_in_config_count( }, ) result = _invoke(tmp_config_dir, "config", "list", "--project", "prod") - assert len(_data(result)["configs"]) == n + assert len(_data(result)["configs"]) == 3 * n assert_api_calls( httpx_mock, [ - ("GET", "/v2/storage/components"), + ("GET", "/v2/storage/components?include=configuration"), ("GET", "/v2/storage/dev-branches"), ( "GET", - f"/v2/storage/branch/{DEFAULT_BRANCH_ID}/search/component-configurations", + ( + f"/v2/storage/branch/{DEFAULT_BRANCH_ID}/search/component-configurations" + "?metadataKeys[]=KBC.configuration.folderName&include=filteredMetadata" + ), ), ], + include_query=True, ) @pytest.mark.parametrize("n_rows", SIZES) @@ -193,10 +295,23 @@ def _jobs(n: int) -> list[dict[str, Any]]: return [ { "id": str(1000 + i), + "runId": str(1000 + i), + "parentRunId": "", + "branchId": str(DEFAULT_BRANCH_ID), "status": "error" if i % 2 else "success", + "isFinished": True, + "mode": "run", "component": "keboola.ex-db", "config": "0", - "startTime": f"2026-09-01T00:{i:02d}:00+00:00", + "createdTime": f"2026-09-01T00:{i:02d}:00+00:00", + "startTime": f"2026-09-01T00:{i:02d}:05+00:00", + "endTime": f"2026-09-01T00:{i:02d}:35+00:00", + "durationSeconds": 30, + "result": ( + {"message": "Table import failed.", "error": {"type": "user"}} + if i % 2 + else {"message": "Component processing finished."} + ), } for i in range(n) ] @@ -226,8 +341,9 @@ def test_job_list_multi_project_fan_out(self, tmp_config_dir: Path, httpx_mock) assert len(_data(result)["jobs"]) == 6 assert_api_calls( httpx_mock, - [JOB_SEARCH_CALL] * 2, + [(*JOB_SEARCH_CALL, PROD_TOKEN), (*JOB_SEARCH_CALL, DEV_TOKEN)], include_query=True, + include_token=True, ordered=False, ) @@ -253,19 +369,70 @@ def test_job_detail_single_call(self, tmp_config_dir: Path, httpx_mock) -> None: # storage --------------------------------------------------------------------- +def _bucket(bucket_id: str, **extra: Any) -> dict[str, Any]: + stage, name = bucket_id.split(".", 1) + return { + "id": bucket_id, + "name": name, + "stage": stage, + "backend": "snowflake", + "backendPath": ["KBC_DB", bucket_id], + "sharing": None, + **extra, + } + + +def _table(table_id: str, bucket: dict[str, Any], i: int, **extra: Any) -> dict[str, Any]: + name = table_id.rsplit(".", 1)[1] + return { + "uri": f"https://connection.keboola.com/v2/storage/tables/{table_id}", + "id": table_id, + "name": name, + "displayName": name, + "bucket": bucket, + "columns": ["id", "value"], + "primaryKey": ["id"], + "created": "2026-09-01T00:00:00+0000", + "lastImportDate": "2026-09-01T00:00:00+0000", + "lastChangeDate": "2026-09-01T00:00:00+0000", + "rowsCount": i, + "dataSizeBytes": 1024 * i, + "isAlias": False, + "isAliasable": True, + "isTyped": False, + **extra, + } + + def _tables(n: int) -> list[dict[str, Any]]: + """``n`` plain tables, ``n`` alias tables of them, and ``n`` tables in a shared bucket.""" + main = _bucket("in.c-main") + aliases = _bucket("out.c-aliases") + shared = _bucket( + "out.c-shared", + sharing="organization", + sharedBy={"id": 7, "name": "me", "date": "2026-09-01T00:00:00+0000"}, + ) 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) + *[_table(f"in.c-main.t{i}", main, i) for i in range(n)], + *[ + _table( + f"out.c-aliases.t{i}", + aliases, + i, + isAlias=True, + isAliasable=False, + aliasColumnsAutoSync=True, + sourceTable={ + "id": f"in.c-main.t{i}", + "uri": f"https://connection.keboola.com/v2/storage/tables/in.c-main.t{i}", + "name": f"t{i}", + "project": {"id": 901, "name": "Production"}, + }, + ) + for i in range(n) + ], + *[_table(f"out.c-shared.s{i}", shared, i) for i in range(n)], ] @@ -278,8 +445,9 @@ def test_storage_tables_constant_in_table_count( 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")]) + assert len(_data(result)["tables"]) == 3 * n + # Pins the query too: the list call sends no `include` parameter. + assert_api_calls(httpx_mock, [("GET", "/v2/storage/tables")], include_query=True) 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.""" diff --git a/tests/test_data_app_service.py b/tests/test_data_app_service.py index e63c73c6..79909a4d 100644 --- a/tests/test_data_app_service.py +++ b/tests/test_data_app_service.py @@ -11,6 +11,7 @@ from __future__ import annotations +import logging from pathlib import Path from typing import Any, ClassVar from unittest.mock import MagicMock, patch @@ -2092,17 +2093,75 @@ def test_wrapped_data_shape_is_still_supported(self, httpx_mock) -> None: with self._client() as client: assert [a["id"] for a in client.list_apps()] == ["1"] - def test_page_cap_stops_a_server_that_ignores_offset(self, httpx_mock) -> None: + def test_page_cap_stops_a_server_that_ignores_offset(self, httpx_mock, caplog) -> None: httpx_mock.add_response(json=self._sandboxes(2), is_reusable=True) with ( patch("keboola_agent_cli.data_science_client.DATA_SCIENCE_APPS_PAGE_SIZE", 2), patch("keboola_agent_cli.data_science_client.DATA_SCIENCE_APPS_MAX_PAGES", 3), + caplog.at_level(logging.WARNING, logger="keboola_agent_cli.data_science_client"), self._client() as client, ): apps = client.list_apps() assert len(apps) == 6 assert len(httpx_mock.get_requests()) == 3 + assert [ + r.getMessage() + for r in caplog.records + if r.name == "keboola_agent_cli.data_science_client" + ] == ["GET /apps: stopped after 3 pages of 2; the listing may be incomplete"] + + def test_error_on_a_later_page_raises_with_no_partial_result(self, httpx_mock) -> None: + """A page that still fails after the client's retries fails the whole listing.""" + from keboola_agent_cli.constants import DATA_SCIENCE_APPS_PAGE_SIZE as size + from keboola_agent_cli.constants import MAX_RETRIES + + httpx_mock.add_response( + url=f"{self.DATA_SCIENCE_BASE}/apps?limit={size}&offset=0", + json=self._sandboxes(size), + ) + httpx_mock.add_response( + url=f"{self.DATA_SCIENCE_BASE}/apps?limit={size}&offset={size}", + status_code=503, + json={"error": "Service Unavailable"}, + is_reusable=True, + ) + with ( + patch("keboola_agent_cli.http_base.time.sleep"), + self._client() as client, + pytest.raises(KeboolaApiError) as excinfo, + ): + client.list_apps() + + assert excinfo.value.status_code == 503 + assert len(httpx_mock.get_requests()) == 1 + MAX_RETRIES + + def test_sync_type_lookup_finds_a_data_app_on_page_two(self, httpx_mock) -> None: + """``load_data_app_types`` (sync pull) reads the type through the paged listing.""" + from keboola_agent_cli.constants import DATA_SCIENCE_APPS_PAGE_SIZE as size + from keboola_agent_cli.data_science_client import DataScienceClient + from keboola_agent_cli.services._sync_data_app import load_data_app_types + + httpx_mock.add_response( + url=f"{self.DATA_SCIENCE_BASE}/apps?limit={size}&offset=0", + json=self._sandboxes(size), + ) + httpx_mock.add_response( + url=f"{self.DATA_SCIENCE_BASE}/apps?limit={size}&offset={size}", + json=[ + { + "id": "d1", + "componentId": "keboola.data-apps", + "configId": "c1", + "type": "python-js", + } + ], + ) + project = ProjectConfig(stack_url="https://connection.keboola.com", token=TEST_TOKEN) + + types = load_data_app_types(DataScienceClient, project, [{"id": "keboola.data-apps"}]) + + assert types == {"c1": "python-js"} class TestDataAppListFiltersWorkspaces: diff --git a/tests/test_integration.py b/tests/test_integration.py index c43d3797..fd13779c 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,6 +1,7 @@ -"""Integration tests for Keboola Agent CLI using real API credentials. +"""Integration tests for Keboola Agent CLI, plus offline CI guard self-tests. -These tests are skipped unless the following environment variables are set: +``TestFullWorkflow`` uses real API credentials. It is skipped unless the +following environment variables are set: - KBA_TEST_TOKEN_AWS: Storage API token for AWS stack - KBA_TEST_URL_AWS: Stack URL for AWS stack (default: https://connection.keboola.com) @@ -8,6 +9,9 @@ KBA_TEST_TOKEN_AWS=your-token uv run pytest tests/test_integration.py -v These tests exercise the full workflow: add project, list, status, config list, remove. + +The ``scripts/check_error_codes.py`` self-tests need no network and no +credentials, so they are not marked ``integration`` and run in the normal suite. """ import json @@ -304,6 +308,26 @@ def test_guard_ignores_enum_usage(self, tmp_path: Path) -> None: mod = _load_guard_script() assert mod._collect_violations(clean) == [] + def test_src_root_is_the_source_tree(self) -> None: + """main() walks SRC_ROOT; a wrong path would scan nothing and report OK.""" + mod = _load_guard_script() + assert mod.SRC_ROOT.is_dir() + assert any(mod.SRC_ROOT.rglob("*.py")) + + def test_main_fails_on_planted_literal( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() -- what `make check-error-codes` runs -- returns 1 on a raw literal.""" + pkg = tmp_path / "src" / "pkg" + pkg.mkdir(parents=True) + (pkg / "planted.py").write_text( + 'raise KeboolaApiError("oops", error_code="FOO")\n', encoding="utf-8" + ) + mod = _load_guard_script() + monkeypatch.setattr(mod, "SRC_ROOT", pkg) + monkeypatch.setattr(sys, "argv", ["check_error_codes.py"]) + assert mod.main() == 1 + class TestErrorCodesDocCompleteness: """Verify the enum-vs-docs/error-codes.md completeness guard.""" @@ -333,3 +357,21 @@ def test_detects_stale_code(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatc stale_doc.write_text(doc, encoding="utf-8") monkeypatch.setattr(mod, "DOC_PATH", stale_doc) assert mod._check_doc_completeness() is False + + def test_main_fails_on_doc_missing_a_code( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when the doc lacks an enum member, even with clean source.""" + mod = _load_guard_script() + doc_lines = mod.DOC_PATH.read_text(encoding="utf-8").splitlines(keepends=True) + pruned = [line for line in doc_lines if not line.startswith("| `INVALID_TOKEN` |")] + assert len(pruned) == len(doc_lines) - 1 + stale_doc = tmp_path / "error-codes.md" + stale_doc.write_text("".join(pruned), encoding="utf-8") + pkg = tmp_path / "src" / "pkg" + pkg.mkdir(parents=True) + (pkg / "clean.py").write_text("x = 1\n", encoding="utf-8") + monkeypatch.setattr(mod, "DOC_PATH", stale_doc) + monkeypatch.setattr(mod, "SRC_ROOT", pkg) + monkeypatch.setattr(sys, "argv", ["check_error_codes.py"]) + assert mod.main() == 1 diff --git a/tests/test_permissions_cli.py b/tests/test_permissions_cli.py index 138c9b49..a49a1959 100644 --- a/tests/test_permissions_cli.py +++ b/tests/test_permissions_cli.py @@ -331,6 +331,7 @@ def test_set_rejected_without_confirmation(self, tmp_path: Path) -> None: app, ["--json", "permissions", "set", "--mode", "allow", "--deny", "cli:write"] ) assert result.exit_code == EXIT_PERMISSION_DENIED + assert "Refusing to update permission policy" in result.output assert store.load().permissions is None def test_set_invalid_mode(self, tmp_path: Path) -> None: