feat(tests,staged_upload): empty-batch contract for StagedDestination Protocol + bug fix (Step 2e)#606
Merged
Conversation
… Protocol + bug fix (Step 2e) Closes out the empty-batch contract suite by adding tests/contracts/test_destination_staged_empty_batch.py covering the two destinations that use the StagedDestination Protocol (stage() + finalize() shape) — staged_upload and salesforce_bulk. Contracts under test: 1. isinstance(dest, StagedDestination) — Protocol satisfaction 2. stage([]) then finalize() returns SyncResult(0, 0, 0) 3. finalize() makes zero httpx.Client.send calls after only empty stage([]) call(s) — the load-bearing contract, since the engine calls finalize() regardless of whether any batch staged records The third contract surfaced a real bug in StagedUploadDestination: finalize() ran the full Phase 1 upload → Phase 2 trigger → Phase 3 poll lifecycle even when self._records was empty, POSTing a 0-byte file to stage.url and then POSTing to trigger.url — wasting a job-id allocation on a zero-row payload. SalesforceBulkDestination already had the right guard at the top of finalize() (`if not self._records: return SyncResult(rows_extracted=0)`); staged_upload was missing it. Fix: added the same empty-source short-circuit immediately after `record_count = len(self._records)`. No auth, upload, trigger, or poll work runs when nothing was staged. Tripwire mechanism is the same record-then-assert pattern as #604 / #605 — captures attempted httpx.Client.send / AsyncClient.send calls into a list rather than raising inside the patch, so broad except Exception row-error handlers in destinations can't swallow the AssertionError and mask the bug. 6 new tests (2 destinations × 3 contracts). Full contract suite now 74 tests, all passing. Empty-batch contract suite final tally: 24 of 24 registered destinations covered. - #593 HTTP webhook × 4 (slack/discord/teams/rest_api) - #594 file × 4 (csv/json/jsonl/parquet) - #595 SQL × 4 (postgres/mysql/clickhouse/snowflake) - #604 hardcoded-endpoint API × 11 (hubspot/jira/linear/notion/twilio/amplitude/zendesk/ google_ads/sendgrid/intercom/github_actions) - #605 special-transport × 2 (email_smtp/google_sheets) - this PR × 2 stage-shape (staged_upload/salesforce_bulk) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Closes out the empty-batch contract suite (#593 HTTP × 4 / #594 file × 4 / #595 SQL × 4 / #604 API × 11 / #605 special × 2). This PR adds the
StagedDestinationProtocol shape forstaged_upload+salesforce_bulk— and surfaces a real bug instaged_uploadthat the contract caught.Coverage now complete: 24 of 24 registered destinations
Bug surfaced + fixed in same PR:
staged_uploadempty short-circuitStagedUploadDestination.finalize()was running the full Phase 1 upload → Phase 2 trigger → Phase 3 poll lifecycle even whenself._recordswas empty:stage.urltrigger.urlwith the (empty) upload referenceSalesforceBulkDestination.finalize()already had the right guard (if not self._records: return SyncResult(rows_extracted=0)at the top).staged_uploadwas missing it.Fix: added the same short-circuit immediately after
record_count = len(self._records)inStagedUploadDestination.finalize(). The contract test now passes.Contract shape (StagedDestination Protocol)
The Protocol is
stage(records, config, opts) -> None+finalize(config, opts) -> SyncResult. Three contracts:isinstance(dest, StagedDestination)— Protocol satisfactionstage([])call(s),finalize()returnsSyncResult(0, 0, 0)finalize()makes zerohttpx.Client.sendcalls after only emptystage([])call(s)The third is load-bearing because the engine calls
finalize()regardless of whether any batch staged records.Tripwire mechanism (same as #604 / #605)
Records attempted
httpx.Client.send/AsyncClient.sendcalls into a captured list rather than raising inside the patch — broadexcept Exceptionrow-error handlers in destinations would swallow an AssertionError raised from the tripwire, masking the bug the contract is meant to catch.Test plan
pytest tests/contracts/test_destination_staged_empty_batch.py— 6 pass (after fix; staged_upload fails before the fix)pytest tests/contracts/— full suite 74 pass (68 from feat(tests): empty-batch contract for email_smtp + google_sheets (Step 2d) #605 + 6 new)pytest tests/unit/test_staged_upload.py tests/unit/test_salesforce_bulk.py— 15 existing pass (no regression from the staged_upload short-circuit addition)ruff check+mypy— clean🤖 Generated with Claude Code