fix(testing): let the Phase 0A storage fake take the kwargs the real Blob takes - #12188
Merged
kodjima33 merged 1 commit intoAug 25, 2026
Merged
Conversation
…Blob takes `_LocalBlob` in the Phase 0A replay harness accepts only the positional argument on `upload_from_filename`, `upload_from_string` and `download_to_filename`, while the real `google.cloud.storage.Blob` accepts more — `content_type` among them. A caller that passes one dies inside the harness with a bare `TypeError`, surfacing three layers away as an HTTP 503 from the sync router with nothing pointing at the fake. The file's own comment calls itself a "copy of sync_cloud_tasks_stack pattern", and that original — backing the blocking gauntlet — already tolerates `*_args, **_kwargs` on all three. This copy dropped it; this restores it. No behaviour change. Verified by running the harness against a caller that passes `content_type`: BLOCKED on all three scenarios before, GREEN/BOUNDED after. With the caller reverted, the shipped diff leaves the harness GREEN/BOUNDED as it is on main today. Failure-Class: none
kodjima33
approved these changes
Aug 25, 2026
kodjima33
left a comment
Collaborator
There was a problem hiding this comment.
Confidence 5/5 (backend/testing): fixes the storage test-fake's method signatures to accept the same kwargs the real GCS Blob takes, matching an established pattern. Trivial, scoped, CI green.
abunet
added a commit
to abunet/omi
that referenced
this pull request
Aug 25, 2026
…nd it was ours to drop `fix/replay-harness-storage-fake-kwargs` (BasedHardware#12188) and the speaker-sample word count (BasedHardware#12169) were both merged upstream, so both files return to byte-identical with theirs — verified, `git diff upstream/main` is empty for each. The single conflict was our own marked workaround in replay_harness_phase0a/apps.py against their now-merged fix: took theirs, and the comment that said "once that is merged this file goes back to being byte-identical to theirs, and this comment with it" went with it. ADR-0030 audit, now including the Dart surface — the gap the +12 merge left, where `getTimeZone` came through unchanged while upstream updated its FCM sibling. The nine `check_oss_*` guards are 0, and our port markers are intact (118 `_object_store()`, 261 `_store()`, 152 `_signed_url`, 58 `get_auth_provider`, 53 `_vector_store()`, 48 `get_document_store`).
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
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.
What changed and why
_LocalBlobin the Phase 0A replay harness accepts only the positional argument onupload_from_filename,upload_from_stringanddownload_to_filename. The realgoogle.cloud.storage.Blobit stands in for accepts more —content_typeamong them — so a callerthat passes one dies inside the harness with a bare
TypeError, surfacing three layers away as anHTTP 503 from the sync router with nothing pointing at the fake:
The file's own comment (line 48) calls itself a "copy of sync_cloud_tasks_stack pattern, not
import". That original —
backend/testing/sync_cloud_tasks_stack/storage.py, backing theblocking gauntlet — already tolerates
*_args, **_kwargson all three methods. The copy droppedit. This restores it, and nothing else: three signatures, no behaviour change.
Closes #12186.
Product invariants affected
none
How it was verified
The defect is latent on
maintoday —upload_syncing_temporal_filecalls the blob with no keyword —so "revert the fix and watch a test go red" would prove nothing. Verified the other way, by adding a
caller that uses the real API (
content_type='application/octet-stream') and running the harness:npm run test:replay-harness-phase0a:emulatormain+ that caller, fake unfixedFEASIBILITY OUTCOME: BLOCKED(base, mutant-unguarded, mutant-guarded)main+ that caller, fake fixedFEASIBILITY OUTCOME: GREEN/BOUNDEDFEASIBILITY OUTCOME: GREEN/BOUNDEDSo the change is load-bearing for any caller that uses the documented API, and inert for
mainas itstands. Ran against the real Firestore emulator and a real
redis-server, the same way the CI jobdoes; the probe caller was reverted before committing and is not part of the diff.
python3 .github/scripts/run_checks.py --base upstream/main --head HEAD --lane cipasses.Tests
No test change. The fake lives in
backend/testing/replay_harness_phase0a/apps.py, which installs theegress guard and rebinds
google.authat import time — importing it from a unit test would pollutesys.modules, whichtests/unit/test_sys_modules_hermeticity.pyexists to prevent. The behaviouralcoverage is the harness run itself, shown above. Happy to add a test if you would rather the fake were
extracted into an importable module, but that seemed like more surface than this fix warrants.
Failure class (fixes)
Failure-Class: none