Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move copy_dir_contents, setup_project into e2e/src/lib.rs so all future test files can share them without duplication. Add list_assets and generate_minimal_png helpers in the same place. Move hex and tempfile from dev-dependencies to dependencies since lib.rs now uses them at compile time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deploy an empty canister with one HTML file and one runtime-generated PNG (no binary committed); assert both keys are present and content types are correct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Run deploy twice with no file changes; assert the second run reports "up to date" and the canister asset list is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Modify one file's content and re-sync; assert the updated file's identity SHA256 changes on the canister while the untouched file's SHA256 stays the same. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove one file from the local dist directory and re-sync; assert the deleted key is absent from the canister while the remaining asset is still intact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a multi-dir fixture with two source directories (dist-a, dist-b) containing non-overlapping files. Test deploys from both dirs and asserts all keys appear on the canister with correct leading-slash paths. Also marks the basic sync E2E checklist item complete in TEST.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`initial_sync` duplicated `basic_deploy` and used a 60-line hand-rolled PNG generator solely to check content-type detection, which is purely extension-based and belongs in a unit test. Replace it with a `load_infers_png_mime` unit test in `assets-sync/src/content.rs`, delete `generate_minimal_png`, consolidate `basic.rs` into `sync.rs`, and update TEST.md accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds the first end-to-end Rust tests that exercise the full asset sync stack (icp CLI → plugin.wasm → live local replica canister), plus a small unit test to keep MIME inference coverage in the assets-sync layer.
Changes:
- Added e2e sync workflow tests (deploy, proxy deploy, no-op, content update, deletion, multi-dir).
- Refactored shared e2e helpers into
e2e/src/lib.rsand updated dependencies accordingly. - Added a unit test for PNG MIME inference and updated
TEST.mdto reflect the testing strategy.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| TEST.md | Updates test-plan documentation for the new e2e test setup and scenarios. |
| e2e/tests/sync.rs | Introduces the main set of e2e sync tests and assertions. |
| e2e/tests/fixture/multi-dir/icp.yaml | Adds a fixture config that syncs from two directories. |
| e2e/tests/fixture/multi-dir/dist-b/app.js | Adds a multi-dir fixture asset (JS). |
| e2e/tests/fixture/multi-dir/dist-a/page.html | Adds a multi-dir fixture asset (HTML). |
| e2e/tests/basic.rs | Removes the earlier/basic e2e test file (superseded by sync.rs). |
| e2e/src/lib.rs | Adds reusable fixture setup + list_assets helper used by integration tests. |
| e2e/Cargo.toml | Moves hex into normal dependencies (now used by the library helper). |
| assets-sync/src/content.rs | Adds a unit test for PNG MIME inference in Content::load(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Compare SHA256s and encodings (not just keys) after the second deploy, so the test catches content or encoding changes that leave the key list unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lwshang
changed the title
raymondk
approved these changes
May 8, 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.
Summary
Adds the first set of end-to-end tests for the asset sync pipeline, exercising the full stack:
plugin.wasmloaded byicp→ assets synced to a live canister on a local replica.Tests added (
e2e/tests/sync.rs)basic_deploy— deploys a fixture project and asserts/index.htmlis present in the canisterbasic_deploy_with_proxy— same, but goes through a proxy canister (--proxyflag)no_op_sync— runs deploy twice; second run must report "up to date" with no state changecontent_update— modifies a file and re-syncs; verifies the SHA256 on the canister changed for the updated file and is unchanged for untouched filesasset_deletion— removes a file locally and re-syncs; verifies the key is deleted from the canister while other keys survivemulti_directory_sync— configures two source directories; verifies files from both appear in the canisterRationale: e2e vs. unit tests
E2e tests in this project are intentionally narrow in scope. They exist only when
icp-climust be involved to wire upcanister.wasmandplugin.wasmtogether — things that cannot be observed in isolation. Everything else belongs in unit tests.A concrete example from this PR: the initial draft included an
initial_synctest that wrote a PNG file and assertedcontent_type == "image/png"on the canister. That assertion was removed from the e2e layer and replaced with aload_infers_png_mimeunit test inassets-sync/src/content.rs. MIME detection is a pure extension lookup inContent::load()— no canister, no CLI, no replica needed. Keeping it in e2e would have added replica startup overhead to test a one-liner inmime_guess.The rule of thumb: if the behaviour under test lives entirely inside
assets-sync(scan, MIME inference, diff logic, encoding), it gets a unit test. E2e tests cover only the seam between the CLI, the plugin, and the canister.Test plan
cargo test -p assets-syncpasses (includes newload_infers_png_mime)cargo test -p e2epasses (requiresicpon PATH anddfxor equivalent for local replica)🤖 Generated with Claude Code