diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 377ec59..2e60627 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -57,23 +57,38 @@ jobs: # Use Tsinghua PyPI mirror for faster downloads in China pip install --cache-dir=$PIP_CACHE_DIR -i https://pypi.tuna.tsinghua.edu.cn/simple -r requirements.txt pip install --cache-dir=$PIP_CACHE_DIR -i https://pypi.tuna.tsinghua.edu.cn/simple -r requirements-dev.txt - pip install --cache-dir=$PIP_CACHE_DIR -i https://pypi.tuna.tsinghua.edu.cn/simple pytest-timeout + pip install --cache-dir=$PIP_CACHE_DIR -i https://pypi.tuna.tsinghua.edu.cn/simple pytest-timeout pytest-cov # Save to cache echo "Saving venv to cache $CACHE_KEY..." cp -a venv $CACHE_DIR/ fi - - name: Run unit tests + - name: Run tests with coverage run: | cd dashboard/backend . venv/bin/activate - pytest tests/unit/ -v --timeout=30 -x + pytest tests/ -v --timeout=30 \ + --cov=. --cov-report=xml --cov-report=term \ + --junit-xml=test-results.xml \ + --cov-fail-under=40 - - name: Run integration tests + - name: Upload coverage report + if: always() run: | cd dashboard/backend - . venv/bin/activate - pytest tests/integration/ -v --timeout=30 -x + if [ -f coverage.xml ]; then + echo "Coverage report generated" + cat coverage.xml + fi + + - name: Upload test results + if: always() + run: | + cd dashboard/backend + if [ -f test-results.xml ]; then + echo "Test results generated" + cat test-results.xml | head -50 + fi frontend-tests: name: Frontend Tests diff --git a/dashboard/backend/main.py b/dashboard/backend/main.py index 102eb3b..04bcda4 100644 --- a/dashboard/backend/main.py +++ b/dashboard/backend/main.py @@ -3,6 +3,7 @@ from fastapi.staticfiles import StaticFiles from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware from fastapi.responses import JSONResponse +from fastapi.exceptions import RequestValidationError from slowapi import Limiter from slowapi.util import get_remote_address from slowapi.errors import RateLimitExceeded @@ -17,6 +18,8 @@ import httpx import logging import time import jwt +import docker.errors +import traceback as tb from datetime import datetime, timezone, timedelta from config import TELEGRAM_BOT_TOKEN, TELEGRAM_CHAT_ID, CORS_ORIGINS, SECRET_KEY, ALGORITHM, VOLUME_ROOT, get_client_ip @@ -77,6 +80,47 @@ app.add_middleware(GZipMiddleware, minimum_size=1000) async def rate_limit_handler(request: Request, exc: RateLimitExceeded): return JSONResponse(status_code=429, content={"detail": "Too many requests, try again later"}) + +@app.exception_handler(RequestValidationError) +async def validation_exception_handler(request: Request, exc: RequestValidationError): + """Handle request validation errors with detailed information.""" + logging.error(f"Validation error on {request.method} {request.url.path}: {exc.errors()}") + return JSONResponse( + status_code=422, + content={"detail": exc.errors()} + ) + + +@app.exception_handler(docker.errors.NotFound) +async def docker_not_found_handler(request: Request, exc: docker.errors.NotFound): + """Handle Docker container/image not found errors.""" + logging.warning(f"Docker resource not found: {exc}") + return JSONResponse( + status_code=404, + content={"detail": "Docker resource not found"} + ) + + +@app.exception_handler(docker.errors.APIError) +async def docker_api_error_handler(request: Request, exc: docker.errors.APIError): + """Handle Docker API errors.""" + logging.error(f"Docker API error: {exc}") + return JSONResponse( + status_code=503, + content={"detail": "Docker service unavailable"} + ) + + +@app.exception_handler(Exception) +async def generic_exception_handler(request: Request, exc: Exception): + """Handle all unhandled exceptions.""" + logging.error(f"Unhandled exception on {request.method} {request.url.path}: {exc}") + logging.error(tb.format_exc()) + return JSONResponse( + status_code=500, + content={"detail": "Internal server error"} + ) + @app.middleware("http") async def security_headers(request: Request, call_next): response = await call_next(request) diff --git a/dashboard/backend/pyproject.toml b/dashboard/backend/pyproject.toml new file mode 100644 index 0000000..053abfd --- /dev/null +++ b/dashboard/backend/pyproject.toml @@ -0,0 +1,39 @@ +[tool.pytest.ini_options] +minversion = "7.0" +testpaths = ["tests"] +python_files = ["test_*.py"] +python_classes = ["Test*"] +python_functions = ["test_*"] +addopts = [ + "-v", + "--strict-markers", + "--tb=short", +] +asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" + +[tool.coverage.run] +source = ["."] +omit = [ + "*/tests/*", + "*/venv/*", + "*/__pycache__/*", + "*/site-packages/*", +] + +[tool.coverage.report] +precision = 2 +show_missing = true +skip_covered = false +exclude_lines = [ + "pragma: no cover", + "def __repr__", + "raise AssertionError", + "raise NotImplementedError", + "if __name__ == .__main__.:", + "if TYPE_CHECKING:", + "@abstractmethod", +] + +[tool.coverage.xml] +output = "coverage.xml" diff --git a/dashboard/backend/routers/docker_router.py b/dashboard/backend/routers/docker_router.py index 25fe88e..ccebcde 100644 --- a/dashboard/backend/routers/docker_router.py +++ b/dashboard/backend/routers/docker_router.py @@ -1,5 +1,5 @@ import docker -from fastapi import APIRouter, Depends, Query +from fastapi import APIRouter, Depends, Query, HTTPException from config import DOCKER_HOST from rbac import require_admin @@ -34,7 +34,7 @@ def list_containers(): @router.post("/containers/{container_id}/{action}", dependencies=[Depends(require_admin())]) def container_action(container_id: str, action: str): if action not in ("start", "stop", "restart"): - return {"error": "invalid action"} + raise HTTPException(status_code=400, detail="Invalid action. Must be one of: start, stop, restart") client = get_docker_client() c = client.containers.get(container_id) getattr(c, action)() diff --git a/dashboard/backend/routers/files.py b/dashboard/backend/routers/files.py index 89a2978..30e705e 100644 --- a/dashboard/backend/routers/files.py +++ b/dashboard/backend/routers/files.py @@ -18,6 +18,8 @@ _BASE_RESOLVED = str(BASE.resolve()) def _safe_path(path: str) -> Path: + # Strip leading slashes to prevent absolute path interpretation + path = path.lstrip("/") target = (BASE / path).resolve() if not str(target).startswith(_BASE_RESOLVED): raise ValueError("forbidden") @@ -75,7 +77,10 @@ def browse(path: str = ""): @router.get("/download") def download(path: str): try: - return FileResponse(_safe_path(path)) + target = _safe_path(path) + if not target.exists(): + raise HTTPException(status_code=404, detail="File not found") + return FileResponse(target) except ValueError: raise HTTPException(status_code=403, detail="Access denied") diff --git a/dashboard/backend/tests/conftest.py b/dashboard/backend/tests/conftest.py index 79635ee..38dc5a6 100644 --- a/dashboard/backend/tests/conftest.py +++ b/dashboard/backend/tests/conftest.py @@ -98,7 +98,11 @@ def expired_access_token(mock_config): @pytest.fixture def mock_docker_client(): - """Mock Docker client for testing.""" + """Mock Docker client for testing. + + Returns a mock client with a single running container. + For error scenarios, use mock_docker_client_with_errors. + """ client = Mock() # Mock container image @@ -126,6 +130,77 @@ def mock_docker_client(): return client +@pytest.fixture +def mock_docker_client_with_errors(): + """Mock Docker client that simulates error scenarios. + + Use this fixture to test error handling: + - Container not found (NotFound exception) + - Connection errors (APIError) + - Permission denied (APIError with 403) + + Example: + def test_container_not_found(test_app, admin_headers, mock_docker_client_with_errors): + # Configure the mock to raise NotFound + import docker.errors + mock_docker_client_with_errors.containers.get.side_effect = docker.errors.NotFound("Container not found") + """ + import docker.errors + + client = Mock() + + # By default, raise NotFound for get operations + client.containers.get.side_effect = docker.errors.NotFound("Container not found") + client.containers.list.return_value = [] + + return client + + +@pytest.fixture +def mock_docker_client_factory(): + """Factory for creating customizable Docker client mocks. + + Returns a function that creates mock clients with specific configurations. + + Example: + def test_custom_container(mock_docker_client_factory): + client = mock_docker_client_factory( + container_name="my-container", + container_status="stopped", + image_tag="my-image:v1.0" + ) + """ + def _create_mock(container_name="test-container", container_status="running", + image_tag="test-image:latest", container_id="abc123"): + client = Mock() + + # Mock container image + mock_image = Mock() + mock_image.tags = [image_tag] + mock_image.id = f"sha256:{container_id}def456" + + # Mock container + container = Mock() + container.id = container_id + container.short_id = container_id[:12] + container.name = container_name + container.status = container_status + container.image = mock_image + container.ports = {"80/tcp": [{"HostPort": "8080"}]} + container.attrs = { + "State": {"Health": {"Status": "healthy"}}, + "Config": {"Image": image_tag} + } + container.logs.return_value = b"test log line 1\ntest log line 2\n" + + client.containers.list.return_value = [container] + client.containers.get.return_value = container + + return client + + return _create_mock + + @pytest.fixture def mock_ssh_connection(): """Mock asyncssh connection for testing.""" @@ -153,6 +228,11 @@ def mock_httpx_client(): @pytest.fixture def test_app(mock_config, temp_auth_file, temp_rbac_file, mock_docker_client): """Create FastAPI test client with mocked dependencies.""" + # Reload routers to pick up new config values + import routers.files + import importlib + importlib.reload(routers.files) + # Patch Docker client before importing main with patch("docker.DockerClient", return_value=mock_docker_client): # Mock the limiter to disable rate limiting during tests diff --git a/dashboard/backend/tests/integration/test_docker.py b/dashboard/backend/tests/integration/test_docker.py index 370b5b9..ce053eb 100644 --- a/dashboard/backend/tests/integration/test_docker.py +++ b/dashboard/backend/tests/integration/test_docker.py @@ -79,9 +79,9 @@ class TestDockerContainerActions: headers=admin_headers ) - assert response.status_code == 200 + assert response.status_code == 400 data = response.json() - assert "error" in data + assert "detail" in data class TestDockerContainerLogs: diff --git a/dashboard/backend/tests/integration/test_files.py b/dashboard/backend/tests/integration/test_files.py index 2534887..b2cbbc3 100644 --- a/dashboard/backend/tests/integration/test_files.py +++ b/dashboard/backend/tests/integration/test_files.py @@ -18,7 +18,7 @@ class TestFileBrowse: assert response.status_code == 200 data = response.json() - assert "files" in data or isinstance(data, list) + assert "entries" in data or isinstance(data, list) def test_browse_without_auth(self, test_app): """Test browsing without authentication.""" @@ -61,7 +61,7 @@ class TestFileUpload: "/api/files/upload", headers=admin_headers, files=files, - data={"path": "/"} + params={"path": "/"} ) assert response.status_code in [200, 201] @@ -73,7 +73,7 @@ class TestFileUpload: response = test_app.post( "/api/files/upload", files=files, - data={"path": "/"} + params={"path": "/"} ) assert response.status_code == 401 @@ -94,7 +94,7 @@ class TestFileUpload: "/api/files/upload", headers=headers, files=files, - data={"path": "/"} + params={"path": "/"} ) # Upload should be admin-only diff --git a/dashboard/backend/tests/unit/test_auth.py b/dashboard/backend/tests/unit/test_auth.py index 2ae0179..1f05f4a 100644 --- a/dashboard/backend/tests/unit/test_auth.py +++ b/dashboard/backend/tests/unit/test_auth.py @@ -242,7 +242,6 @@ class TestTOTP: # No secret saved, should return True (2FA disabled) assert verify_totp("any_code") - @pytest.mark.skip(reason="Flaky test - file I/O timing issues in CI") def test_verify_totp_replay_protection(self, mock_config, temp_auth_file, sample_totp_secret): """Test that same TOTP code cannot be used twice.""" save_totp_secret(sample_totp_secret) @@ -277,7 +276,6 @@ class TestPasswordPersistence: loaded = load_password_hash() assert loaded == hashed - @pytest.mark.skip(reason="Test isolation issue - previous test state persists") def test_load_password_hash_from_env(self, mock_config, temp_auth_file): """Test loading password hash from environment variable.""" # Clear any hash in the file first @@ -288,8 +286,13 @@ class TestPasswordPersistence: with open(temp_auth_file, 'w') as f: json.dump(data, f) + # Reload auth_service to clear any cached data + import auth_service + import importlib + importlib.reload(auth_service) + # When no hash in file, should return env var - loaded = load_password_hash() + loaded = auth_service.load_password_hash() assert loaded == mock_config.ADMIN_PASSWORD_HASH