Merge pull request #253 from christianlouis/copilot/fix-test-failures-bulk-operations
fix(test): align 11 tests with refactored status tracking and renamed APIs
This commit is contained in:
@@ -7,7 +7,7 @@ from unittest.mock import MagicMock, patch
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.models import FileRecord, ProcessingLog
|
||||
from app.models import FileProcessingStep, FileRecord, ProcessingLog
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@@ -178,7 +178,7 @@ class TestStatusFilter:
|
||||
def test_status_filter_pending(self, client: TestClient, db_session):
|
||||
"""Test filtering files by pending status."""
|
||||
# Create files with different statuses
|
||||
# File 1: Pending (no logs)
|
||||
# File 1: Pending (no processing steps)
|
||||
file1 = FileRecord(
|
||||
filehash="hash1",
|
||||
original_filename="pending.pdf",
|
||||
@@ -188,7 +188,7 @@ class TestStatusFilter:
|
||||
)
|
||||
db_session.add(file1)
|
||||
|
||||
# File 2: Processing (has in_progress log)
|
||||
# File 2: Processing (has in_progress step)
|
||||
file2 = FileRecord(
|
||||
filehash="hash2",
|
||||
original_filename="processing.pdf",
|
||||
@@ -199,14 +199,12 @@ class TestStatusFilter:
|
||||
db_session.add(file2)
|
||||
db_session.flush()
|
||||
|
||||
log2 = ProcessingLog(
|
||||
step2 = FileProcessingStep(
|
||||
file_id=file2.id,
|
||||
task_id="task2",
|
||||
step_name="OCR",
|
||||
step_name="extract_text",
|
||||
status="in_progress",
|
||||
message="Processing...",
|
||||
)
|
||||
db_session.add(log2)
|
||||
db_session.add(step2)
|
||||
db_session.commit()
|
||||
|
||||
# Test pending filter
|
||||
@@ -229,14 +227,12 @@ class TestStatusFilter:
|
||||
db_session.add(file_record)
|
||||
db_session.flush()
|
||||
|
||||
log = ProcessingLog(
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="task1",
|
||||
step_name="OCR",
|
||||
step_name="extract_text",
|
||||
status="in_progress",
|
||||
message="Processing...",
|
||||
)
|
||||
db_session.add(log)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
# Test processing filter
|
||||
@@ -257,14 +253,12 @@ class TestStatusFilter:
|
||||
db_session.add(file_record)
|
||||
db_session.flush()
|
||||
|
||||
log = ProcessingLog(
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="task1",
|
||||
step_name="OCR",
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
message="Completed",
|
||||
)
|
||||
db_session.add(log)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
# Test completed filter
|
||||
@@ -285,14 +279,13 @@ class TestStatusFilter:
|
||||
db_session.add(file_record)
|
||||
db_session.flush()
|
||||
|
||||
log = ProcessingLog(
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="task1",
|
||||
step_name="OCR",
|
||||
step_name="extract_text",
|
||||
status="failure",
|
||||
message="Failed",
|
||||
error_message="Failed",
|
||||
)
|
||||
db_session.add(log)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
# Test failed filter
|
||||
|
||||
@@ -514,7 +514,7 @@ class TestStepSummary:
|
||||
|
||||
now = datetime.now()
|
||||
logs = [
|
||||
MockLog("hash_file", "success", now - timedelta(minutes=5)),
|
||||
MockLog("check_text", "success", now - timedelta(minutes=5)),
|
||||
MockLog("create_file_record", "success", now - timedelta(minutes=4)),
|
||||
MockLog("extract_metadata_with_gpt", "failure", now - timedelta(minutes=3)),
|
||||
MockLog("upload_to_dropbox", "success", now - timedelta(minutes=2)),
|
||||
|
||||
+27
-21
@@ -3,7 +3,7 @@ Tests for file listing, pagination, filtering, and detail endpoints.
|
||||
"""
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from app.models import FileRecord, ProcessingLog
|
||||
from app.models import FileProcessingStep, FileRecord, ProcessingLog
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
@@ -186,15 +186,13 @@ class TestFileListingPagination:
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
|
||||
# Add a processing log
|
||||
log = ProcessingLog(
|
||||
# Add a processing step (used for status determination)
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="test_task",
|
||||
step_name="test_step",
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
message="Test message"
|
||||
)
|
||||
db_session.add(log)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
response = client.get("/api/files")
|
||||
@@ -223,7 +221,16 @@ class TestFileDetailEndpoint:
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
|
||||
# Add processing logs
|
||||
# Add processing steps (used for status determination)
|
||||
for step_name, status in [("extract_text", "success"), ("extract_metadata_with_gpt", "in_progress"), ("embed_metadata_into_pdf", "success")]:
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name=step_name,
|
||||
status=status,
|
||||
)
|
||||
db_session.add(step)
|
||||
|
||||
# Add processing logs (for the logs section)
|
||||
for i, status in enumerate(["success", "in_progress", "success"]):
|
||||
log = ProcessingLog(
|
||||
file_id=file_record.id,
|
||||
@@ -270,33 +277,32 @@ class TestFileDetailEndpoint:
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
|
||||
# Test 1: No logs = pending
|
||||
# Test 1: No steps = pending
|
||||
response = client.get(f"/api/files/{file_record.id}")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["processing_status"]["status"] == "pending"
|
||||
|
||||
# Test 2: Success log = completed
|
||||
log = ProcessingLog(
|
||||
# Test 2: Success step = completed
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="task_1",
|
||||
step_name="step_1",
|
||||
status="success"
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
)
|
||||
db_session.add(log)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
response = client.get(f"/api/files/{file_record.id}")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["processing_status"]["status"] == "completed"
|
||||
|
||||
# Test 3: Failure log = failed
|
||||
log2 = ProcessingLog(
|
||||
# Test 3: Failure step = failed
|
||||
step2 = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
task_id="task_2",
|
||||
step_name="step_2",
|
||||
status="failure"
|
||||
step_name="extract_metadata_with_gpt",
|
||||
status="failure",
|
||||
error_message="API error",
|
||||
)
|
||||
db_session.add(log2)
|
||||
db_session.add(step2)
|
||||
db_session.commit()
|
||||
|
||||
response = client.get(f"/api/files/{file_record.id}")
|
||||
|
||||
@@ -95,8 +95,7 @@ class TestEmbedMetadataPathTraversal:
|
||||
|
||||
def test_malicious_filename_in_metadata_is_sanitized(self, tmp_path):
|
||||
"""Test that malicious filenames from GPT metadata are sanitized."""
|
||||
from app.tasks.embed_metadata_into_pdf import unique_filepath
|
||||
from app.utils.filename_utils import sanitize_filename
|
||||
from app.utils.filename_utils import get_unique_filepath_with_counter, sanitize_filename
|
||||
|
||||
# Simulate malicious metadata from GPT
|
||||
malicious_filename = "../../etc/passwd"
|
||||
@@ -110,7 +109,7 @@ class TestEmbedMetadataPathTraversal:
|
||||
assert "\\" not in sanitized
|
||||
|
||||
# Verify unique_filepath with sanitized name stays in directory
|
||||
result = unique_filepath(str(tmp_path), sanitized, ".pdf")
|
||||
result = get_unique_filepath_with_counter(str(tmp_path), sanitized, ".pdf")
|
||||
result_path = Path(result)
|
||||
|
||||
# Ensure result is within tmp_path
|
||||
@@ -118,8 +117,7 @@ class TestEmbedMetadataPathTraversal:
|
||||
|
||||
def test_embed_metadata_validates_filename_field(self, tmp_path):
|
||||
"""Test that embed_metadata_into_pdf sanitizes the filename from metadata."""
|
||||
from app.tasks.embed_metadata_into_pdf import unique_filepath
|
||||
from app.utils.filename_utils import sanitize_filename
|
||||
from app.utils.filename_utils import get_unique_filepath_with_counter, sanitize_filename
|
||||
|
||||
# Test various malicious filenames
|
||||
malicious_filenames = [
|
||||
@@ -136,7 +134,7 @@ class TestEmbedMetadataPathTraversal:
|
||||
sanitized = sanitize_filename(malicious)
|
||||
|
||||
# Verify no path traversal is possible
|
||||
result = unique_filepath(str(tmp_path), sanitized, ".pdf")
|
||||
result = get_unique_filepath_with_counter(str(tmp_path), sanitized, ".pdf")
|
||||
result_path = Path(result)
|
||||
|
||||
# Result must be direct child of tmp_path
|
||||
|
||||
@@ -163,10 +163,14 @@ def test_process_document_duplicate_file(db_session, tmp_path):
|
||||
|
||||
# Verify that duplicate was detected
|
||||
assert result["status"] == "duplicate_file"
|
||||
assert result["file_id"] == existing_id
|
||||
assert result["original_file_id"] == existing_id
|
||||
|
||||
# Verify only one FileRecord exists
|
||||
assert db_session.query(FileRecord).count() == 1
|
||||
# A new duplicate FileRecord is created alongside the original
|
||||
assert db_session.query(FileRecord).count() == 2
|
||||
duplicate = db_session.query(FileRecord).filter(FileRecord.is_duplicate.is_(True)).first()
|
||||
assert duplicate is not None
|
||||
assert duplicate.duplicate_of_id == existing_id
|
||||
assert result["file_id"] == duplicate.id
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
|
||||
Reference in New Issue
Block a user