diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d46c8d7..36a2ae9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,7 +12,7 @@ jobs: with: python-version: ${{ matrix.python-version }} - run: uv sync --all-extras - - run: uv run pytest --cov --cov-report=xml + - run: uv run pytest --cov --cov-report=xml --cov-fail-under=98 - name: Upload coverage if: matrix.python-version == '3.13' uses: codecov/codecov-action@v7 diff --git a/.gitignore b/.gitignore index 81eb142..d62456e 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ Thumbs.db # 1Password shell-plugin config: local tool state, not project config. .op/ + +# Internal ticket tracker and spec; never committed. +.internal/ diff --git a/pyproject.toml b/pyproject.toml index 5b3945e..a39af1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,6 +104,7 @@ addopts = "-m 'not network'" markers = ["network: hits the live internet; excluded from the default run"] [tool.coverage.run] +source = ["src"] omit = ["src/mcp_gitlab/__init__.py", "src/mcp_gitlab/__main__.py"] [tool.mypy] diff --git a/tests/conftest.py b/tests/conftest.py index 94e557d..99eea13 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,27 +2,74 @@ from __future__ import annotations +from collections.abc import AsyncIterator, Iterator +from contextlib import asynccontextmanager +from typing import Any + import pytest import respx +from fastmcp import Client, FastMCP from mcp_gitlab.client import GitLabClient from mcp_gitlab.config import GitLabConfig +from mcp_gitlab.servers.gitlab import mcp TEST_URL = "https://gitlab.example.com" TEST_TOKEN = "test-token" +API_BASE = f"{TEST_URL}/api/v4" @pytest.fixture -def config() -> GitLabConfig: - return GitLabConfig(url=TEST_URL, token=TEST_TOKEN) +async def client() -> AsyncIterator[GitLabClient]: + """A bare GitLabClient for client-level tests, closed after the test.""" + gl = GitLabClient(GitLabConfig(url=TEST_URL, token=TEST_TOKEN)) + yield gl + await gl.close() @pytest.fixture -def client(config: GitLabConfig) -> GitLabClient: - return GitLabClient(config) +def mock_api() -> Iterator[respx.MockRouter]: + with respx.mock(base_url=API_BASE) as router: + yield router + + +@asynccontextmanager +async def _tool_client(*, read_only: bool) -> AsyncIterator[tuple[Client, respx.MockRouter]]: + """The real ``mcp`` server, its lifespan swapped for one that hands tools a + GitLabClient pointed at a respx-mocked API. + + The original lifespan is restored in ``finally`` so a failure inside + ``Client(mcp)`` cannot leak the mock into later tests. + """ + config = GitLabConfig(url=TEST_URL, token=TEST_TOKEN, read_only=read_only) + gl = GitLabClient(config) + + @asynccontextmanager + async def mock_lifespan(server: FastMCP) -> AsyncIterator[dict[str, Any]]: + try: + yield {"client": gl, "config": config} + finally: + await gl.close() + + original = mcp._lifespan + mcp._lifespan = mock_lifespan + try: + with respx.mock(base_url=API_BASE) as router: + async with Client(mcp) as mcp_client: + yield mcp_client, router + finally: + mcp._lifespan = original @pytest.fixture -def mock_api() -> respx.MockRouter: - with respx.mock(base_url="https://gitlab.example.com/api/v4") as router: - yield router +async def tool_client() -> AsyncIterator[tuple[Client, respx.MockRouter]]: + """(FastMCP client, respx router) against the server in read-write mode.""" + async with _tool_client(read_only=False) as pair: + yield pair + + +@pytest.fixture +async def readonly_client() -> AsyncIterator[tuple[Client, respx.MockRouter]]: + """(FastMCP client, respx router) against the server in read-only mode.""" + async with _tool_client(read_only=True) as pair: + yield pair diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 67eb110..4f9367d 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -4,18 +4,10 @@ import httpx import pytest -import respx from mcp_gitlab.client import GitLabClient -from mcp_gitlab.config import GitLabConfig from mcp_gitlab.exceptions import GitLabApiError, GitLabAuthError, GitLabNotFoundError -BASE = "https://gitlab.example.com/api/v4" - - -def _make_client() -> GitLabClient: - return GitLabClient(GitLabConfig(url="https://gitlab.example.com", token="test-token")) - class TestEncodeId: def test_numeric_string(self): @@ -85,61 +77,45 @@ def test_self_hosted_with_port(self): class TestRequest: - @pytest.mark.asyncio - async def test_get_project(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123").mock( - return_value=httpx.Response(200, json={"id": 123, "name": "test"}) - ) - client = _make_client() - result = await client.get_project(123) - assert result["id"] == 123 - assert result["name"] == "test" - - @pytest.mark.asyncio - async def test_auth_error_401(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123").mock(return_value=httpx.Response(401, text="Unauthorized")) - client = _make_client() - with pytest.raises(GitLabAuthError) as exc_info: - await client.get_project(123) - assert exc_info.value.status_code == 401 - - @pytest.mark.asyncio - async def test_not_found_error(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/999").mock(return_value=httpx.Response(404, text="Not Found")) - client = _make_client() - with pytest.raises(GitLabNotFoundError): - await client.get_project(999) - - @pytest.mark.asyncio - async def test_server_error(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123").mock( - return_value=httpx.Response(500, text="Internal Server Error") - ) - client = _make_client() - with pytest.raises(GitLabApiError) as exc_info: - await client.get_project(123) - assert exc_info.value.status_code == 500 - - @pytest.mark.asyncio - async def test_html_response_error(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123").mock( - return_value=httpx.Response( - 200, - text="Login", - headers={"content-type": "text/html"}, - ) + async def test_get_project(self, client, mock_api): + mock_api.get("/projects/123").mock( + return_value=httpx.Response(200, json={"id": 123, "name": "test"}) + ) + result = await client.get_project(123) + assert result["id"] == 123 + assert result["name"] == "test" + + async def test_auth_error_401(self, client, mock_api): + mock_api.get("/projects/123").mock(return_value=httpx.Response(401, text="Unauthorized")) + with pytest.raises(GitLabAuthError) as exc_info: + await client.get_project(123) + assert exc_info.value.status_code == 401 + + async def test_not_found_error(self, client, mock_api): + mock_api.get("/projects/999").mock(return_value=httpx.Response(404, text="Not Found")) + with pytest.raises(GitLabNotFoundError): + await client.get_project(999) + + async def test_server_error(self, client, mock_api): + mock_api.get("/projects/123").mock( + return_value=httpx.Response(500, text="Internal Server Error") + ) + with pytest.raises(GitLabApiError) as exc_info: + await client.get_project(123) + assert exc_info.value.status_code == 500 + + async def test_html_response_error(self, client, mock_api): + mock_api.get("/projects/123").mock( + return_value=httpx.Response( + 200, + text="Login", + headers={"content-type": "text/html"}, ) - client = _make_client() - with pytest.raises(GitLabApiError, match="HTML"): - await client.get_project(123) + ) + with pytest.raises(GitLabApiError, match="HTML"): + await client.get_project(123) - @pytest.mark.asyncio - async def test_html_response_error_on_raw_path(self): + async def test_html_response_error_on_raw_path(self, client, mock_api): """A raw read must not hand back a login page as content. The raw return used to sit above the HTML guard, so an auth redirect @@ -147,115 +123,87 @@ async def test_html_response_error_on_raw_path(self): get_job_log is the only raw=True caller, and a trace is text/plain, so HTML here is always a failure rather than a payload. """ - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/jobs/7/trace").mock( - return_value=httpx.Response( - 200, - text="Login", - headers={"content-type": "text/html"}, - ) + mock_api.get("/projects/123/jobs/7/trace").mock( + return_value=httpx.Response( + 200, + text="Login", + headers={"content-type": "text/html"}, ) - client = _make_client() - with pytest.raises(GitLabApiError, match="HTML"): - await client.get_job_log(123, 7) + ) + with pytest.raises(GitLabApiError, match="HTML"): + await client.get_job_log(123, 7) - @pytest.mark.asyncio - async def test_raw_path_still_returns_plain_text(self): + async def test_raw_path_still_returns_plain_text(self, client, mock_api): """The guard must not swallow legitimate raw traces.""" - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/jobs/7/trace").mock( - return_value=httpx.Response( - 200, - text="$ echo build\nbuild ok\n", - headers={"content-type": "text/plain"}, - ) + mock_api.get("/projects/123/jobs/7/trace").mock( + return_value=httpx.Response( + 200, + text="$ echo build\nbuild ok\n", + headers={"content-type": "text/plain"}, ) - client = _make_client() - assert "build ok" in await client.get_job_log(123, 7) - - @pytest.mark.asyncio - async def test_empty_response(self): - async with respx.mock(base_url=BASE) as router: - router.delete("/projects/123").mock(return_value=httpx.Response(204)) - client = _make_client() - result = await client.delete_project(123) - assert result is None - - @pytest.mark.asyncio - async def test_list_branches(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/repository/branches").mock( - return_value=httpx.Response(200, json=[{"name": "main"}, {"name": "develop"}]) - ) - client = _make_client() - branches, next_page = await client.list_branches(123) - assert len(branches) == 2 - assert branches[0]["name"] == "main" - assert next_page is None # no X-Next-Page header -> last page - - @pytest.mark.asyncio - async def test_list_reports_next_page(self): + ) + assert "build ok" in await client.get_job_log(123, 7) + + async def test_empty_response(self, client, mock_api): + mock_api.delete("/projects/123").mock(return_value=httpx.Response(204)) + result = await client.delete_project(123) + assert result is None + + async def test_list_branches(self, client, mock_api): + mock_api.get("/projects/123/repository/branches").mock( + return_value=httpx.Response(200, json=[{"name": "main"}, {"name": "develop"}]) + ) + branches, next_page = await client.list_branches(123) + assert len(branches) == 2 + assert branches[0]["name"] == "main" + assert next_page is None # no X-Next-Page header -> last page + + async def test_list_reports_next_page(self, client, mock_api): """X-Next-Page is the only signal that a list was cut short.""" - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/repository/branches").mock( - return_value=httpx.Response( - 200, - json=[{"name": "main"}], - headers={"X-Next-Page": "2", "X-Total-Pages": "7"}, - ) + mock_api.get("/projects/123/repository/branches").mock( + return_value=httpx.Response( + 200, + json=[{"name": "main"}], + headers={"X-Next-Page": "2", "X-Total-Pages": "7"}, ) - client = _make_client() - branches, next_page = await client.list_branches(123) - assert len(branches) == 1 - assert next_page == 2 + ) + branches, next_page = await client.list_branches(123) + assert len(branches) == 1 + assert next_page == 2 - @pytest.mark.asyncio - async def test_blank_next_page_header_means_last_page(self): + async def test_blank_next_page_header_means_last_page(self, client, mock_api): """GitLab sends X-Next-Page as an empty string on the final page.""" - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/repository/branches").mock( - return_value=httpx.Response( - 200, json=[{"name": "main"}], headers={"X-Next-Page": ""} - ) - ) - client = _make_client() - _branches, next_page = await client.list_branches(123) - assert next_page is None - - @pytest.mark.asyncio - async def test_create_merge_request(self): - async with respx.mock(base_url=BASE) as router: - router.post("/projects/123/merge_requests").mock( - return_value=httpx.Response(201, json={"iid": 1, "title": "Test MR"}) - ) - client = _make_client() - result = await client.create_merge_request( - 123, - { - "source_branch": "feature", - "target_branch": "main", - "title": "Test MR", - }, - ) - assert result["iid"] == 1 + mock_api.get("/projects/123/repository/branches").mock( + return_value=httpx.Response(200, json=[{"name": "main"}], headers={"X-Next-Page": ""}) + ) + _branches, next_page = await client.list_branches(123) + assert next_page is None - @pytest.mark.asyncio - async def test_get_job_log(self): - async with respx.mock(base_url=BASE) as router: - router.get("/projects/123/jobs/456/trace").mock( - return_value=httpx.Response(200, text="line1\nline2\nline3") - ) - client = _make_client() - result = await client.get_job_log(123, 456) - assert "line1" in result - assert "line3" in result - - @pytest.mark.asyncio - async def test_path_encoding(self): - async with respx.mock(base_url=BASE) as router: - route = router.get("/projects/my-group%2Fmy-project").mock( - return_value=httpx.Response(200, json={"id": 1}) - ) - client = _make_client() - await client.get_project("my-group/my-project") - assert route.called + async def test_create_merge_request(self, client, mock_api): + mock_api.post("/projects/123/merge_requests").mock( + return_value=httpx.Response(201, json={"iid": 1, "title": "Test MR"}) + ) + result = await client.create_merge_request( + 123, + { + "source_branch": "feature", + "target_branch": "main", + "title": "Test MR", + }, + ) + assert result["iid"] == 1 + + async def test_get_job_log(self, client, mock_api): + mock_api.get("/projects/123/jobs/456/trace").mock( + return_value=httpx.Response(200, text="line1\nline2\nline3") + ) + result = await client.get_job_log(123, 456) + assert "line1" in result + assert "line3" in result + + async def test_path_encoding(self, client, mock_api): + route = mock_api.get("/projects/my-group%2Fmy-project").mock( + return_value=httpx.Response(200, json={"id": 1}) + ) + await client.get_project("my-group/my-project") + assert route.called diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index ee1e2a8..19786d9 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -1,11 +1,40 @@ -"""Tests for exceptions.""" +"""Tests for exceptions and the _err envelope that maps them.""" + +import json + +import pytest from mcp_gitlab.exceptions import ( GitLabApiError, GitLabAuthError, + GitLabError, GitLabNotFoundError, GitLabWriteDisabledError, ) +from mcp_gitlab.servers.gitlab import _err + +API = {"error", "status_code", "body"} +HINTED = API | {"hint"} + + +@pytest.mark.parametrize( + ("exc", "keys"), + [ + (GitLabError("base"), {"error"}), + (GitLabApiError(500, "Internal Server Error", "boom"), API), + (GitLabApiError(409, "Conflict"), HINTED), + (GitLabApiError(422, "Unprocessable"), HINTED), + (GitLabApiError(429, "Too Many Requests"), HINTED), + (GitLabAuthError(401), HINTED), + (GitLabAuthError(403), HINTED), + (GitLabNotFoundError("gone"), HINTED), + (GitLabWriteDisabledError(), {"error", "hint"}), + ], + ids=["base", "api-500", "api-409", "api-422", "api-429", "auth-401", "auth-403", "404", "ro"], +) +def test_err_envelope_key_set(exc, keys): + """Exact key set per exception type; dropping ``body`` or ``hint`` fails here.""" + assert json.loads(_err(exc)).keys() == keys def test_api_error(): diff --git a/tests/unit/test_server_assembly.py b/tests/unit/test_server_assembly.py new file mode 100644 index 0000000..f21d794 --- /dev/null +++ b/tests/unit/test_server_assembly.py @@ -0,0 +1,42 @@ +"""Boot the server through its real lifespan and check what it registers. + +The count of ``@mcp.tool`` / ``@mcp.resource`` / ``@mcp.prompt`` decorators is read +from ``src/`` at test time, so a tool that silently drops out of the registry (a +decorator that breaks introspection, a module that stops being imported) fails here +rather than in a client. +""" + +from __future__ import annotations + +import importlib +import re +from pathlib import Path + +from fastmcp import Client + +from mcp_gitlab.servers.gitlab import mcp + +SRC = Path(__file__).resolve().parents[2] / "src" + + +def _decorators(kind: str) -> int: + pattern = re.compile(rf"^@mcp\.{kind}\b", re.M) + return sum(len(pattern.findall(p.read_text())) for p in SRC.rglob("*.py")) + + +async def test_real_lifespan_registers_everything(monkeypatch): + monkeypatch.setenv("GITLAB_URL", "https://gitlab.example.com") + monkeypatch.setenv("GITLAB_TOKEN", "test-token") + monkeypatch.delenv("GITLAB_READ_ONLY", raising=False) + # main() imports these for their decorators; do the same here. + for module in ("mcp_gitlab.servers.resources", "mcp_gitlab.servers.prompts"): + importlib.import_module(module) + + async with Client(mcp) as client: + assert len(await client.list_tools()) == _decorators("tool") + resources = await client.list_resources() + assert len(resources) == _decorators("resource") > 0 + assert len(await client.list_prompts()) == _decorators("prompt") > 0 + for resource in resources: + (content,) = await client.read_resource(resource.uri) + assert content.text.lstrip().startswith("#"), resource.uri diff --git a/tests/unit/test_tool_contract.py b/tests/unit/test_tool_contract.py new file mode 100644 index 0000000..1029d33 --- /dev/null +++ b/tests/unit/test_tool_contract.py @@ -0,0 +1,694 @@ +"""Wire contract for every ``@mcp.tool``: the request it sends, the envelope it returns. + +One row per tool: ``(tool_name, call_args, method, path, expected_query, expected_json_body)``. +Rows were derived by reading each tool body and the ``GitLabClient`` method it calls, not +guessed. They are what makes collapsing the tool bodies -- and later the one-caller client +methods -- a mechanical change rather than a leap. + +``test_request_shape`` pins what goes over the wire on success; ``test_forced_failure`` +drives the same route to 404 so every tool's ``except`` arm actually executes and its +``_err`` envelope is checked. ``test_every_tool_has_a_row`` keeps the table honest. +""" + +from __future__ import annotations + +import json + +import pytest +from httpx import Response + +P = "/projects/123" +MR = f"{P}/merge_requests/1" +G = "/groups/10" +VARS = [{"key": "A", "value": "1"}] +ACTIONS = [{"action": "create", "file_path": "a.txt", "content": "x"}] +LINKS = [{"name": "bin", "url": "https://example.com/bin"}] + +ROWS = [ + # Projects + ("gitlab_get_project", {"project_id": "123"}, "GET", P, {}, None), + ( + "gitlab_create_project", + {"name": "proj", "visibility": "private"}, + "POST", + "/projects", + {}, + {"name": "proj", "visibility": "private"}, + ), + ("gitlab_delete_project", {"project_id": "123"}, "DELETE", P, {}, None), + ( + "gitlab_update_project_merge_settings", + {"project_id": "123", "merge_method": "ff"}, + "PUT", + P, + {}, + {"merge_method": "ff"}, + ), + # Project approvals + ("gitlab_get_project_approvals", {"project_id": "123"}, "GET", f"{P}/approvals", {}, None), + ( + "gitlab_update_project_approvals", + {"project_id": "123", "approvals_before_merge": 2}, + "POST", + f"{P}/approvals", + {}, + {"approvals_before_merge": 2}, + ), + ( + "gitlab_list_project_approval_rules", + {"project_id": "123"}, + "GET", + f"{P}/approval_rules", + {}, + None, + ), + ( + "gitlab_create_project_approval_rule", + {"project_id": "123", "name": "r", "approvals_required": 1, "user_ids": [5]}, + "POST", + f"{P}/approval_rules", + {}, + {"name": "r", "approvals_required": 1, "user_ids": [5]}, + ), + ( + "gitlab_update_project_approval_rule", + {"project_id": "123", "rule_id": 4, "approvals_required": 2}, + "PUT", + f"{P}/approval_rules/4", + {}, + {"approvals_required": 2}, + ), + ( + "gitlab_delete_project_approval_rule", + {"project_id": "123", "rule_id": 4}, + "DELETE", + f"{P}/approval_rules/4", + {}, + None, + ), + # MR approval rules + ( + "gitlab_list_mr_approval_rules", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/approval_rules", + {}, + None, + ), + ( + "gitlab_create_mr_approval_rule", + {"project_id": "123", "mr_iid": 1, "name": "r", "approvals_required": 1}, + "POST", + f"{MR}/approval_rules", + {}, + {"name": "r", "approvals_required": 1}, + ), + ( + "gitlab_update_mr_approval_rule", + {"project_id": "123", "mr_iid": 1, "rule_id": 4, "name": "n"}, + "PUT", + f"{MR}/approval_rules/4", + {}, + {"name": "n"}, + ), + ( + "gitlab_delete_mr_approval_rule", + {"project_id": "123", "mr_iid": 1, "rule_id": 4}, + "DELETE", + f"{MR}/approval_rules/4", + {}, + None, + ), + # Groups + ( + "gitlab_list_groups", + {"search": "team"}, + "GET", + "/groups", + {"per_page": "50", "search": "team", "page": "1"}, + None, + ), + ("gitlab_get_group", {"group_id": "10"}, "GET", G, {}, None), + ( + "gitlab_share_project_with_group", + {"project_id": "123", "group_id": 10, "access_level": "developer"}, + "POST", + f"{P}/share", + {}, + {"group_id": 10, "group_access": 30}, + ), + ( + "gitlab_unshare_project_with_group", + {"project_id": "123", "group_id": 10}, + "DELETE", + f"{P}/share/10", + {}, + None, + ), + ( + "gitlab_share_group_with_group", + {"target_group_id": "10", "source_group_id": 20, "access_level": "maintainer"}, + "POST", + f"{G}/share", + {}, + {"group_id": 20, "group_access": 40}, + ), + ( + "gitlab_unshare_group_with_group", + {"target_group_id": "10", "source_group_id": 20}, + "DELETE", + f"{G}/share/20", + {}, + None, + ), + # Branches + ( + "gitlab_list_branches", + {"project_id": "123", "search": "feat"}, + "GET", + f"{P}/repository/branches", + {"per_page": "100", "search": "feat", "page": "1"}, + None, + ), + ( + "gitlab_create_branch", + {"project_id": "123", "branch_name": "feat/x", "ref": "main"}, + "POST", + f"{P}/repository/branches", + {}, + {"branch": "feat/x", "ref": "main"}, + ), + ( + "gitlab_delete_branch", + {"project_id": "123", "branch_name": "old"}, + "DELETE", + f"{P}/repository/branches/old", + {}, + None, + ), + # Commits + ( + "gitlab_list_commits", + {"project_id": "123", "ref_name": "main"}, + "GET", + f"{P}/repository/commits", + {"per_page": "40", "ref_name": "main", "page": "1"}, + None, + ), + ( + "gitlab_get_commit", + {"project_id": "123", "sha": "abc"}, + "GET", + f"{P}/repository/commits/abc", + {}, + None, + ), + ( + "gitlab_create_commit", + {"project_id": "123", "branch": "main", "commit_message": "m", "actions": ACTIONS}, + "POST", + f"{P}/repository/commits", + {}, + {"branch": "main", "commit_message": "m", "actions": ACTIONS}, + ), + ( + "gitlab_compare", + {"project_id": "123", "from": "main", "to": "dev"}, + "GET", + f"{P}/repository/compare", + {"from": "main", "to": "dev"}, + None, + ), + # Merge requests + ( + "gitlab_list_mrs", + {"project_id": "123", "state": "opened"}, + "GET", + f"{P}/merge_requests", + {"per_page": "20", "state": "opened", "page": "1"}, + None, + ), + ("gitlab_get_mr", {"project_id": "123", "mr_iid": 1}, "GET", MR, {}, None), + ( + "gitlab_create_mr", + {"project_id": "123", "source_branch": "f", "target_branch": "main", "title": "t"}, + "POST", + f"{P}/merge_requests", + {}, + {"source_branch": "f", "target_branch": "main", "title": "t"}, + ), + ( + "gitlab_update_mr", + {"project_id": "123", "mr_iid": 1, "title": "t2"}, + "PUT", + MR, + {}, + {"title": "t2"}, + ), + ( + "gitlab_merge_mr", + {"project_id": "123", "mr_iid": 1, "delete_source_branch": True}, + "PUT", + f"{MR}/merge", + {}, + {"should_remove_source_branch": True}, + ), + ( + "gitlab_merge_mr_sequence", + {"project_id": "123", "mr_iids": [7], "require_mergeable_status": False, "squash": True}, + "PUT", + f"{P}/merge_requests/7/merge", + {}, + {"squash": True}, + ), + ( + "gitlab_rebase_mr", + {"project_id": "123", "mr_iid": 1}, + "PUT", + f"{MR}/rebase", + {}, + {"skip_ci": False}, + ), + ("gitlab_mr_changes", {"project_id": "123", "mr_iid": 1}, "GET", f"{MR}/changes", {}, None), + # MR notes + ( + "gitlab_list_mr_notes", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/notes", + {"per_page": "100", "page": "1"}, + None, + ), + ( + "gitlab_add_mr_note", + {"project_id": "123", "mr_iid": 1, "body": "hi", "internal": True}, + "POST", + f"{MR}/notes", + {}, + {"body": "hi", "internal": True}, + ), + ( + "gitlab_delete_mr_note", + {"project_id": "123", "mr_iid": 1, "note_id": 5}, + "DELETE", + f"{MR}/notes/5", + {}, + None, + ), + ( + "gitlab_update_mr_note", + {"project_id": "123", "mr_iid": 1, "note_id": 5, "body": "b"}, + "PUT", + f"{MR}/notes/5", + {}, + {"body": "b"}, + ), + ( + "gitlab_award_emoji", + {"project_id": "123", "mr_iid": 1, "note_id": 5, "emoji": "thumbsup"}, + "POST", + f"{MR}/notes/5/award_emoji", + {}, + {"name": "thumbsup"}, + ), + ( + "gitlab_remove_emoji", + {"project_id": "123", "mr_iid": 1, "note_id": 5, "award_id": 9}, + "DELETE", + f"{MR}/notes/5/award_emoji/9", + {}, + None, + ), + # MR discussions + ( + "gitlab_list_mr_discussions", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/discussions", + {"per_page": "100", "page": "1"}, + None, + ), + ( + "gitlab_create_mr_discussion", + {"project_id": "123", "mr_iid": 1, "body": "nit"}, + "POST", + f"{MR}/discussions", + {}, + {"body": "nit"}, + ), + ( + "gitlab_reply_to_discussion", + {"project_id": "123", "mr_iid": 1, "discussion_id": "abc", "body": "r"}, + "POST", + f"{MR}/discussions/abc/notes", + {}, + {"body": "r"}, + ), + ( + "gitlab_resolve_discussion", + {"project_id": "123", "mr_iid": 1, "discussion_id": "abc", "resolved": True}, + "PUT", + f"{MR}/discussions/abc", + {}, + {"resolved": True}, + ), + # MR approvals and metadata + ( + "gitlab_approve_mr", + {"project_id": "123", "mr_iid": 1, "sha": "abc"}, + "POST", + f"{MR}/approve", + {}, + {"sha": "abc"}, + ), + ( + "gitlab_unapprove_mr", + {"project_id": "123", "mr_iid": 1}, + "POST", + f"{MR}/unapprove", + {}, + None, + ), + ( + "gitlab_get_mr_approvals", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/approvals", + {}, + None, + ), + ( + "gitlab_list_mr_pipelines", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/pipelines", + {"page": "1"}, + None, + ), + ( + "gitlab_list_mr_commits", + {"project_id": "123", "mr_iid": 1}, + "GET", + f"{MR}/commits", + {"page": "1"}, + None, + ), + ( + "gitlab_subscribe_mr", + {"project_id": "123", "mr_iid": 1}, + "POST", + f"{MR}/subscribe", + {}, + None, + ), + ( + "gitlab_unsubscribe_mr", + {"project_id": "123", "mr_iid": 1}, + "POST", + f"{MR}/unsubscribe", + {}, + None, + ), + # Pipelines + ( + "gitlab_list_pipelines", + {"project_id": "123", "status": "failed"}, + "GET", + f"{P}/pipelines", + {"per_page": "20", "status": "failed", "page": "1"}, + None, + ), + ( + "gitlab_get_pipeline", + {"project_id": "123", "pipeline_id": 100}, + "GET", + f"{P}/pipelines/100", + {}, + None, + ), + ( + "gitlab_create_pipeline", + {"project_id": "123", "ref": "main", "variables": VARS}, + "POST", + f"{P}/pipeline", + {}, + {"ref": "main", "variables": VARS}, + ), + ( + "gitlab_retry_pipeline", + {"project_id": "123", "pipeline_id": 100}, + "POST", + f"{P}/pipelines/100/retry", + {}, + None, + ), + ( + "gitlab_cancel_pipeline", + {"project_id": "123", "pipeline_id": 100}, + "POST", + f"{P}/pipelines/100/cancel", + {}, + None, + ), + # Jobs + ("gitlab_retry_job", {"project_id": "123", "job_id": 7}, "POST", f"{P}/jobs/7/retry", {}, None), + ( + "gitlab_play_job", + {"project_id": "123", "job_id": 7, "variables": VARS}, + "POST", + f"{P}/jobs/7/play", + {}, + {"job_variables_attributes": VARS}, + ), + ( + "gitlab_cancel_job", + {"project_id": "123", "job_id": 7}, + "POST", + f"{P}/jobs/7/cancel", + {}, + None, + ), + ( + "gitlab_get_job_log", + {"project_id": "123", "job_id": 7}, + "GET", + f"{P}/jobs/7/trace", + {}, + None, + ), + # Tags + ( + "gitlab_list_tags", + {"project_id": "123", "order_by": "version"}, + "GET", + f"{P}/repository/tags", + {"per_page": "20", "order_by": "version", "page": "1"}, + None, + ), + ( + "gitlab_get_tag", + {"project_id": "123", "tag_name": "v1.0"}, + "GET", + f"{P}/repository/tags/v1.0", + {}, + None, + ), + ( + "gitlab_create_tag", + {"project_id": "123", "tag_name": "v1.0", "ref": "main", "message": "rel"}, + "POST", + f"{P}/repository/tags", + {}, + {"tag_name": "v1.0", "ref": "main", "message": "rel"}, + ), + ( + "gitlab_delete_tag", + {"project_id": "123", "tag_name": "v1.0"}, + "DELETE", + f"{P}/repository/tags/v1.0", + {}, + None, + ), + # Releases + ( + "gitlab_list_releases", + {"project_id": "123"}, + "GET", + f"{P}/releases", + {"per_page": "20", "page": "1"}, + None, + ), + ( + "gitlab_get_release", + {"project_id": "123", "tag_name": "v1.0"}, + "GET", + f"{P}/releases/v1.0", + {}, + None, + ), + ( + "gitlab_create_release", + {"project_id": "123", "tag_name": "v1.0", "links": LINKS}, + "POST", + f"{P}/releases", + {}, + {"tag_name": "v1.0", "assets": {"links": LINKS}}, + ), + ( + "gitlab_update_release", + {"project_id": "123", "tag_name": "v1.0", "name": "n"}, + "PUT", + f"{P}/releases/v1.0", + {}, + {"name": "n"}, + ), + ( + "gitlab_delete_release", + {"project_id": "123", "tag_name": "v1.0"}, + "DELETE", + f"{P}/releases/v1.0", + {}, + None, + ), + # Project variables + ( + "gitlab_list_variables", + {"project_id": "123"}, + "GET", + f"{P}/variables", + {"per_page": "100", "page": "1"}, + None, + ), + ( + "gitlab_create_variable", + {"project_id": "123", "key": "K", "value": "V", "masked": True}, + "POST", + f"{P}/variables", + {}, + {"key": "K", "value": "V", "masked": True}, + ), + ( + "gitlab_update_variable", + {"project_id": "123", "key": "K", "value": "V", "environment_scope": "prod"}, + "PUT", + f"{P}/variables/K", + {"filter[environment_scope]": "prod"}, + {"value": "V"}, + ), + ( + "gitlab_delete_variable", + {"project_id": "123", "key": "K", "environment_scope": "prod"}, + "DELETE", + f"{P}/variables/K", + {"filter[environment_scope]": "prod"}, + None, + ), + # Group variables + ( + "gitlab_list_group_variables", + {"group_id": "10"}, + "GET", + f"{G}/variables", + {"per_page": "100", "page": "1"}, + None, + ), + ( + "gitlab_create_group_variable", + {"group_id": "10", "key": "K", "value": "V"}, + "POST", + f"{G}/variables", + {}, + {"key": "K", "value": "V"}, + ), + ( + "gitlab_update_group_variable", + {"group_id": "10", "key": "K", "value": "V", "protected": True}, + "PUT", + f"{G}/variables/K", + {}, + {"value": "V", "protected": True}, + ), + ( + "gitlab_delete_group_variable", + {"group_id": "10", "key": "K"}, + "DELETE", + f"{G}/variables/K", + {}, + None, + ), + # Issues + ( + "gitlab_list_issues", + {"project_id": "123", "assignee_id": 4}, + "GET", + f"{P}/issues", + {"per_page": "20", "assignee_id": "4", "page": "1"}, + None, + ), + ("gitlab_get_issue", {"project_id": "123", "issue_iid": 2}, "GET", f"{P}/issues/2", {}, None), + ( + "gitlab_create_issue", + {"project_id": "123", "title": "t", "confidential": True}, + "POST", + f"{P}/issues", + {}, + {"title": "t", "confidential": True}, + ), + ( + "gitlab_update_issue", + {"project_id": "123", "issue_iid": 2, "state_event": "close"}, + "PUT", + f"{P}/issues/2", + {}, + {"state_event": "close"}, + ), + ( + "gitlab_add_issue_comment", + {"project_id": "123", "issue_iid": 2, "body": "c"}, + "POST", + f"{P}/issues/2/notes", + {}, + {"body": "c"}, + ), +] + +# Keys _err sets for GitLabNotFoundError. gitlab_merge_mr_sequence adds merged_so_far on top. +NOT_FOUND_KEYS = {"error", "status_code", "body", "hint"} + + +def _parse(result) -> dict: + return json.loads(result.content[0].text) + + +@pytest.mark.parametrize("row", ROWS, ids=[r[0] for r in ROWS]) +async def test_request_shape(tool_client, row): + name, args, method, path, query, body = row + client, router = tool_client + payload: list | dict = [] if name.startswith("gitlab_list_") else {} + route = router.request(method, path).mock(return_value=Response(200, json=payload)) + + parsed = _parse(await client.call_tool(name, args)) + + assert "error" not in parsed + req = route.calls.last.request + assert req.method == method + assert req.url.raw_path.split(b"?")[0].decode() == f"/api/v4{path}" + assert dict(req.url.params) == query + assert (json.loads(req.content) if req.content else None) == body + + +@pytest.mark.parametrize("row", ROWS, ids=[r[0] for r in ROWS]) +async def test_forced_failure(tool_client, row): + name, args, method, path, _query, _body = row + client, router = tool_client + router.request(method, path).mock(return_value=Response(404, json={"message": "404 Not Found"})) + + parsed = _parse(await client.call_tool(name, args)) + + assert NOT_FOUND_KEYS <= parsed.keys() + assert parsed["status_code"] == 404 + + +async def test_every_tool_has_a_row(tool_client): + client, _router = tool_client + assert {r[0] for r in ROWS} == {t.name for t in await client.list_tools()} diff --git a/tests/unit/test_tools.py b/tests/unit/test_tools.py index c256b86..ff9da35 100644 --- a/tests/unit/test_tools.py +++ b/tests/unit/test_tools.py @@ -3,61 +3,11 @@ from __future__ import annotations import json -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager from typing import Any import pytest -import respx -from fastmcp import Client, FastMCP from httpx import Response -from mcp_gitlab.client import GitLabClient -from mcp_gitlab.config import GitLabConfig - -TEST_URL = "https://gitlab.example.com" -TEST_TOKEN = "test-token" - - -def _make_mcp(*, read_only: bool = False) -> FastMCP: - """Build a FastMCP server with mocked lifespan.""" - config = GitLabConfig(url=TEST_URL, token=TEST_TOKEN, read_only=read_only) - client = GitLabClient(config) - - @asynccontextmanager - async def mock_lifespan(server: FastMCP) -> AsyncIterator[dict[str, Any]]: - try: - yield {"client": client, "config": config} - finally: - await client.close() - - # Import the real mcp instance and swap lifespan - from mcp_gitlab.servers.gitlab import mcp - - original_lifespan = mcp._lifespan - mcp._lifespan = mock_lifespan - return mcp, original_lifespan - - -@pytest.fixture -async def tool_client(): - """FastMCP test client with mocked lifespan and respx-mocked HTTP.""" - mcp, original_lifespan = _make_mcp() - with respx.mock(base_url=f"{TEST_URL}/api/v4") as router: - async with Client(mcp) as client: - yield client, router - mcp._lifespan = original_lifespan - - -@pytest.fixture -async def readonly_client(): - """FastMCP test client in read-only mode.""" - mcp, original_lifespan = _make_mcp(read_only=True) - with respx.mock(base_url=f"{TEST_URL}/api/v4") as router: - async with Client(mcp) as client: - yield client, router - mcp._lifespan = original_lifespan - def _parse(result: Any) -> dict | list: """Extract JSON from a tool call result."""