test: DEFI-2100: Migrate scenario-2 and scenario-3 e2e tests to PocketIC#525
Open
mbjorkqvist wants to merge 10 commits into
Open
test: DEFI-2100: Migrate scenario-2 and scenario-3 e2e tests to PocketIC#525mbjorkqvist wants to merge 10 commits into
mbjorkqvist wants to merge 10 commits into
Conversation
Adds a new e2e-tests/test-utils crate with the WASM loader, PocketIC bootstrap, generic candid call helpers, bitcoin-canister introspection (get_blockchain_info, get_stable_height, tick_until_*) and typed wrappers (bitcoin_get_balance, bitcoin_get_utxos, bitcoin_get_block_headers, bitcoin_send_transaction). Refactors scenario-1 to consume them; behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace e2e-tests/scenario-2.sh with a PocketIC-driven Rust integration test that reproduces the original script's assertions: blockchain height 4 after all source-canister blocks are ingested, ADDRESS balance of 40_000 satoshi (4 blocks * 10_000 txs * 1 sat), and the 1000-UTXO response cap. Extract the hard-coded address into a new scenario-2/src/lib.rs so the canister bin and the integration test share one definition (same pattern as scenario-1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace e2e-tests/scenario-3.sh with a PocketIC-driven Rust integration test
covering both branches of bitcoin_send_transaction:
- Happy path: send a valid (empty segwit-encoded) transaction and verify
the source canister received it via the inter-canister
bitcoin_send_transaction_internal call. The bitcoin canister's
send_transaction awaits that call, so by the time the update returns
LAST_TRANSACTION on the source is already updated.
- Error path: send malformed bytes and assert the rejection message
contains MalformedTransaction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrite the e2e-scenario-2 and e2e-scenario-3 CI jobs to mirror e2e-scenario-1: install PocketIC via the dfinity/pocketic action, build the source-canister WASM with scripts/build-canister.sh, and run cargo test against the package. Drop the dfinity/setup-dfx step and the now-unused cargo-build dependency. Exclude scenario-2 and scenario-3 from the cargo-tests matrix; like scenario-1 they now need the PocketIC binary and built WASM artifacts at runtime, which only the dedicated e2e jobs provide. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Continues the dfx→PocketIC migration (after #524) for scenario-2 and scenario-3 e2e tests, and extracts the shared PocketIC setup/helpers out of scenario-1 into a new e2e-test-utils crate so the remaining migrations become mechanical.
Changes:
- New
e2e-test-utilscrate centralizing WASM loading, PocketIC bootstrap, generic candid call helpers, bitcoin-canister introspection (blockchain info, stable height via/metrics, tick-until helpers), and typed wrappers for the bitcoin canister API. - Scenario-1's test refactored to consume
e2e-test-utils(behavior unchanged, file shrinks ~190 lines). Scenario-2 and scenario-3 ported from their.shscripts to Rust tests reproducing the same assertions; the scenario-2ADDRESSconstant moved into a newscenario-2/src/lib.rsshared by the bin and the test. - CI:
e2e-scenario-2ande2e-scenario-3jobs dropdfx/setup-dfx, install PocketIC, build the source canister WASM viascripts/build-canister.sh, and runcargo test -p scenario-{2,3};cargo-testsexcludes scenario-2/3; jobs no longer depend oncargo-build.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
e2e-tests/test-utils/Cargo.toml |
New crate manifest for shared e2e helpers. |
e2e-tests/test-utils/src/lib.rs |
Extracted setup, candid helpers, introspection, and typed BTC method wrappers. |
e2e-tests/scenario-1/Cargo.toml |
Switches dev-deps to e2e-test-utils, drops ic-btc-canister and serde_bytes. |
e2e-tests/scenario-1/tests/tests.rs |
Refactor to use shared helpers; behavior unchanged. |
e2e-tests/scenario-2/Cargo.toml |
Adds dev-deps for the new Rust e2e test. |
e2e-tests/scenario-2/src/lib.rs |
New library exposing the shared ADDRESS constant. |
e2e-tests/scenario-2/src/main.rs |
Imports ADDRESS from the new lib. |
e2e-tests/scenario-2/tests/tests.rs |
New Rust port of scenario-2.sh assertions. |
e2e-tests/scenario-2.sh |
Deleted (replaced by Rust test). |
e2e-tests/scenario-3/Cargo.toml |
Adds dev-deps for the new Rust e2e test. |
e2e-tests/scenario-3/tests/tests.rs |
New Rust port of scenario-3.sh assertions (valid + malformed tx paths). |
e2e-tests/scenario-3.sh |
Deleted (replaced by Rust test). |
Cargo.toml |
Registers the new e2e-tests/test-utils workspace member and path dep. |
Cargo.lock |
Updated dependency graph for the new crate and dev-deps. |
.github/workflows/workflow.yml |
Excludes scenario-2/3 from cargo-tests; rewrites the two e2e jobs to use PocketIC + cargo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
The e2e test-utils load_wasm helper previously spawned bash and ran scripts/build-canister.sh when the *_WASM_PATH env var was unset. That shell middleman is unnecessary and obscures the build invocation. Replace it with the same pattern used by ic-test-utilities-load-wasm and the sol-rpc-canister integration tests: drive cargo programmatically via escargot, with cargo_metadata locating the workspace target directory. Build into target/wasm-cargo-bin so the wasm build doesn't invalidate the native (test-binary) build cache. Also tighten CI: panic loudly if the env var is missing under CI rather than silently rebuilding inside the test process, which would mask a CI misconfiguration as a slow test run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The tests/tests.rs in both crates use only the typed helpers from e2e-test-utils and never reference pocket_ic directly. pocket-ic still gets pulled in transitively through e2e-test-utils, so the explicit dev-dependency was dead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The dfx-driven scenario-2.sh and scenario-3.sh scripts were deleted when the tests moved to PocketIC, so nothing references these canister definitions anymore. The e2e-scenario-1 entry stays because charge-cycles-on-reject.sh still uses it via dfx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three jobs were byte-for-byte identical except for the package name and the per-scenario WASM_PATH env var. Replacing them with a matrix over scenario-1/2/3 drops ~80 lines of YAML and makes future Rust / PocketIC version bumps one edit instead of three. fail-fast: false preserves the previous behavior where one scenario's failure didn't cancel the others. The checks-pass aggregator collapses correspondingly — a single needs: e2e-scenario covers all matrix instances. Note for the maintainer: if e2e-scenario-1 / e2e-scenario-2 / e2e-scenario-3 are listed as required status checks in branch protection, they'll need updating to the new matrix check names (e2e-scenario (scenario-1) etc.) or replaced with checks-pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gregorydemay
approved these changes
May 20, 2026
Contributor
gregorydemay
left a comment
gregorydemay
left a comment
There was a problem hiding this comment.
Thanks @mbjorkqvist !
| shell: bash | ||
| run: | | ||
| cargo test --release --all-targets --workspace --exclude benchmarks --exclude e2e-cdk-bitcoin-canister --exclude scenario-1 -- --color always | ||
| cargo test --release --all-targets --workspace --exclude benchmarks --exclude e2e-cdk-bitcoin-canister --exclude scenario-1 --exclude scenario-2 --exclude scenario-3 -- --color always |
Contributor
There was a problem hiding this comment.
nit: could we use a regex to exclude those end--to-end tests?
| needs: canister-build-reproducibility | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: |
Contributor
There was a problem hiding this comment.
nice simplification with the matrix 🥳
| tick_until_main_chain_height(&pic, btc_id, 5, 500); | ||
|
|
||
| let info = setup.get_blockchain_info(); | ||
| let info = get_blockchain_info(&pic, btc_id); |
Contributor
There was a problem hiding this comment.
nit: you could have kept the Setup struct in the test-utils libs and use it here. to avoid touching too much this file. This is mostly a stylistic choice (diff is small) so nothing to do, just thought worth mentioning.
Comment on lines
15
to
20
| "e2e-scenario-1": { | ||
| "type": "custom", | ||
| "candid": "../e2e-tests/scenario-1/candid.did", | ||
| "build": "../scripts/build-canister.sh scenario-1", | ||
| "wasm": "../target/wasm32-unknown-unknown/release/scenario-1.wasm.gz" | ||
| }, |
Contributor
There was a problem hiding this comment.
nit: shouldn't we also remove that block?
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.
Continue the dfx → PocketIC migration started in #524, this time covering
scenario-2andscenario-3, and extracting the reusable bits into a sharede2e-test-utilscrate so the remaining migrations are mechanical.Why. The same motivation as #524: shell tests are slow (full
dfx startper run), brittle (substring-matching Candid text), and excluded from typed checks. Scenario-1 proved the PocketIC pattern; this PR generalises it. After this lands, 7 bitcoin-canister shell scripts remain (set_config, post_upgrade_config, bitcoin-canister-metadata, cycles_burn, disable-api-…, upgradability, charge-cycles-on-reject), all of which can reuse the helpers introduced here.What changes.
e2e-tests/test-utils/crate (e2e-test-utils) collects the shared infrastructure extracted from scenario-1:load_wasm, the bitcoin-subnet PocketIC bootstrap, generic candid call helpers (query,update,update_raw), bitcoin-canister introspection (get_blockchain_info,get_stable_height,tick_until_main_chain_height,tick_until_stable_height) and typed wrappers (bitcoin_get_balance{_query},bitcoin_get_utxos{_query},bitcoin_get_block_headers,bitcoin_send_transaction).scenario-1/tests/tests.rsis refactored to consumee2e-test-utils. Behaviour is unchanged; the test still passes locally in ~8s. The file shrinks from 376 → 187 lines (the genesis-through-block-5 header blobs now dominate).scenario-2/tests/tests.rsreproduces every assertion from the deletedscenario-2.sh: blockchain height 4 after the 4 source-canister blocks are ingested, ADDRESS balance of 40_000 satoshi (4 × 10_000 × 1 sat), and the 1000-UTXO response cap. The hard-coded address moves into a newscenario-2/src/lib.rsso the canister bin and the test share one definition (same pattern as scenario-1).scenario-3/tests/tests.rscovers both branches ofbitcoin_send_transaction: a valid (empty segwit-encoded) transaction that reaches the source canister via the inter-canisterbitcoin_send_transaction_internalcall (verified by querying the source'sget_last_transaction), and a malformed-bytes path that must reject with a message containingMalformedTransaction. The bitcoin canister'ssend_transactionawaits the inter-canister call before returning, so no extra ticks are needed between the update call and the source-canister query.e2e-scenario-2ande2e-scenario-3CI jobs dropdfxanddfinity/setup-dfx, install PocketIC via thedfinity/pocketic@20c33db1aa87cc6ece50857ac632c37acf5e0322action, build the relevant source-canister WASM withscripts/build-canister.sh, and runcargo test --release -p scenario-{2,3}. WASM paths are passed viaIC_BTC_CANISTER_WASM_PATHandE2E_SCENARIO_{2,3}_WASM_PATH; theic-btc-canisterWASM still comes from thecanister-build-reproducibilityartifact. Both jobs lose theircargo-builddependency since cargo runs the test directly.scenario-2andscenario-3are excluded from the maincargo-testsmatrix (same reason as scenario-1: they need the PocketIC binary and the built WASM artifacts that only the dedicated e2e jobs produce).e2e-tests/utils.shis left in place — still sourced by the 7 remaining shell scripts. It can be deleted after the last shell script is gone.