From 2d46959d019646ea1f289a01218660b5be2c8bde Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 27 Feb 2026 21:53:06 +0100 Subject: [PATCH] fix(test): fix 17 pre-existing unit test failures and add astrolabe CI build Unit test fixes: - test_userinfo_routes: patch nextcloud_httpx_client instead of httpx.AsyncClient - test_instrument_tool: patch trace_operation in metrics module (where imported) - test_management_app_password_endpoints: patch nextcloud_httpx_client and get_settings at correct import locations - test_management_status_endpoint: patch detect_auth_mode and get_settings at correct import locations (api.management, not config/config_validators) - test_token_exchange: fix TokenBrokerService constructor args (client_id/ client_secret instead of encryption_key) CI: - Add Node.js setup and astrolabe build step (composer + npm ci + npm run build) Co-Authored-By: Claude Opus 4.6 --- .github/workflows/test.yml | 12 +++++ pyproject.toml | 2 +- tests/server/auth/test_userinfo_routes.py | 4 +- tests/server/oauth/test_token_exchange.py | 6 +-- tests/unit/test_instrument_tool.py | 2 +- .../test_management_app_password_endpoints.py | 28 ++++++------ tests/unit/test_management_status_endpoint.py | 45 +++++++++++-------- 7 files changed, 59 insertions(+), 40 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8cc02f6..87cd3d3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,6 +72,18 @@ jobs: cd third_party/oidc composer install --no-dev + - name: Set up Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: 22 + + - name: Build Astrolabe app + run: | + cd third_party/astrolabe + composer install --no-dev --optimize-autoloader + npm ci + npm run build + # Start services with the appropriate profile - name: Run docker compose uses: hoverkraft-tech/compose-action@4894d2492015c1774ee5a13a95b1072093087ec3 # v2.5.0 diff --git a/pyproject.toml b/pyproject.toml index 47f387c..ac6f4d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,7 +64,7 @@ Changelog = "https://github.com/cbcoutinho/nextcloud-mcp-server/blob/master/CHAN [tool.pytest.ini_options] anyio_mode = "auto" -addopts = "--headed -p no:asyncio" # Disable pytest-asyncio plugin, use only anyio +addopts = "-p no:asyncio" # Disable pytest-asyncio plugin, use only anyio log_cli = 1 log_cli_level = "ERROR" log_level = "ERROR" diff --git a/tests/server/auth/test_userinfo_routes.py b/tests/server/auth/test_userinfo_routes.py index a2ad4a9..0e5c9cd 100644 --- a/tests/server/auth/test_userinfo_routes.py +++ b/tests/server/auth/test_userinfo_routes.py @@ -34,7 +34,7 @@ async def test_query_idp_userinfo_success(mocker): mock_client.__aexit__.return_value = None mocker.patch( - "nextcloud_mcp_server.auth.userinfo_routes.httpx.AsyncClient", + "nextcloud_mcp_server.auth.userinfo_routes.nextcloud_httpx_client", return_value=mock_client, ) @@ -59,7 +59,7 @@ async def test_query_idp_userinfo_failure(mocker): mock_client.__aexit__.return_value = None mocker.patch( - "nextcloud_mcp_server.auth.userinfo_routes.httpx.AsyncClient", + "nextcloud_mcp_server.auth.userinfo_routes.nextcloud_httpx_client", return_value=mock_client, ) diff --git a/tests/server/oauth/test_token_exchange.py b/tests/server/oauth/test_token_exchange.py index bf3017f..8f1704e 100644 --- a/tests/server/oauth/test_token_exchange.py +++ b/tests/server/oauth/test_token_exchange.py @@ -61,14 +61,12 @@ async def token_exchange_service(token_storage): @pytest.fixture async def token_broker(token_storage): """Create test token broker service.""" - # Use the same encryption key as storage - encryption_key = token_storage._test_encryption_key - broker = TokenBrokerService( storage=token_storage, oidc_discovery_url="http://test-idp/.well-known/openid-configuration", nextcloud_host="http://test-nextcloud", - encryption_key=encryption_key, + client_id="test-client", + client_secret="test-secret", cache_ttl=300, cache_early_refresh=30, ) diff --git a/tests/unit/test_instrument_tool.py b/tests/unit/test_instrument_tool.py index d7abb1a..bd30b3a 100644 --- a/tests/unit/test_instrument_tool.py +++ b/tests/unit/test_instrument_tool.py @@ -32,7 +32,7 @@ def mock_metrics(): def mock_tracer(): """Mock OpenTelemetry tracer.""" with patch( - "nextcloud_mcp_server.observability.tracing.trace_operation" + "nextcloud_mcp_server.observability.metrics.trace_operation" ) as mock_trace: # Configure mock to act as a context manager that allows exceptions to propagate mock_trace.return_value.__enter__ = MagicMock(return_value=None) diff --git a/tests/unit/test_management_app_password_endpoints.py b/tests/unit/test_management_app_password_endpoints.py index 8a83c8b..d80c967 100644 --- a/tests/unit/test_management_app_password_endpoints.py +++ b/tests/unit/test_management_app_password_endpoints.py @@ -184,7 +184,7 @@ async def test_provision_app_password_success(temp_storage, mocker): """Test successful app password provisioning.""" # Mock settings (imported locally in the function) mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -203,7 +203,7 @@ async def test_provision_app_password_success(temp_storage, mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -233,7 +233,7 @@ async def test_provision_app_password_success(temp_storage, mocker): async def test_provision_app_password_nextcloud_validation_fails(mocker): """Test that failed Nextcloud validation returns 401.""" mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -251,7 +251,7 @@ async def test_provision_app_password_nextcloud_validation_fails(mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -356,7 +356,7 @@ async def test_delete_app_password_success(temp_storage, mocker): # Mock settings (imported locally in the function) mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -374,7 +374,7 @@ async def test_delete_app_password_success(temp_storage, mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -404,7 +404,7 @@ async def test_delete_app_password_not_found(temp_storage, mocker): """Test deleting non-existent app password.""" # Mock settings (imported locally in the function) mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -422,7 +422,7 @@ async def test_delete_app_password_not_found(temp_storage, mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -447,7 +447,7 @@ async def test_delete_app_password_not_found(temp_storage, mocker): async def test_delete_app_password_invalid_credentials(mocker): """Test that invalid credentials returns 401 for deletion.""" mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -465,7 +465,7 @@ async def test_delete_app_password_invalid_credentials(mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -521,7 +521,7 @@ async def test_delete_app_password_username_mismatch(): async def test_provision_app_password_rate_limiting(mocker): """Test that rate limiting blocks excessive provisioning attempts.""" mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -539,7 +539,7 @@ async def test_provision_app_password_rate_limiting(mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) @@ -584,7 +584,7 @@ async def test_provision_app_password_rate_limiting(mocker): async def test_rate_limiting_is_per_user(mocker): """Test that rate limiting is applied per user, not globally.""" mocker.patch( - "nextcloud_mcp_server.config.get_settings", + "nextcloud_mcp_server.api.passwords.get_settings", return_value=MagicMock( nextcloud_host="http://localhost:8080", nextcloud_verify_ssl=True, @@ -602,7 +602,7 @@ async def test_rate_limiting_is_per_user(mocker): mock_client.__aexit__ = AsyncMock() mocker.patch( - "nextcloud_mcp_server.api.passwords.httpx.AsyncClient", + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", return_value=mock_client, ) diff --git a/tests/unit/test_management_status_endpoint.py b/tests/unit/test_management_status_endpoint.py index 11fff7e..ac6ad1a 100644 --- a/tests/unit/test_management_status_endpoint.py +++ b/tests/unit/test_management_status_endpoint.py @@ -70,10 +70,11 @@ class TestStatusEndpointOidcConfig: # get_settings and detect_auth_mode are imported inside the function with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.MULTI_USER_BASIC, ), ): @@ -107,10 +108,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.MULTI_USER_BASIC, ), ): @@ -135,10 +137,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.MULTI_USER_BASIC, ), ): @@ -167,10 +170,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.OAUTH_SINGLE_AUDIENCE, ), ): @@ -202,10 +206,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.SINGLE_USER_BASIC, ), ): @@ -235,10 +240,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.MULTI_USER_BASIC, ), ): @@ -267,10 +273,11 @@ class TestStatusEndpointOidcConfig: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.MULTI_USER_BASIC, ), ): @@ -295,10 +302,11 @@ class TestStatusEndpointBasicResponse: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.SINGLE_USER_BASIC, ), ): @@ -320,10 +328,11 @@ class TestStatusEndpointBasicResponse: with ( patch( - "nextcloud_mcp_server.config.get_settings", return_value=mock_settings + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, ), patch( - "nextcloud_mcp_server.config_validators.detect_auth_mode", + "nextcloud_mcp_server.api.management.detect_auth_mode", return_value=AuthMode.SINGLE_USER_BASIC, ), ):