test(ci): make document-upload tests hermetic, remove #210 quarantine#211
Conversation
The whole test_documents_routes module + test_graph_service::test_skips_self_edges were quarantined from CI under #210. Root cause: the document-upload legacy pipeline calls apply_graph_update -> db.connection.table() -> the single shared httpx client, escaping each test's per-route `table` mock and hitting the network (ConnectError in CI). Add an autouse conftest fixture that replaces db.connection._client with a benign stub returning empty responses, so no test can make a real Supabase call — services that bypass per-test mocks now no-op instead of connecting. Separately fix test_skips_self_edges, where a lazily created mock key (graph_edges, never accessed when the self-edge is skipped) raised KeyError and masked the real assertion. Remove the CI quarantine; all 22 document tests + the graph test now run and pass (gated suite: 614 passed).
|
Warning Review limit reached
More reviews will be available in 26 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
What
Restores real CI coverage for the document-upload pipeline by making its tests hermetic, then removes the #210 quarantine from
ci.yml.Why
Standing up CI (#162) revealed the entire
test_documents_routesmodule (22 tests) +test_graph_service::test_skips_self_edgeswere failing on a clean toolchain — never caught because there was no CI. Root cause: the upload legacy pipeline callsapply_graph_update→db.connection.table()→ the single sharedhttpx.Client, escaping each test's per-routetablemock and hitting the network.How
_hermetic_supabase_clientfixture replacesdb.connection._clientwith a stub returning benign empty responses — a shared safety net so no test can make a real Supabase call, regardless of which service bypasses per-test mocks. (Not per-test stubs.)mocks["graph_edges"]lookup intest_skips_self_edges— the factory creates table mocks lazily, so when the self-edge is correctly skippedgraph_edgesis never accessed and the bare lookup raisedKeyError, masking the realassert_not_called.--ignore=tests/test_documents_routes.pyand the--deselectfor the graph test. Only the heavy OCR integration tests remain excluded (manual evals workflow).Verified
Full gated suite with the quarantine removed: 614 passed (was 555 with 23 quarantined). All 22 document tests + the graph test genuinely run and PASS — not skipped, not xfail. No regression in the db-wrapper/storage tests (the new autouse mock is overridden by their own per-test mocks).
Closes #210