🧪 [testing improvement] Add tests for _export_photo_manifest - #12488
🧪 [testing improvement] Add tests for _export_photo_manifest#12488undivisible wants to merge 4 commits into
Conversation
…port.py` Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Git-on-my-level
left a comment
There was a problem hiding this comment.
Verified locally in a worktree — the new coverage itself is solid; one mechanical blocker remains.
What's good (backend/tests/services/users/test_data_export.py):
- Each new test maps onto a real branch of
_export_photo_manifestinbackend/services/users/data_export.py: the missing-reference raise withrequire_bytes=True("retained image bytes reference is missing or malformed"), both malformed-inline paths (non-stringbase64, invalid base64), the valid-inline path (bytes_available/bytes_base64), and both storage paths throughdownload_frame_request_pixels— including the empty-payload raise ("retained image object is empty") andassert_called_once_with("uid1", "storage-1")argument checks. - All 6 new tests pass, and the full file is 39/39 green locally against the current production function. The
require_bytes=Falselegacy path was already covered bytest_legacy_conversation_photo_without_any_bytes_reference_exports_metadata, so there's no gap there.
Blocking: the Formatting check fails, and it's a one-line fix.
The repo pins Black 26.5.1 via scripts/backend-python-format (line-length 120). Running the identical check locally, the only reformat it wants is a second blank line before the first new test:
assert manifest["bytes_unavailable_reason"] == "no_retained_bytes_reference"
+
+def test_export_photo_manifest_require_bytes_missing_reference():(Two blank lines before a top-level def.) Adding that blank line — or running scripts/backend-python-format --write backend/tests/services/users/test_data_export.py — should turn the check green; nothing else in the added code needs reformatting.
Once formatting is fixed this looks ready — no further maintainer judgment needed beyond that.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
kodjima33
left a comment
There was a problem hiding this comment.
Approve only: test-coverage addition, not a bug fix — stays approve-only per policy.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f07426b6-c62f-46ca-bf65-18bcc5b0e867) |
|
Addressed Git-on-my-level Black CR: added the second blank line before |
…port.py` Failure-Class: none Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_dfcf3b39-c695-44fd-a19f-327fa40eefcf) |
|
Checked the new head ( Heads-up: your formatting fix from On the other two red checks: Hygiene and PR Metadata Preflight both fail only on by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Formatting CI fails because Black 26.5.1 requires two blank lines between top-level test functions. Jules' later push dropped the blank line added in bddb8f2; restore it so the check stays green. Co-authored-by: Max Carter 祁明思 <max@tsc.hk>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b725e525-5875-472f-9e59-db49688ec219) |
Dismissing: the only blocking concern (missing second blank line before test_export_photo_manifest_require_bytes_missing_reference, Black 26.5.1) was restored in b0a2199 — pinned formatting check and the full test file pass on the current head.
Git-on-my-level
left a comment
There was a problem hiding this comment.
Verified on the new head (b0a21991) — the last blocker from the earlier review is resolved.
What I checked (backend/tests/services/users/test_data_export.py, +75):
- All six new tests map onto real branches of
_export_photo_manifestinbackend/services/users/data_export.py: therequire_bytes=Truefail-closed raise ("retained image bytes reference is missing or malformed"), both malformed-inline paths (non-stringbase64, invalid base64), the valid-inlinebytes_available/bytes_base64passthrough, and bothdownload_frame_request_pixelsstorage paths — including the empty-payload raise ("retained image object is empty") and theassert_called_once_with("uid1", "storage-1")argument checks. Storage access is stubbed viamonkeypatch.setattron the module seam, so nothing reaches external services. - Local run: 39/39 tests in the file pass on this head against the current production function.
- Formatting: the second blank line before
test_export_photo_manifest_require_bytes_missing_reference— added inbddb8f2abf, dropped by the0941f722rebase — is back inb0a21991.scripts/backend-python-format --checkpasses locally, matching the green Formatting check. The earlier Hygiene/Preflight reds were thefirestore-query-coveragebaseline drift that also failed onmain, unrelated to this change.
Test-coverage addition only, no production changes. Approving, and dismissing the earlier changes-requested review since its formatting concern is fixed on this head. Thanks for the quick turnaround across the rebase churn.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
🎯 What: Added missing unit tests for the
_export_photo_manifestfunction inbackend/services/users/data_export.pyto ensure correct portability metadata generation.📊 Coverage: Covered edge cases for
require_bytes=Trueincluding missing references, malformed inline bytes, empty storage objects, and successful extraction of both inline bytes and storage payload.✨ Result: Improved test coverage and reliability for data export features.
PR created automatically by Jules for task 6564531978469913391 started by @undivisible
Note
Low Risk
Test-only changes with no runtime or export logic modifications.
Overview
Adds seven unit tests in
test_data_export.pyfor_export_photo_manifestwhenrequire_bytes=True(the frame-request / fail-closed path).They lock in
PortabilityExportIncompletefor missing byte references, malformed inlinebase64(wrong type or invalid encoding), and empty downloaded storage objects, withdownload_frame_request_pixelsmocked where needed. They also assert successful manifests for valid inline base64 and non-empty storage payloads (bytes_availableandbytes_base64).No production code changes — coverage and regression safety for portability photo manifest behavior only.
Reviewed by Cursor Bugbot for commit b0a2199. Configure here.