Merge pull request #429 from christianlouis/copilot/fix-complete-status-error
fix(tests): align "completed" test fixtures with terminal-step guard semantics
This commit is contained in:
@@ -253,12 +253,13 @@ class TestStatusFilter:
|
||||
db_session.add(file_record)
|
||||
db_session.flush()
|
||||
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
for step_name in ("extract_text", "send_to_all_destinations"):
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name=step_name,
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
# Test completed filter
|
||||
|
||||
@@ -477,12 +477,13 @@ class TestFileQueriesDeduplicationEnabled:
|
||||
db_session.add(file1)
|
||||
db_session.flush()
|
||||
|
||||
step = FileProcessingStep(
|
||||
file_id=file1.id,
|
||||
step_name="check_for_duplicates",
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
for step_name in ("check_for_duplicates", "send_to_all_destinations"):
|
||||
step = FileProcessingStep(
|
||||
file_id=file1.id,
|
||||
step_name=step_name,
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
query = db_session.query(FileRecord)
|
||||
|
||||
+16
-14
@@ -187,13 +187,14 @@ class TestFileListingPagination:
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
|
||||
# Add a processing step (used for status determination)
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
# Add processing steps (used for status determination)
|
||||
for step_name in ("extract_text", "send_to_all_destinations"):
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name=step_name,
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
response = client.get("/api/files")
|
||||
@@ -287,13 +288,14 @@ class TestFileDetailEndpoint:
|
||||
assert response.status_code == 200
|
||||
assert response.json()["processing_status"]["status"] == "pending"
|
||||
|
||||
# Test 2: Success step = completed
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name="extract_text",
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
# Test 2: Success step including terminal step = completed
|
||||
for step_name in ("extract_text", "send_to_all_destinations"):
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
step_name=step_name,
|
||||
status="success",
|
||||
)
|
||||
db_session.add(step)
|
||||
db_session.commit()
|
||||
|
||||
response = client.get(f"/api/files/{file_record.id}")
|
||||
|
||||
@@ -76,7 +76,7 @@ def sample_files(db_session):
|
||||
)
|
||||
db_session.add(step3)
|
||||
|
||||
# File 4: completed (has success step, no failures)
|
||||
# File 4: completed (has success step including terminal step)
|
||||
file4 = FileRecord(
|
||||
filehash="hash4",
|
||||
original_filename="completed.pdf",
|
||||
@@ -87,10 +87,11 @@ def sample_files(db_session):
|
||||
db_session.add(file4)
|
||||
db_session.flush()
|
||||
|
||||
step4 = FileProcessingStep(file_id=file4.id, step_name="extract_text", status="success")
|
||||
db_session.add(step4)
|
||||
step4a = FileProcessingStep(file_id=file4.id, step_name="extract_text", status="success")
|
||||
step4b = FileProcessingStep(file_id=file4.id, step_name="send_to_all_destinations", status="success")
|
||||
db_session.add_all([step4a, step4b])
|
||||
|
||||
# File 5: completed with multiple success steps
|
||||
# File 5: completed with multiple success steps including terminal step
|
||||
file5 = FileRecord(
|
||||
filehash="hash5",
|
||||
original_filename="completed2.pdf",
|
||||
@@ -111,7 +112,12 @@ def sample_files(db_session):
|
||||
step_name="extract_metadata_with_gpt",
|
||||
status="success",
|
||||
)
|
||||
db_session.add_all([step5a, step5b])
|
||||
step5c = FileProcessingStep(
|
||||
file_id=file5.id,
|
||||
step_name="send_to_all_destinations",
|
||||
status="success",
|
||||
)
|
||||
db_session.add_all([step5a, step5b, step5c])
|
||||
|
||||
# File 6: has success but also failure (should be filtered out from completed)
|
||||
file6 = FileRecord(
|
||||
|
||||
@@ -437,7 +437,7 @@ class TestFileStatusMissingCoverage:
|
||||
assert result["has_errors"] is False
|
||||
|
||||
def test_get_files_processing_status_with_completed_steps(self, db_session):
|
||||
"""Covers line 189->188: completed + skipped == total_steps → completed status."""
|
||||
"""Covers completed + skipped == total_steps with terminal step → completed status."""
|
||||
from datetime import datetime
|
||||
from unittest.mock import patch
|
||||
|
||||
@@ -457,6 +457,7 @@ class TestFileStatusMissingCoverage:
|
||||
for step_name, step_status in [
|
||||
("create_file_record", "success"),
|
||||
("finalize_document_storage", "skipped"),
|
||||
("send_to_all_destinations", "success"),
|
||||
]:
|
||||
step = FileProcessingStep(
|
||||
file_id=file_record.id,
|
||||
|
||||
@@ -743,3 +743,102 @@ startxref
|
||||
# Verify that retry was triggered
|
||||
# The retry method raises a special exception
|
||||
assert exc_info.value is not None
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.requires_db
|
||||
def test_process_document_initializes_file_steps_for_new_file(db_session, tmp_path):
|
||||
"""
|
||||
Test that process_document calls initialize_file_steps for new file records
|
||||
so that all mandatory pipeline steps are pre-created as "pending".
|
||||
|
||||
This ensures status tracking reflects the complete expected pipeline from
|
||||
the start and prevents incomplete files from being falsely marked as
|
||||
"completed" just because the steps that *did* run all succeeded.
|
||||
"""
|
||||
# Create a test PDF file with embedded text
|
||||
test_pdf = tmp_path / "test.pdf"
|
||||
pdf_content = b"""%PDF-1.4
|
||||
1 0 obj
|
||||
<<
|
||||
/Type /Catalog
|
||||
/Pages 2 0 R
|
||||
>>
|
||||
endobj
|
||||
2 0 obj
|
||||
<<
|
||||
/Type /Pages
|
||||
/Kids [3 0 R]
|
||||
/Count 1
|
||||
>>
|
||||
endobj
|
||||
3 0 obj
|
||||
<<
|
||||
/Type /Page
|
||||
/Parent 2 0 R
|
||||
/MediaBox [0 0 612 792]
|
||||
/Resources <<
|
||||
/Font <<
|
||||
/F1 <<
|
||||
/Type /Font
|
||||
/Subtype /Type1
|
||||
/BaseFont /Helvetica
|
||||
>>
|
||||
>>
|
||||
>>
|
||||
/Contents 4 0 R
|
||||
>>
|
||||
endobj
|
||||
4 0 obj
|
||||
<<
|
||||
/Length 44
|
||||
>>
|
||||
stream
|
||||
BT
|
||||
/F1 12 Tf
|
||||
100 700 Td
|
||||
(Test content) Tj
|
||||
ET
|
||||
endstream
|
||||
endobj
|
||||
xref
|
||||
0 5
|
||||
0000000000 65535 f
|
||||
0000000009 00000 n
|
||||
0000000058 00000 n
|
||||
0000000115 00000 n
|
||||
0000000306 00000 n
|
||||
trailer
|
||||
<<
|
||||
/Size 5
|
||||
/Root 1 0 R
|
||||
>>
|
||||
startxref
|
||||
399
|
||||
%%EOF
|
||||
"""
|
||||
test_pdf.write_bytes(pdf_content)
|
||||
|
||||
with (
|
||||
patch("app.tasks.process_document.SessionLocal") as mock_session_local,
|
||||
patch("app.tasks.process_document.settings") as mock_settings,
|
||||
patch("app.tasks.process_document.log_task_progress"),
|
||||
patch("app.tasks.process_document.extract_metadata_with_gpt") as mock_extract,
|
||||
patch("app.tasks.process_document.initialize_file_steps") as mock_init_steps,
|
||||
):
|
||||
mock_settings.workdir = str(tmp_path)
|
||||
mock_settings.enable_deduplication = False
|
||||
mock_settings.enable_text_quality_check = False
|
||||
mock_session_local.return_value.__enter__.return_value = db_session
|
||||
mock_session_local.return_value.__exit__.return_value = None
|
||||
mock_extract.delay = MagicMock()
|
||||
|
||||
result = process_document.run(str(test_pdf))
|
||||
|
||||
assert result["status"] == "Text extracted locally"
|
||||
assert "file_id" in result
|
||||
|
||||
# initialize_file_steps must have been called exactly once with the new file's ID
|
||||
mock_init_steps.assert_called_once()
|
||||
called_file_id = mock_init_steps.call_args[0][1]
|
||||
assert called_file_id == result["file_id"]
|
||||
|
||||
Reference in New Issue
Block a user