test: pipeline failure paths and upload queueing degradation - #173
Merged
Merged
Conversation
Refs #156. Layer 3/6 of the quality gate: the happy-path pipeline e2e tests already existed (test_pipeline_e2e.py) and were unblocked in the coverage-audit pass (#171) by committing the two fixture files they'd always needed. This covers the failure side of the same layer — what happens when ingestion cannot succeed. ## Pipeline-level failures (app/workers/pipeline.py::run) Four scenarios, verified against real parsing behaviour rather than assumed: - Unsupported extension (.exe) -> ERROR with a message. run() has its own extension check independent of the upload endpoint's MIME-type check — defence in depth, worth its own test. - Corrupt PDF (garbage bytes with a .pdf extension) -> pymupdf4llm raises FileDataError; the pipeline's outer exception handler must turn that into a terminal ERROR state. Confirmed by hand what pymupdf4llm actually does here before writing the assertion. - Empty (0-byte) PDF -> pymupdf4llm raises EmptyFileError, same handling. - A structurally valid PDF with no extractable text (a blank page) -> must reach READY with chunk_count == 0, NOT error and not stuck. This is the "document that parses but yields zero chunks" case the issue calls out by name. All four confirm the same property: pipeline.run()'s outer except Exception always reaches a terminal, inspectable state. A document is never left mid-flight by a pipeline-internal failure. ## API-level validation (app/api/documents_api.py) - Unsupported MIME type -> 415, and no CourseDocument row is created (the check runs before document_service.create_document). - Oversized file -> 413, same no-row guarantee. - A file of exactly 50 MB -> accepted. The size check is `> _MAX_UPLOAD_ BYTES`, so the boundary is inclusive; worth a test in its own right since an off-by-one here silently rejects legitimate uploads at exactly the documented limit. ## Upload queueing degradation The README documents a real, known symptom: "Document stuck in processing forever — Redis not running -> ARQ worker not started." That full scenario (a job sitting unconsumed in a live Redis queue) is infrastructure state, not a code path, and isn't reproducible in a unit/integration test. What *is* testable, and was previously untested, is the branch the symptom depends on: - No arq_pool on app.state (the default before an ARQ worker connects) -> falls back to background_tasks.add_task(pipeline.run, ...) rather than silently dropping the document. - arq_pool present -> enqueues through it rather than running in-process, so a production deployment with the queue configured doesn't silently stop using it. Documented explicitly in the module docstring that this covers the code path, not the full stuck-forever failure mode — the honest boundary of what this test file can and cannot claim. ## Numbers | | Before | After | |---|---|---| | Backend tests | 946 | 955 | | Coverage (branch) | 82.04% | 82.26% | mypy clean across 84 files. ## Remaining #156 scope, not done here - Password-protected PDF (not verified what pymupdf4llm actually raises; should be checked before writing an assertion rather than assumed) - PPTX/DOCX-specific failure equivalents (only PDF failure paths covered here; the happy path already covers all three formats) - Full docker compose up / health-endpoint-under-degradation checks — layer 6 territory (#159), not this layer
17 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #156 — partial. Progresses layer 3/6 of #146.
Scope
The happy-path e2e tests for this layer already existed in
test_pipeline_e2e.py, unblocked by #171's fixture work. This PR covers the failure side: what happens when ingestion cannot succeed, and whether the system degrades honestly.What's covered
Pipeline-level failures (
app/workers/pipeline.py::run) — four scenarios, each verified against actual parsing behaviour rather than assumed:ERRORwith a message (defence-in-depth check independent of the upload endpoint's own MIME-type check)pymupdf4llmraisesFileDataError; confirmed the outer exception handler turns it into a terminal error stateEmptyFileError, same handlingREADYwithchunk_count == 0, not error and not stuck — this is the "parses but yields zero chunks" case named explicitly in Test gate 3/6 · End-to-end pipeline and graceful degradation tests #156API-level validation (
app/api/documents_api.py) — unsupported MIME → 415 with noCourseDocumentrow created; oversized file → 413, same guarantee; a file at exactly the 50 MB limit → accepted (boundary check — the size comparison is>, so an off-by-one here would silently reject legitimate uploads).Upload queueing degradation — the README documents a real symptom: "Document stuck in processing forever — Redis not running → ARQ worker not started." The full scenario (a job sitting unconsumed in a live Redis queue) is infrastructure state, not reproducible in an integration test. What's testable, and was untested, is the branch that symptom depends on: no
arq_pool→ falls back toBackgroundTasks;arq_poolpresent → enqueues through it. Both directions matter — the second guards against a production deployment silently falling back to in-process execution.Numbers
mypy clean across 84 files.
Not done here — remaining #156 scope
pymupdf4llmactually raises for one; didn't want to assert against a guessdocker compose up/ health-under-degradation — that's layer 6 (Test gate 6/6 · Enforce the gate — branch protection, flakiness, speed #159) territory, not this layerLeft #156 open rather than closing it, since these remain.