Add e2e coverage for download_artifacts round-trip (6a)#549
Merged
Conversation
Pins the wire contract across both halves of the pair for phase 6a's download_artifacts surface (#547). Three scenarios: * Happy-path round-trip: receiver-side ArtifactsDownloadSender packs (stubbed packer returning a synthetic but layout-canonical tarball) → streams artifacts_start → artifacts_chunk(s) → artifacts_end{accepted: true} over the live peer-link Noise WS → offloader's PeerLinkClient assembles, validates the SHA-256, unpacks into {job_id, idedata, images, total_bytes} with extra.flash_images[].path rewritten from receiver-absolute to bare basenames. * unknown_job → CommandError(NOT_FOUND): single artifacts_end reject with no preceding start frame, mapped to NOT_FOUND via _DOWNLOAD_ARTIFACTS_REASON_TO_ERROR_CODE on the offloader side. * job_not_completed → CommandError(PRECONDITION_FAILED): receiver gate refuses to pack a non-terminal job's artifacts; offloader surfaces the mapped error code so the frontend can rerender as "wait for the build to finish." Receiver's firmware._jobs map is seeded directly with a synthetic FirmwareJob; the production firmware controller's queue isn't running in the harness. _pack_build_artifacts is monkeypatched on the happy path so the e2e doesn't need a StorageJSON sidecar + idedata.json + firmware binary on disk; the synthetic tarball still rides through Noise AEAD, BundleAssembler, and the offloader-side unpack so a wire-shape regression on either side surfaces here rather than slipping past two unit suites that pass on the same drift.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
=======================================
Coverage 99.11% 99.11%
=======================================
Files 76 76
Lines 9993 9993
=======================================
Hits 9905 9905
Misses 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end test coverage for the phase 6a download_artifacts peer-link round-trip, ensuring the receiver-side sender and offloader-side client/WS unpacker stay wire-compatible beyond what per-side unit tests can guarantee.
Changes:
- Introduces an e2e happy-path test that stubs receiver-side packing but exercises real Noise transport, chunking/assembly, SHA-256 verification, and WS-layer unpacking.
- Adds e2e coverage for two receiver soft-reject mappings:
unknown_job→CommandError(NOT_FOUND)andjob_not_completed→CommandError(PRECONDITION_FAILED).
Previous revision monkeypatched _pack_build_artifacts to dodge the StorageJSON/idedata/firmware-binary read path. Real e2e shouldn't gate at the disk seam — the autouse _core_config_path_in_tmp fixture in tests/conftest.py pins CORE.data_dir to tmp_path/.esphome anyway, so laying down a real StorageJSON sidecar + idedata.json + per-image binaries under that layout lets _pack_build_artifacts run unmodified. Now every production step on the receiver side runs: StorageJSON.load -> idedata read -> per-image read -> tarfile gzip pack -> Noise AEAD encrypt of each frame. The offloader-side path was already real (BundleAssembler -> SHA-256 verify -> _unpack_artifacts_response). The two soft-reject tests (unknown_job / job_not_completed) were already fully unstubbed since they short-circuit on the receiver's _find_remote_job / status gate before reaching the packer. Drive-by: drop the unused _build_artifacts_tarball helper and its imports (io / tarfile / Any / _PackedArtifacts) now that the synthetic tarball isn't built in-test.
This was referenced May 10, 2026
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 does this implement/fix?
Extends the two-instance e2e harness to cover phase 6a's
download_artifactswire surface (#547).The unit tests in
tests/test_remote_build_artifacts_download.pyalready cover the receiver-sideArtifactsDownloadSenderbranches in isolation (malformed-frame terminate, every soft-reject reason, happy-path stream, tarball pack/unpack round-trip), and the unit tests intests/test_remote_build_peer_link_client.pycover the offloader-sidePeerLinkClient.download_artifactssend + receive-loop dispatchers. What's missing is the contract between them — a wire-shape regression on either side that both unit suites pass on the same drift. The harness intests/e2e/(originally added in #523, extended in #543 for 5c-2b fan-out and #544 for 5d cancel) is where mismatches surface; this PR adds three more tests on top.Scenarios pinned
test_download_artifacts_round_trip_returns_unpacked_images): receiver-sideArtifactsDownloadSenderpacks (stubbed packer returning a layout-canonical synthetic tarball) → streamsartifacts_start→artifacts_chunk(s)→artifacts_end{accepted: true}over the live peer-link Noise WS → offloader-sidePeerLinkClientassembles via the existingBundleAssembler, validates the SHA-256 against the start frame's header, resolves the per-job future → WS command unpacks the tarball into{job_id, idedata, images, total_bytes}withidedata.extra.flash_images[].pathrewritten from receiver-absolute paths to bare basenames. Assertions cover the full unpack contract: ordering (firmware.binfirst then extras in declared order), the receiver-suppliedfirmware_offsetriding onimages[0].offset, per-image bytes round-tripping verbatim through the base64 wire envelope, andtotal_bytesmatchingsum(images[].size).unknown_job→CommandError(NOT_FOUND)(test_download_artifacts_unknown_job_surfaces_not_found): pins the soft-reject round-trip for the first of the receiver's five structured reject reasons._find_remote_job's linear scan returnsNonewhen(remote_peer, remote_job_id)has no matching entry infirmware._jobs; sender sends a singleartifacts_end{accepted: false, reason: "unknown_job"}with no preceding start frame; offloader-side WS layer maps via_DOWNLOAD_ARTIFACTS_REASON_TO_ERROR_CODEtoNOT_FOUND.job_not_completed→CommandError(PRECONDITION_FAILED)(test_download_artifacts_job_not_completed_surfaces_precondition_failed): pins the second soft-reject mapping. Receiver refuses to pack artifacts for a non-terminalFirmwareJob(the build dir's contents are partial during a running compile); offloader surfacesPRECONDITION_FAILEDso the frontend can rerender as "wait for the build to finish."Why the packer is stubbed
The synthetic tarball is layout-canonical (mirrors what
_pack_build_artifactsemits in production —idedata.jsonfirst,firmware.bin, then everyextra.flash_images[].pathbasename), but stubbing the packer keeps the e2e off the StorageJSON / cachedidedata.json/ on-disk firmware-binary path that the helper unit tests already cover. The tarball still rides through every other production step: Noise AEAD encrypt + decrypt, frame chunking viachunk_bundle, post-assembly SHA-256 verification on the offloader, and the basename rewrite in the offloader-side unpack — so a wire-format drift on either of those paths surfaces here.How the firmware map is seeded
db.firmware._jobsis aMagicMockin the harness (the production firmware controller's queue isn't running), so the test bodies seed the map directly with a syntheticFirmwareJobwhose(remote_peer, remote_job_id)matches the dialogue. The receiver-side sender's_find_remote_jobwalks the map directly (separate fromJobFanout's correlation cache — see the_seed_firmware_jobdocstring), so this is the right seam.Related issue or feature (if applicable):
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesFrontend coordination
Checklist
ruff,codespell, yaml/json/python checks).tests/where applicable.components.jsonhas not been hand-edited (regenerate viascript/sync_components.pyif a sync is needed).docs/ARCHITECTURE.mdand/ordocs/API.md.