diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 22323a7..975a8cd 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -65,7 +65,9 @@ Each `*API` class holds a `client: BildClient` and only issues HTTP via | Suite | Role | | --- | --- | | `tests/test_auth.py` | token required, bearer header, no Content-Type, 401/403 | -| `tests/test_client_routes.py` | every resource method hits the expected path/method | +| `tests/test_transport.py` | 4xx/5xx mapping, empty/204 bodies, timeout, base URL, `.env` | +| `tests/test_resolvers.py` | branch/version auto-resolve fallbacks and ValueErrors | +| `tests/test_client_routes.py` | every resource method hits the expected path/method/body | | `tests/test_import.py` | package import smoke | | `tests/test_live_api.py` | read-only calls against the real API when a token is present | | `tests/test_architecture.py` | layer and public-surface invariants | diff --git a/bild/client.py b/bild/client.py index 58e6588..a62a6a4 100644 --- a/bild/client.py +++ b/bild/client.py @@ -713,7 +713,13 @@ def _pick_list(payload: Any): def _safe_json(response: requests.Response): + content = getattr(response, "content", None) + if content in (b"", ""): + return None try: return response.json() except Exception: - return {"raw": response.text} + text = getattr(response, "text", "") + if not text or not str(text).strip(): + return None + return {"raw": text} diff --git a/docs/CONVENTIONS.md b/docs/CONVENTIONS.md index 41103a5..f36fef7 100644 --- a/docs/CONVENTIONS.md +++ b/docs/CONVENTIONS.md @@ -35,6 +35,7 @@ delete files, search). Do not "fix" them to `POST`. - Unit tests use a fake session. They must not need a network or token. - Route tests assert path suffix, method, and important JSON/query fields. + `tests/test_client_routes.py` must call every public `*API` method. - Live tests (`tests/test_live_api.py`) are read-only and skip without `BILD_API_KEY`. - Prefer `unittest` for class-scoped live setup; pytest collects both. diff --git a/docs/QUALITY_SCORE.md b/docs/QUALITY_SCORE.md index fee8536..ff13c9f 100644 --- a/docs/QUALITY_SCORE.md +++ b/docs/QUALITY_SCORE.md @@ -7,11 +7,11 @@ C = usable but thin; D = missing or stale. | Area | Grade | Notes | | --- | --- | --- | | Auth / token loading | B | Env + `.env` + constructor. No expiry awareness. | -| HTTP transport | B | Header contract tested. No retries or pagination helpers. | +| HTTP transport | B | Header contract, errors, empty bodies tested. No retries or pagination helpers. | | Resource coverage | A | Route tests cover the documented groups. | | Types | C | Runtime hints only; mypy is not strict on untyped defs. | -| Unit tests | B | Auth + full route table. Helpers are duplicated. | -| Live tests | B | Read-only, skip without key. Account-data dependent. | +| Unit tests | A | Auth, transport, resolvers, and per-group route table with method/body asserts. | +| Live tests | B | Read-only, skip without key. Account-data dependent; `s3Url` lists skipped. | | Docs / agent map | A | `AGENTS.md` + `docs/` catalog, linted. | | Packaging | B | setuptools, source install. Not on PyPI. | | Lint / format | A | ruff + custom harness linters in CI. | @@ -20,8 +20,7 @@ C = usable but thin; D = missing or stale. ## Gaps to close next 1. Split `bild/client.py` into transport + `bild/resources/` (tech debt). -2. Deduplicate fake session helpers in tests. -3. Tighten mypy (`check_untyped_defs`) after the split. -4. Publish to PyPI when the public surface is stable. +2. Tighten mypy (`check_untyped_defs`) after the split. +3. Publish to PyPI when the public surface is stable. See [exec-plans/tech-debt-tracker.md](exec-plans/tech-debt-tracker.md). diff --git a/docs/RELIABILITY.md b/docs/RELIABILITY.md index 9308ecc..651e4eb 100644 --- a/docs/RELIABILITY.md +++ b/docs/RELIABILITY.md @@ -5,8 +5,9 @@ - Timeout: 30 seconds per request (`timeout=` on `BildClient`). - Live tests use 60 seconds. - No automatic retries. Callers retry if they need to. -- JSON parse failures become `{"raw": response.text}` rather than raising - from the transport layer. +- Empty success bodies (HTTP 204 / no bytes) return `None`. +- Non-empty JSON parse failures become `{"raw": response.text}` rather + than raising from the transport layer. ## Error mapping diff --git a/docs/design-docs/http-client.md b/docs/design-docs/http-client.md index c8a3eb6..594f4d9 100644 --- a/docs/design-docs/http-client.md +++ b/docs/design-docs/http-client.md @@ -34,7 +34,21 @@ Resource methods that accept `branch_id: str | None` should call ## Response helper -`_safe_json` returns parsed JSON or `{"raw": response.text}`. +`_safe_json` returns parsed JSON. An empty success body (HTTP 204, or no +bytes) returns `None` so DELETE helpers are not confused with +`{"raw": ""}`. Non-empty non-JSON bodies become `{"raw": response.text}`. + `_pick_list` / `_pick_from_response` tolerate `{data: ...}` and `{items: ...}` envelopes. Keep that tolerance at the helper layer, not copied into every resource method. + +## List envelopes that point at S3 + +Some list endpoints (notably `files.list`) may return `{s3Url: ...}` +instead of an inline JSON array when the result is large. The client +returns that envelope as-is and does **not** fetch the URL. + +Following `s3Url` would be a second HTTP hop, usually to a signed URL +that must not receive the Bild `Authorization` header. Callers that need +the file list should GET the URL themselves (or use `client.get` only +against Bild paths). Do not add an automatic follow without a new spec. diff --git a/docs/exec-plans/tech-debt-tracker.md b/docs/exec-plans/tech-debt-tracker.md index 2bd8544..bc2d333 100644 --- a/docs/exec-plans/tech-debt-tracker.md +++ b/docs/exec-plans/tech-debt-tracker.md @@ -3,7 +3,6 @@ | ID | Item | Why it exists | Suggested fix | Severity | | --- | --- | --- | --- | --- | | TD-1 | `bild/client.py` holds transport and every `*API` class | Fast first implementation | Split transport vs `bild/resources/*.py`; keep public imports stable | Medium | -| TD-2 | `FakeResponse` / fake session duplicated in tests | Tests grew independently | Shared `tests/fakes.py` | Low | | TD-3 | mypy does not use `check_untyped_defs` | Current helpers are loosely typed | Annotate helpers after TD-1, then tighten | Low | | TD-4 | Package not published to PyPI | Still source-install | Release process + version policy | Low | | TD-5 | No retries / pagination helpers | API wrappers stay thin | Add only with a design doc and tests | Low | diff --git a/tests/fakes.py b/tests/fakes.py new file mode 100644 index 0000000..562110f --- /dev/null +++ b/tests/fakes.py @@ -0,0 +1,103 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any +from urllib.parse import urlparse + + +@dataclass +class FakeResponse: + status_code: int = 200 + payload: Any = None + raw_text: str | None = None + + @property + def ok(self) -> bool: + return 200 <= self.status_code < 300 + + @property + def content(self) -> bytes: + if self.raw_text is not None: + return self.raw_text.encode("utf-8") + if self.payload is None: + return b"" + return b"{}" + + @property + def text(self) -> str: + if self.raw_text is not None: + return self.raw_text + if self.payload is None: + return "" + return str(self.payload) + + def json(self) -> Any: + if self.payload is None: + raise ValueError("No JSON body") + return self.payload + + +class RecordingSession: + def __init__( + self, + status_code: int = 200, + payload: Any = None, + *, + empty: bool = False, + ): + self.headers: dict[str, str] = {} + self.calls: list[dict[str, Any]] = [] + self.status_code = status_code + if empty: + self.payload = None + else: + self.payload = {"ok": True} if payload is None else payload + + def request(self, method, url, params=None, json=None, timeout=None, **kwargs): + self.calls.append( + { + "method": method.upper(), + "url": url, + "path": urlparse(url).path, + "params": params, + "json": json, + "json_passed": "json" in kwargs or json is not None, + "headers": dict(self.headers), + "timeout": timeout, + } + ) + return FakeResponse(self.status_code, payload=self.payload) + + +class ScriptedSession: + def __init__(self, by_suffix: dict[str, FakeResponse] | None = None): + self.headers: dict[str, str] = {} + self.calls: list[dict[str, Any]] = [] + self.by_suffix = by_suffix or {} + + def request(self, method, url, params=None, json=None, timeout=None, **kwargs): + path = urlparse(url).path + self.calls.append( + { + "method": method.upper(), + "url": url, + "path": path, + "json": json, + "params": params, + "timeout": timeout, + } + ) + for suffix, response in self.by_suffix.items(): + if path.endswith(suffix): + return response + return FakeResponse(200, {"ok": True, "path": path}) + + +class RouteSession(ScriptedSession): + def __init__(self): + super().__init__( + { + "/branches": FakeResponse(200, {"data": [{"id": "branch-main", "isMain": True}]}), + "/latest": FakeResponse(200, {"data": {"fileVersion": "v-latest"}}), + } + ) diff --git a/tests/test_auth.py b/tests/test_auth.py index ddd120c..5a7f19d 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -4,8 +4,6 @@ import sys import types import unittest -from dataclasses import dataclass -from urllib.parse import urlparse try: import requests # noqa: F401 @@ -17,46 +15,7 @@ from bild import BildAuthError, BildClient from bild.client import DEFAULT_BASE_URL - - -@dataclass -class FakeResponse: - status_code: int - payload: dict - - @property - def ok(self): - return 200 <= self.status_code < 300 - - def json(self): - return self.payload - - @property - def text(self): - return str(self.payload) - - -class RecordingSession: - def __init__(self, status_code: int = 200, payload: dict | None = None): - self.headers = {} - self.calls = [] - self.status_code = status_code - self.payload = payload or {"ok": True} - - def request(self, method, url, params=None, json=None, timeout=None, **kwargs): - self.calls.append( - { - "method": method.upper(), - "url": url, - "path": urlparse(url).path, - "params": params, - "json": json, - "json_passed": "json" in kwargs or json is not None, - "headers": dict(self.headers), - "timeout": timeout, - } - ) - return FakeResponse(self.status_code, self.payload) +from tests.fakes import RecordingSession class TestBildAuth(unittest.TestCase): diff --git a/tests/test_client_routes.py b/tests/test_client_routes.py index ea6df24..3e80d2f 100644 --- a/tests/test_client_routes.py +++ b/tests/test_client_routes.py @@ -1,10 +1,10 @@ from __future__ import annotations +import inspect import sys import types import unittest -from dataclasses import dataclass -from urllib.parse import urlparse +from pathlib import Path try: import requests # noqa: F401 @@ -15,250 +15,387 @@ sys.modules["requests"] = fake_requests from bild import BildClient - - -@dataclass -class FakeResponse: - status_code: int - payload: dict - - @property - def ok(self): - return 200 <= self.status_code < 300 - - def json(self): - return self.payload - - @property - def text(self): - return str(self.payload) - - -class FakeSession: - def __init__(self): - self.calls = [] - self.headers = {} - - def request(self, method, url, params=None, json=None, timeout=None): - path = urlparse(url).path - self.calls.append({"method": method.upper(), "path": path, "json": json, "params": params}) - - if path.endswith("/branches"): - return FakeResponse(200, {"data": [{"id": "branch-main", "isMain": True}]}) - if path.endswith("/latest"): - return FakeResponse(200, {"data": {"fileVersion": "v-latest"}}) - - return FakeResponse(200, {"ok": True, "path": path}) +from bild.client import _Resources +from tests.fakes import RouteSession class TestBildClientRoutes(unittest.TestCase): def setUp(self): - self.client = BildClient(token="test-token", session=FakeSession()) + self.client = BildClient(token="test-token", session=RouteSession()) def last(self): return self.client.session.calls[-1] - def test_full_route_coverage(self): - c = self.client - - verified = c.verify() + def assert_call(self, method: str, path_suffix: str, *, json=None, params=None): + call = self.last() + self.assertEqual(call["method"], method, call) + self.assertTrue( + call["path"].endswith(path_suffix), + f"expected path ending {path_suffix!r}, got {call['path']!r}", + ) + if method in ("GET", "DELETE"): + self.assertIsNone(call["json"], call) + if json is not None: + self.assertEqual(call["json"], json) + if params is not None: + self.assertEqual(call["params"], params) + + def test_verify(self): + verified = self.client.verify() self.assertEqual(verified["function"], "BildClient.verify") self.assertTrue(verified["ok"]) - self.assertTrue(self.last()["path"].endswith("/projects")) + self.assert_call("GET", "/projects") + def test_users(self): + c = self.client c.api.users.list() - self.assertTrue(self.last()["path"].endswith("/users")) - c.api.users.invite(["a@example.com"], projects=[{"id": "p1"}]) - self.assertTrue(self.last()["path"].endswith("/users/add")) - self.assertEqual(self.last()["method"], "PUT") + self.assert_call("GET", "/users") + + c.api.users.invite( + ["a@example.com"], + projects=[{"id": "p1"}], + pdm_role="Member", + ) + self.assert_call( + "PUT", + "/users/add", + json={ + "emails": ["a@example.com"], + "projects": [{"id": "p1"}], + "pdmRole": "Member", + }, + ) + c.api.users.remove(["u1"]) - self.assertTrue(self.last()["path"].endswith("/users/remove")) - c.api.users.update(["u1"], projects=[]) - self.assertTrue(self.last()["path"].endswith("/users/update")) + self.assert_call("PUT", "/users/remove", json={"userIDs": ["u1"]}) + + c.api.users.update(["u1"], projects=[], company_role="Admin") + self.assert_call( + "PUT", + "/users/update", + json={"userIDs": ["u1"], "projects": [], "companyRole": "Admin"}, + ) + c.api.users.create_token(name="ci") - self.assertTrue(self.last()["path"].endswith("/users/apiToken")) - self.assertEqual(self.last()["method"], "POST") + self.assert_call("POST", "/users/apiToken", json={"name": "ci"}) - c.api.projects.list() - self.assertTrue(self.last()["path"].endswith("/projects")) + def test_projects(self): + self.client.api.projects.list() + self.assert_call("GET", "/projects") + def test_project_users(self): + c = self.client c.api.project_users.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/users")) + self.assert_call("GET", "/projects/p1/users") + c.api.project_users.add([{"id": "u1", "accessType": "Editor"}], project_ids=["p1"]) - self.assertTrue(self.last()["path"].endswith("/projects/users/add")) - self.assertEqual(self.last()["method"], "POST") + self.assert_call( + "POST", + "/projects/users/add", + json={"users": [{"id": "u1", "accessType": "Editor"}], "projectIDs": ["p1"]}, + ) + c.api.project_users.update([{"id": "u1", "accessType": "Viewer"}], project_ids=["p1"]) - self.assertTrue(self.last()["path"].endswith("/projects/users/update")) + self.assert_call( + "PUT", + "/projects/users/update", + json={"users": [{"id": "u1", "accessType": "Viewer"}], "projectIDs": ["p1"]}, + ) + c.api.project_users.remove(["p1"], ["u1"]) - self.assertTrue(self.last()["path"].endswith("/projects/users/remove")) + self.assert_call( + "PUT", + "/projects/users/remove", + json={"projectIDs": ["p1"], "userIDs": ["u1"]}, + ) - c.api.branches.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches")) + def test_branches(self): + self.client.api.branches.list("p1") + self.assert_call("GET", "/projects/p1/branches") + def test_commits(self): + c = self.client c.api.commits.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/commits")) + self.assert_call("GET", "/projects/p1/commits") c.api.commits.list("p1", "b1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/commits")) + self.assert_call("GET", "/projects/p1/branches/b1/commits") c.api.commits.get("p1", "b1", "c1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/commits/c1")) + self.assert_call("GET", "/projects/p1/branches/b1/commits/c1") + def test_files(self): + c = self.client c.api.files.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/files")) + self.assert_call("GET", "/projects/p1/files") c.api.files.list("p1", "b1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/files")) + self.assert_call("GET", "/projects/p1/branches/b1/files") + c.api.files.list_released("2024-01-01T00:00:00Z") - self.assertTrue(self.last()["path"].endswith("/files/released")) - self.assertEqual(self.last()["params"]["fromTime"], "2024-01-01T00:00:00Z") + self.assert_call("GET", "/files/released", params={"fromTime": "2024-01-01T00:00:00Z"}) + c.api.files.list_versions("p1", None, "f1") - self.assertTrue( - self.last()["path"].endswith("/projects/p1/branches/branch-main/files/f1/versions") - ) + self.assert_call("GET", "/projects/p1/branches/branch-main/files/f1/versions") c.api.files.get_latest("p1", None, "f1") - self.assertTrue( - self.last()["path"].endswith("/projects/p1/branches/branch-main/files/f1/latest") - ) + self.assert_call("GET", "/projects/p1/branches/branch-main/files/f1/latest") c.api.files.get_released("p1", "b1", "f1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/files/f1/released")) + self.assert_call("GET", "/projects/p1/branches/b1/files/f1/released") c.api.files.get_version("p1", "b1", "f1", "v1") - self.assertTrue( - self.last()["path"].endswith("/projects/p1/branches/b1/files/f1/versions/v1") - ) + self.assert_call("GET", "/projects/p1/branches/b1/files/f1/versions/v1") c.api.files.get_thumbnail("p1", "b1", "f1", "v1") - self.assertTrue(self.last()["path"].endswith("/thumbnail")) + self.assert_call("GET", "/thumbnail") c.api.files.get_children("p1", "b1", "f1", "v1") - self.assertTrue(self.last()["path"].endswith("/children")) + self.assert_call("GET", "/children") + c.api.files.export_universal("p1", None, "f1", output_format="stl") - self.assertTrue(self.last()["path"].endswith("/fileActions/f1/universalFormat")) - self.assertEqual(self.last()["method"], "PUT") - self.assertEqual(self.last()["json"]["fileVersionID"], "v-latest") + self.assert_call( + "PUT", + "/fileActions/f1/universalFormat", + json={"fileVersionID": "v-latest", "universalFileFormat": "stl"}, + ) + c.api.files.export_universal( + "p1", "b1", "f1", output_format="step", file_version="v1", file_config="cfg" + ) + self.assert_call( + "PUT", + "/fileActions/f1/universalFormat", + json={ + "fileVersionID": "v1", + "universalFileFormat": "step", + "fileConfig": "cfg", + }, + ) + c.api.files.export_universal_many( "p1", "b1", {"fileIDs": ["f1"], "formats": {"CAD": ["STL"]}} ) - self.assertTrue(self.last()["path"].endswith("/files/exportUniversalFiles")) + self.assert_call( + "POST", + "/files/exportUniversalFiles", + json={"fileIDs": ["f1"], "formats": {"CAD": ["STL"]}}, + ) + c.api.files.move("p1", "b1", ["f1"], "parent-1") - self.assertTrue(self.last()["path"].endswith("/fileActions/move")) + self.assert_call( + "PUT", + "/fileActions/move", + json={"moveFiles": ["f1"], "newParentID": "parent-1"}, + ) c.api.files.delete("p1", "b1", ["f1"]) - self.assertTrue(self.last()["path"].endswith("/fileActions/delete")) + self.assert_call("PUT", "/fileActions/delete", json={"fileIDs": ["f1"]}) + def test_uploads(self): + c = self.client c.api.uploads.initiate("p1", "b1", [{"name": "x"}]) - self.assertTrue(self.last()["path"].endswith("/fileActions/initiateUpload")) + self.assert_call("PUT", "/fileActions/initiateUpload", json={"files": [{"name": "x"}]}) c.api.uploads.complete("p1", "b1", [{"id": "x"}]) - self.assertTrue(self.last()["path"].endswith("/fileActions/completeUpload")) - self.assertEqual(self.last()["method"], "POST") + self.assert_call("POST", "/fileActions/completeUpload", json={"files": [{"id": "x"}]}) + c.api.uploads.complete("p1", "b1", [{"id": "x"}], keep_checked_out=True) + self.assert_call( + "POST", + "/fileActions/completeUpload", + json={"files": [{"id": "x"}], "keepFilesCheckedOut": True}, + ) + def test_checkouts(self): + c = self.client c.api.checkouts.checkout("p1", "b1", ["f1"]) - self.assertTrue(self.last()["path"].endswith("/fileActions/checkout")) + self.assert_call("PUT", "/fileActions/checkout", json={"fileIDs": ["f1"]}) c.api.checkouts.cancel("p1", "b1", ["f1"]) - self.assertTrue(self.last()["path"].endswith("/fileActions/cancelCheckout")) + self.assert_call("PUT", "/fileActions/cancelCheckout", json={"fileIDs": ["f1"]}) c.api.checkouts.initiate_checkin("p1", "b1", [{"id": "f1"}]) - self.assertTrue(self.last()["path"].endswith("/fileActions/initiateCheckin")) + self.assert_call("PUT", "/fileActions/initiateCheckin", json={"files": [{"id": "f1"}]}) c.api.checkouts.complete_checkin("p1", "b1", [{"id": "f1"}], message="v2") - self.assertTrue(self.last()["path"].endswith("/fileActions/completeCheckin")) + self.assert_call( + "POST", + "/fileActions/completeCheckin", + json={"files": [{"id": "f1"}], "message": "v2"}, + ) + def test_shared_links(self): + c = self.client c.api.shared_links.list() - self.assertTrue(self.last()["path"].endswith("/sharedLinks")) + self.assert_call("GET", "/sharedLinks") c.api.shared_links.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/sharedLinks")) + self.assert_call("GET", "/projects/p1/sharedLinks") c.api.shared_links.list("p1", "b1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/sharedLinks")) + self.assert_call("GET", "/projects/p1/branches/b1/sharedLinks") + c.api.shared_links.create_live("p1", "b1", "Review Link", ["f1"]) - self.assertTrue(self.last()["path"].endswith("/files/sharedLink")) - self.assertEqual(self.last()["method"], "POST") + self.assert_call( + "POST", + "/files/sharedLink", + json={"name": "Review Link", "fileIDs": ["f1"]}, + ) + c.api.shared_links.create_live( + "p1", + "b1", + "Review Link", + ["f1"], + types=["stl"], + config_map={"f1": "cfg"}, + ) + self.assert_call( + "POST", + "/files/sharedLink", + json={ + "name": "Review Link", + "fileIDs": ["f1"], + "types": ["stl"], + "configMap": {"f1": "cfg"}, + }, + ) + c.api.shared_links.create_static("p1", "b1", "f1", "v1") - self.assertTrue(self.last()["path"].endswith("/fileVersion/v1/sharedLink")) + self.assert_call("POST", "/fileVersion/v1/sharedLink", json={}) c.api.shared_links.refresh("p1", "b1", "s1") - self.assertTrue(self.last()["path"].endswith("/sharedLinks/s1/refresh")) + self.assert_call("PUT", "/sharedLinks/s1/refresh") + self.assertIsNone(self.last()["json"]) c.api.shared_links.delete("p1", "b1", ["s1"]) - self.assertTrue(self.last()["path"].endswith("/sharedLinks/delete")) + self.assert_call("PUT", "/sharedLinks/delete", json={"sharedLinkIDs": ["s1"]}) + def test_metadata(self): + c = self.client c.api.metadata.list_fields() - self.assertTrue(self.last()["path"].endswith("/metadataFields")) + self.assert_call("GET", "/metadataFields") c.api.metadata.get("p1", "b1", "f1") - self.assertTrue(self.last()["path"].endswith("/files/f1/metadata")) + self.assert_call("GET", "/files/f1/metadata") c.api.metadata.get_for_version("p1", "b1", "f1", "v1") - self.assertTrue(self.last()["path"].endswith("/versions/v1/metadata")) + self.assert_call("GET", "/versions/v1/metadata") c.api.metadata.update("p1", "b1", {"fileIDs": ["f1"]}) - self.assertTrue(self.last()["path"].endswith("/files/updateMetadata")) + self.assert_call("PUT", "/files/updateMetadata", json={"fileIDs": ["f1"]}) + def test_feedback(self): + c = self.client c.api.feedback.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/feedbackItems")) + self.assert_call("GET", "/projects/p1/feedbackItems") c.api.feedback.list("p1", file_id="f1") - self.assertTrue(self.last()["path"].endswith("/files/f1/feedbackItems")) + self.assert_call("GET", "/files/f1/feedbackItems") c.api.feedback.get("p1", "i1") - self.assertTrue(self.last()["path"].endswith("/feedbackItems/i1")) + self.assert_call("GET", "/feedbackItems/i1") c.api.feedback.update("p1", "i1", {"status": "inProgress"}) - self.assertEqual(self.last()["method"], "PUT") + self.assert_call("PUT", "/feedbackItems/i1", json={"status": "inProgress"}) c.api.feedback.initiate_attachment("p1", "i1", {"fileName": "a.txt"}) - self.assertTrue(self.last()["path"].endswith("/feedbackItems/i1/attachment")) + self.assert_call("PUT", "/feedbackItems/i1/attachment", json={"fileName": "a.txt"}) c.api.feedback.complete_attachment("p1", "i1", "att1", {"name": "a.txt"}) - self.assertTrue(self.last()["path"].endswith("/attachment/att1")) - self.assertEqual(self.last()["method"], "POST") + self.assert_call("POST", "/attachment/att1", json={"name": "a.txt"}) c.api.feedback.delete_attachment("p1", "i1", "att1") - self.assertEqual(self.last()["method"], "DELETE") + self.assert_call("DELETE", "/attachment/att1") + def test_packages(self): + c = self.client c.api.packages.list() - self.assertTrue(self.last()["path"].endswith("/packages")) + self.assert_call("GET", "/packages") c.api.packages.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/packages")) + self.assert_call("GET", "/projects/p1/packages") c.api.packages.get("p1", "pkg1") - self.assertTrue(self.last()["path"].endswith("/packages/pkg1")) + self.assert_call("GET", "/packages/pkg1") + def test_revisions(self): + c = self.client c.api.revisions.list() - self.assertTrue(self.last()["path"].endswith("/revisions")) + self.assert_call("GET", "/revisions") c.api.revisions.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/revisions")) + self.assert_call("GET", "/projects/p1/revisions") c.api.revisions.list("p1", "b1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/revisions")) + self.assert_call("GET", "/projects/p1/branches/b1/revisions") c.api.revisions.list("p1", "b1", "f1") - self.assertTrue(self.last()["path"].endswith("/files/f1/revisions")) + self.assert_call("GET", "/files/f1/revisions") c.api.revisions.get("p1", "b1", "f1", "r1") - self.assertTrue(self.last()["path"].endswith("/revisions/r1")) + self.assert_call("GET", "/revisions/r1") c.api.revisions.get_closure("p1", "b1", "f1") - self.assertTrue(self.last()["path"].endswith("/files/f1/closure")) + self.assert_call("GET", "/files/f1/closure") c.api.revisions.release("p1", "b1", [{"revisionID": "r1", "revisionNumber": "A"}]) - self.assertTrue(self.last()["path"].endswith("/revisions/release")) + self.assert_call( + "PUT", + "/revisions/release", + json=[{"revisionID": "r1", "revisionNumber": "A"}], + ) c.api.revisions.cancel("p1", "b1", ["r1"]) - self.assertTrue(self.last()["path"].endswith("/revisions/cancel")) + self.assert_call("PUT", "/revisions/cancel", json={"revisionIDs": ["r1"]}) + + def test_revisions_list_requires_ids(self): + with self.assertRaises(ValueError): + self.client.api.revisions.list(file_id="f1") + with self.assertRaises(ValueError): + self.client.api.revisions.list(branch_id="b1") + def test_approvals(self): + c = self.client c.api.approvals.list() - self.assertTrue(self.last()["path"].endswith("/approvals")) + self.assert_call("GET", "/approvals") c.api.approvals.list("p1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/approvals")) + self.assert_call("GET", "/projects/p1/approvals") c.api.approvals.get("p1", "a1") - self.assertTrue(self.last()["path"].endswith("/approvals/a1")) + self.assert_call("GET", "/approvals/a1") c.api.approvals.close("p1", "a1", "approved") - self.assertTrue(self.last()["path"].endswith("/approvals/a1/close")) - self.assertEqual(self.last()["params"]["status"], "approved") + self.assert_call("PUT", "/approvals/a1/close", params={"status": "approved"}) + self.assertIsNone(self.last()["json"]) + def test_boms(self): + c = self.client c.api.boms.list("p1", "b1") - self.assertTrue(self.last()["path"].endswith("/projects/p1/branches/b1/boms")) + self.assert_call("GET", "/projects/p1/branches/b1/boms") c.api.boms.get("p1", "b1", "bom1") - self.assertTrue(self.last()["path"].endswith("/boms/bom1")) + self.assert_call("GET", "/boms/bom1") c.api.boms.download( "p1", "b1", "bom1", - {"version_id": "v", "view_id": "w", "type": "Indented", "formats": {}}, + {"versionId": "v", "viewId": "w", "type": "Indented", "formats": {}}, + ) + self.assert_call( + "POST", + "/boms/bom1/download", + json={"versionId": "v", "viewId": "w", "type": "Indented", "formats": {}}, ) - self.assertTrue(self.last()["path"].endswith("/boms/bom1/download")) - c.api.search.files("bolt") - self.assertEqual(self.last()["method"], "PUT") - self.assertTrue(self.last()["path"].endswith("/search")) - self.assertEqual(self.last()["json"]["search_key"], "bolt") + def test_search(self): + self.client.api.search.files("bolt") + self.assert_call("PUT", "/search", json={"search_key": "bolt"}) + self.client.api.search.files("bolt", page_size=5, from_offset=10) + self.assert_call( + "PUT", + "/search", + json={"search_key": "bolt"}, + params={"pageSize": 5, "from": 10}, + ) + def test_webhooks(self): + c = self.client c.api.webhooks.list() - self.assertTrue(self.last()["path"].endswith("/webhooks/subscriptions")) + self.assert_call("GET", "/webhooks/subscriptions") c.api.webhooks.create({"eventType": "file.updated", "targetURL": "https://example.com"}) - self.assertEqual(self.last()["method"], "POST") + self.assert_call( + "POST", + "/webhooks/subscriptions", + json={"eventType": "file.updated", "targetURL": "https://example.com"}, + ) c.api.webhooks.get("sub1") - self.assertTrue(self.last()["path"].endswith("/webhooks/subscriptions/sub1")) + self.assert_call("GET", "/webhooks/subscriptions/sub1") c.api.webhooks.update("sub1", {"isActive": False}) - self.assertEqual(self.last()["method"], "PUT") + self.assert_call("PUT", "/webhooks/subscriptions/sub1", json={"isActive": False}) c.api.webhooks.rotate_secret("sub1") - self.assertTrue(self.last()["path"].endswith("/rotate")) + self.assert_call("POST", "/rotate") + self.assertIsNone(self.last()["json"]) c.api.webhooks.delete("sub1") - self.assertEqual(self.last()["method"], "DELETE") + self.assert_call("DELETE", "/webhooks/subscriptions/sub1") + + +class TestRouteMethodCoverage(unittest.TestCase): + def test_every_public_api_method_is_called(self): + src = Path(__file__).read_text(encoding="utf-8") + client = BildClient(token="test-token", session=RouteSession()) + missing: list[str] = [] + for name in _Resources.__annotations__: + resource = getattr(client.api, name) + for method_name, _fn in inspect.getmembers( + type(resource), predicate=inspect.isfunction + ): + if method_name.startswith("_"): + continue + needle = f"api.{name}.{method_name}(" + if needle not in src: + missing.append(needle) + self.assertEqual(missing, [], f"Route tests missing calls: {missing}") if __name__ == "__main__": diff --git a/tests/test_live_api.py b/tests/test_live_api.py index 2281f2d..8ab7aec 100644 --- a/tests/test_live_api.py +++ b/tests/test_live_api.py @@ -115,12 +115,13 @@ def test_verify_handshake(self): result = self.client.verify() self.assertTrue(result["ok"]) self.assertEqual(result["function"], "BildClient.verify") - self.assertTrue(result.get("users")) - self.assertTrue(result.get("projects")) + self.assertIn("users", result) + self.assertIn("projects", result) def test_list_users(self): users = _as_list(self.client.api.users.list()) - self.assertGreater(len(users), 0) + if not users: + self.skipTest("No users returned") self.assertTrue(users[0].get("id") or users[0].get("email")) def test_list_projects(self): @@ -141,6 +142,10 @@ def test_list_commits(self): commits = _as_list(self.client.api.commits.list(self.project_id, self.branch_id), "commits") self.assertIsInstance(commits, list) + def test_list_commits_without_branch(self): + commits = _as_list(self.client.api.commits.list(self.project_id), "commits") + self.assertIsInstance(commits, list) + def test_get_commit(self): if not self.commit_id: self.skipTest("No commits available") @@ -152,11 +157,15 @@ def test_get_commit(self): def test_list_files(self): payload = self.client.api.files.list(self.project_id, self.branch_id) self.assertIsInstance(payload, dict) - self.assertTrue( - isinstance(payload.get("data"), (list, dict)) - or payload.get("s3Url") - or payload.get("items") - ) + if payload.get("s3Url") and not payload.get("data") and not payload.get("items"): + self.skipTest("files.list returned an s3Url envelope; the SDK does not fetch it") + self.assertTrue(isinstance(payload.get("data"), (list, dict)) or payload.get("items")) + + def test_list_files_without_branch(self): + payload = self.client.api.files.list(self.project_id) + self.assertIsInstance(payload, (dict, list)) + if isinstance(payload, dict) and payload.get("s3Url") and not payload.get("data"): + self.skipTest("files.list returned an s3Url envelope; the SDK does not fetch it") def test_list_released_files(self): payload = self.client.api.files.list_released("2026-01-01T00:00:00Z") @@ -277,6 +286,12 @@ def test_approvals(self): project = self.client.api.approvals.list(self.project_id) self.assertIsInstance(account, dict) self.assertIsInstance(project, dict) + items = _as_list(account, "approvals") + approval_id = _first_id(items, "id") + if approval_id: + approval_project = items[0].get("projectID") or self.project_id + detail = self.client.api.approvals.get(approval_project, approval_id) + self.assertIsInstance(detail, dict) def test_boms(self): payload = self.client.api.boms.list(self.project_id, self.branch_id) @@ -291,11 +306,16 @@ def test_search_files(self): payload = self.client.api.search.files("bolt", page_size=5) self.assertIsInstance(payload, dict) files = _as_list(payload, "files") - self.assertGreater(len(files), 0) + self.assertIsInstance(files, list) def test_webhooks(self): payload = self.client.api.webhooks.list() self.assertIsInstance(payload, (dict, list)) + items = _as_list(payload) + subscription_id = _first_id(items, "id") + if subscription_id: + detail = self.client.api.webhooks.get(subscription_id) + self.assertIsInstance(detail, dict) def test_released_file_is_optional(self): if not self.file_id: diff --git a/tests/test_resolvers.py b/tests/test_resolvers.py new file mode 100644 index 0000000..e0f2084 --- /dev/null +++ b/tests/test_resolvers.py @@ -0,0 +1,102 @@ +from __future__ import annotations + +import sys +import types +import unittest + +try: + import requests # noqa: F401 +except ImportError: + fake_requests = types.ModuleType("requests") + fake_requests.Session = object + fake_requests.Response = object + sys.modules["requests"] = fake_requests + +from bild import BildClient +from tests.fakes import FakeResponse, RecordingSession, ScriptedSession + + +class TestResolveBranchId(unittest.TestCase): + def _client(self, payload) -> BildClient: + session = ScriptedSession({"/branches": FakeResponse(200, payload)}) + return BildClient(token="test-token", session=session) + + def test_explicit_id_skips_lookup(self): + session = RecordingSession() + client = BildClient(token="test-token", session=session) + self.assertEqual(client.resolve_branch_id("p1", "given"), "given") + self.assertEqual(session.calls, []) + + def test_prefers_is_main(self): + client = self._client( + { + "data": [ + {"id": "other", "name": "dev"}, + {"id": "main-id", "isMain": True}, + ] + } + ) + self.assertEqual(client.resolve_branch_id("p1"), "main-id") + + def test_prefers_is_default(self): + client = self._client({"data": [{"id": "def-id", "isDefault": True}]}) + self.assertEqual(client.resolve_branch_id("p1"), "def-id") + + def test_prefers_default_flag(self): + client = self._client({"items": [{"id": "flag-id", "default": True}]}) + self.assertEqual(client.resolve_branch_id("p1"), "flag-id") + + def test_falls_back_to_name_main(self): + client = self._client({"data": [{"id": "named", "name": "Main"}]}) + self.assertEqual(client.resolve_branch_id("p1"), "named") + + def test_falls_back_to_name_master(self): + client = self._client({"data": [{"branchId": "master-id", "name": "master"}]}) + self.assertEqual(client.resolve_branch_id("p1"), "master-id") + + def test_falls_back_to_first_branch(self): + client = self._client({"data": [{"id": "only"}]}) + self.assertEqual(client.resolve_branch_id("p1"), "only") + + def test_empty_list_raises(self): + client = self._client({"data": []}) + with self.assertRaises(ValueError): + client.resolve_branch_id("p1") + + def test_missing_ids_raise(self): + client = self._client({"data": [{"name": "main"}, "skip-me"]}) + with self.assertRaises(ValueError): + client.resolve_branch_id("p1") + + +class TestResolveFileVersion(unittest.TestCase): + def test_explicit_version_skips_lookup(self): + session = RecordingSession() + client = BildClient(token="test-token", session=session) + self.assertEqual( + client.resolve_file_version("p1", "b1", "f1", "v-given"), + "v-given", + ) + self.assertEqual(session.calls, []) + + def test_reads_nested_file_version(self): + session = ScriptedSession( + {"/latest": FakeResponse(200, {"data": {"fileVersion": "v-latest"}})} + ) + client = BildClient(token="test-token", session=session) + self.assertEqual(client.resolve_file_version("p1", "b1", "f1"), "v-latest") + + def test_reads_file_version_id(self): + session = ScriptedSession({"/latest": FakeResponse(200, {"fileVersionID": "v-id"})}) + client = BildClient(token="test-token", session=session) + self.assertEqual(client.resolve_file_version("p1", "b1", "f1"), "v-id") + + def test_missing_version_raises(self): + session = ScriptedSession({"/latest": FakeResponse(200, {"data": {}})}) + client = BildClient(token="test-token", session=session) + with self.assertRaises(ValueError): + client.resolve_file_version("p1", "b1", "f1") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_transport.py b/tests/test_transport.py new file mode 100644 index 0000000..38e8a3f --- /dev/null +++ b/tests/test_transport.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +import os +import sys +import tempfile +import types +import unittest +from pathlib import Path +from unittest.mock import patch + +try: + import requests # noqa: F401 +except ImportError: + fake_requests = types.ModuleType("requests") + fake_requests.Session = object + fake_requests.Response = object + sys.modules["requests"] = fake_requests + +from bild import BildAPIError, BildAuthError, BildClient +from bild.client import _load_env_file, _safe_json +from tests.fakes import FakeResponse, RecordingSession + + +class TestTransport(unittest.TestCase): + def test_constructor_token_wins_over_env(self): + previous = os.environ.get("BILD_API_KEY") + os.environ["BILD_API_KEY"] = "env-token" + try: + session = RecordingSession() + BildClient(token="ctor-token", session=session) + self.assertEqual(session.headers["Authorization"], "Bearer ctor-token") + finally: + if previous is None: + os.environ.pop("BILD_API_KEY", None) + else: + os.environ["BILD_API_KEY"] = previous + + def test_custom_base_url_and_timeout(self): + session = RecordingSession() + client = BildClient( + token="jwt-token", + base_url="https://example.test/api/", + timeout=12.5, + session=session, + ) + client.api.users.list() + call = session.calls[-1] + self.assertEqual(call["url"], "https://example.test/api/users") + self.assertEqual(call["timeout"], 12.5) + + def test_404_raises_api_error_not_auth(self): + session = RecordingSession(status_code=404, payload={"message": "missing"}) + client = BildClient(token="jwt-token", session=session) + with self.assertRaises(BildAPIError) as ctx: + client.api.projects.list() + self.assertNotIsInstance(ctx.exception, BildAuthError) + self.assertEqual(ctx.exception.status_code, 404) + self.assertEqual(ctx.exception.payload, {"message": "missing"}) + + def test_500_raises_api_error(self): + session = RecordingSession(status_code=500, payload={"message": "boom"}) + client = BildClient(token="jwt-token", session=session) + with self.assertRaises(BildAPIError) as ctx: + client.api.projects.list() + self.assertEqual(ctx.exception.status_code, 500) + self.assertEqual(ctx.exception.payload, {"message": "boom"}) + + def test_delete_does_not_send_json_body(self): + session = RecordingSession() + client = BildClient(token="jwt-token", session=session) + client.api.webhooks.delete("sub1") + call = session.calls[-1] + self.assertEqual(call["method"], "DELETE") + self.assertEqual(call["path"], "/webhooks/subscriptions/sub1") + self.assertIsNone(call["json"]) + self.assertFalse(call["json_passed"]) + self.assertNotIn("Content-Type", call["headers"]) + + def test_empty_204_returns_none(self): + session = RecordingSession(status_code=204, empty=True) + client = BildClient(token="jwt-token", session=session) + result = client.api.webhooks.delete("sub1") + self.assertIsNone(result) + + def test_non_json_body_returns_raw(self): + response = FakeResponse(status_code=200, payload=None, raw_text="nope") + self.assertEqual(_safe_json(response), {"raw": "nope"}) + + def test_empty_body_returns_none(self): + response = FakeResponse(status_code=204, payload=None) + self.assertIsNone(_safe_json(response)) + + def test_env_file_does_not_override_existing(self): + with tempfile.TemporaryDirectory() as tmp: + env_path = Path(tmp) / ".env" + env_path.write_text("BILD_SDK_ENV_LOAD_TEST=from-file\n", encoding="utf-8") + with patch.dict(os.environ, {"BILD_SDK_ENV_LOAD_TEST": "from-process"}): + with patch("bild.client.Path.cwd", return_value=Path(tmp)): + _load_env_file() + self.assertEqual(os.environ["BILD_SDK_ENV_LOAD_TEST"], "from-process") + + def test_env_file_sets_missing_key(self): + previous = os.environ.pop("BILD_SDK_ENV_LOAD_TEST", None) + try: + with tempfile.TemporaryDirectory() as tmp: + env_path = Path(tmp) / ".env" + env_path.write_text("BILD_SDK_ENV_LOAD_TEST=from-file\n", encoding="utf-8") + with patch("bild.client.Path.cwd", return_value=Path(tmp)): + _load_env_file() + self.assertEqual(os.environ["BILD_SDK_ENV_LOAD_TEST"], "from-file") + finally: + if previous is None: + os.environ.pop("BILD_SDK_ENV_LOAD_TEST", None) + else: + os.environ["BILD_SDK_ENV_LOAD_TEST"] = previous + + +if __name__ == "__main__": + unittest.main()