fix: Add service containers and fix test failures

- Add Redis and RabbitMQ services to CI workflow
- Fix Jinja2 template error by passing file=None in error cases
- Fix test expecting dict response format for list_files endpoint
- Fix NOT NULL constraint by providing valid local_filename
- Fix retry-subtask to validate subtask name before checking processed file
- Add mock for process_document in reprocess test

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-02-08 08:14:09 +00:00
parent 4ca24a2e0e
commit 0a461343e7
7 changed files with 62 additions and 30 deletions
+20
View File
@@ -5,6 +5,26 @@ on: [push, pull_request]
jobs: jobs:
test: test:
runs-on: ubuntu-latest runs-on: ubuntu-latest
services:
redis:
image: redis:7
ports:
- 6379:6379
options: >-
--health-cmd "redis-cli ping"
--health-interval 10s
--health-timeout 5s
--health-retries 5
rabbitmq:
image: rabbitmq:3-management
ports:
- 5672:5672
- 15672:15672
options: >-
--health-cmd "rabbitmq-diagnostics -q ping"
--health-interval 10s
--health-timeout 5s
--health-retries 5
steps: steps:
- name: Checkout Code - name: Checkout Code
uses: actions/checkout@v3 uses: actions/checkout@v3
+24 -24
View File
@@ -437,30 +437,6 @@ def retry_subtask(
if not file_record: if not file_record:
raise HTTPException(status_code=404, detail=f"File with ID {file_id} not found") raise HTTPException(status_code=404, detail=f"File with ID {file_id} not found")
# Check for processed file (upload tasks work with processed files)
workdir = settings.workdir
processed_dir = os.path.join(workdir, "processed")
# Try to find the processed file
base_filename = os.path.splitext(file_record.original_filename)[0]
potential_paths = [
os.path.join(processed_dir, f"{file_record.filehash}.pdf"),
os.path.join(processed_dir, f"{base_filename}_processed.pdf"),
os.path.join(processed_dir, file_record.original_filename),
]
file_path = None
for path in potential_paths:
if os.path.exists(path):
file_path = path
break
if not file_path:
raise HTTPException(
status_code=400,
detail="Processed file not found. Cannot retry upload."
)
# Map subtask names to their corresponding Celery tasks # Map subtask names to their corresponding Celery tasks
from app.tasks.upload_to_dropbox import upload_to_dropbox from app.tasks.upload_to_dropbox import upload_to_dropbox
from app.tasks.upload_to_nextcloud import upload_to_nextcloud from app.tasks.upload_to_nextcloud import upload_to_nextcloud
@@ -492,6 +468,30 @@ def retry_subtask(
detail=f"Invalid subtask name: {subtask_name}. Must be one of: {', '.join(task_map.keys())}" detail=f"Invalid subtask name: {subtask_name}. Must be one of: {', '.join(task_map.keys())}"
) )
# Check for processed file (upload tasks work with processed files)
workdir = settings.workdir
processed_dir = os.path.join(workdir, "processed")
# Try to find the processed file
base_filename = os.path.splitext(file_record.original_filename)[0]
potential_paths = [
os.path.join(processed_dir, f"{file_record.filehash}.pdf"),
os.path.join(processed_dir, f"{base_filename}_processed.pdf"),
os.path.join(processed_dir, file_record.original_filename),
]
file_path = None
for path in potential_paths:
if os.path.exists(path):
file_path = path
break
if not file_path:
raise HTTPException(
status_code=400,
detail="Processed file not found. Cannot retry upload."
)
# Queue the specific upload task # Queue the specific upload task
upload_task = task_map[subtask_name] upload_task = task_map[subtask_name]
task = upload_task.delay(file_path, file_id) task = upload_task.delay(file_path, file_id)
+2
View File
@@ -170,6 +170,7 @@ def file_detail_page(request: Request, file_id: int, db: Session = Depends(get_d
if not file_record: if not file_record:
return templates.TemplateResponse("file_detail.html", { return templates.TemplateResponse("file_detail.html", {
"request": request, "request": request,
"file": None,
"error": f"File with ID {file_id} not found" "error": f"File with ID {file_id} not found"
}) })
@@ -216,6 +217,7 @@ def file_detail_page(request: Request, file_id: int, db: Session = Depends(get_d
logger.error(f"Error retrieving file details: {str(e)}") logger.error(f"Error retrieving file details: {str(e)}")
return templates.TemplateResponse("file_detail.html", { return templates.TemplateResponse("file_detail.html", {
"request": request, "request": request,
"file": None,
"error": str(e) "error": str(e)
}) })
+2
View File
@@ -466,6 +466,7 @@
cursor: not-allowed; cursor: not-allowed;
} }
</style> </style>
{% if file %}
<script> <script>
// JavaScript for handling retry functionality // JavaScript for handling retry functionality
async function reprocessFile() { async function reprocessFile() {
@@ -590,6 +591,7 @@
} }
} }
</script> </script>
{% endif %}
{% endblock %} {% endblock %}
{% block content %} {% block content %}
+5 -2
View File
@@ -68,8 +68,11 @@ class TestFileEndpoints:
response = client.get("/api/files") response = client.get("/api/files")
assert response.status_code == 200 assert response.status_code == 200
data = response.json() data = response.json()
assert isinstance(data, list) assert isinstance(data, dict)
assert len(data) == 0 assert "files" in data
assert "pagination" in data
assert isinstance(data["files"], list)
assert len(data["files"]) == 0
def test_get_nonexistent_file(self, client: TestClient): def test_get_nonexistent_file(self, client: TestClient):
"""Test getting a file that doesn't exist.""" """Test getting a file that doesn't exist."""
+1 -3
View File
@@ -147,9 +147,7 @@ class TestBulkOperations:
file_record = FileRecord( file_record = FileRecord(
filehash=f"hash{i}", filehash=f"hash{i}",
original_filename=f"test{i}.pdf", original_filename=f"test{i}.pdf",
local_filename=( local_filename=f"/tmp/test{i}.pdf", # Both files have local_filename
f"/tmp/test{i}.pdf" if i == 0 else None
), # Second file has no local file
file_size=1024, file_size=1024,
mime_type="application/pdf", mime_type="application/pdf",
) )
+8 -1
View File
@@ -3,6 +3,7 @@ Tests for file detail view improvements including reprocessing and preview endpo
""" """
import os import os
import pytest import pytest
from unittest.mock import patch, MagicMock
from fastapi.testclient import TestClient from fastapi.testclient import TestClient
from app.models import FileRecord, ProcessingLog from app.models import FileRecord, ProcessingLog
@@ -11,8 +12,14 @@ from app.models import FileRecord, ProcessingLog
class TestFileReprocessing: class TestFileReprocessing:
"""Tests for single file reprocessing endpoint.""" """Tests for single file reprocessing endpoint."""
def test_reprocess_existing_file(self, client: TestClient, db_session, sample_pdf_path): @patch("app.api.files.process_document")
def test_reprocess_existing_file(self, mock_process_document, client: TestClient, db_session, sample_pdf_path):
"""Test reprocessing an existing file.""" """Test reprocessing an existing file."""
# Setup mock
mock_task = MagicMock()
mock_task.id = "test-task-123"
mock_process_document.delay.return_value = mock_task
# Create a file record # Create a file record
file_record = FileRecord( file_record = FileRecord(
filehash="abc123", filehash="abc123",