Refactor 🔧: Add helpers & split Moonbase XCM tests #3367
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WASM runtime size check:Compared to target branchMoonbase runtime: 2348 KB (no changes) ✅ Moonbeam runtime: 2504 KB (no changes) 🚨 Moonriver runtime: 2496 KB (no changes) 🚨 Compared to latest release (runtime-3800)Moonbase runtime: 2348 KB (+304 KB compared to latest release) Moonbeam runtime: 2504 KB (+340 KB compared to latest release) 🚨 Moonriver runtime: 2496 KB (+332 KB compared to latest release) 🚨 |
pLabarta
added
B0-silent
Coverage Report@@ Coverage Diff @@
## master pablo/refactor-moonbase-xcm-tests +/- ##
=====================================================================
- Coverage 73.50% 73.34% -0.16%
+ Files 409 425 +16
- Lines 99362 98773 -589
=====================================================================
- Hits 73028 72439 -589
Misses 26334 26334
|
| use cumulus_primitives_core::relay_chain::HrmpChannelId; | ||
|
|
||
| #[test] | ||
| fn hrmp_init_accept_through_root() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4798 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn hrmp_close_works() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4884 in fb5af73
| use xcm_simulator::TestExt; | ||
|
|
||
| #[test] | ||
| fn test_automatic_versioning_on_runtime_upgrade_with_relay() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2043 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn test_automatic_versioning_on_runtime_upgrade_with_para_b() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2169 in fb5af73
| use crate::{xcm_mock::*, xcm_testing::helpers::*}; | ||
|
|
||
| #[test] | ||
| fn evm_account_receiving_assets_should_handle_sufficients_ref_count() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2445 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn empty_account_should_not_be_reset() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2513 in fb5af73
| use xcm_simulator::TestExt; | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_to_para_b() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_from_para_b_to_para_c() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 406 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_to_para_b_and_back_to_para_a() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 520 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_to_para_b_and_back_to_para_a_with_new_reanchoring() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 623 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_to_para_b_with_trader() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 792 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_para_a_asset_to_para_b_with_trader_and_fee() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 867 in fb5af73
|
|
||
| // Send a relay asset (like DOT) to a parachain A | ||
| #[test] | ||
| fn receive_relay_asset_from_relay() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 100 in fb5af73
|
|
||
| // Send relay asset (like DOT) back from Parachain A to relaychain | ||
| #[test] | ||
| fn send_relay_asset_to_relay() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 148 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_relay_asset_to_para_b() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 239 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_with_fee() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multiasset() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 3235 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multicurrencies() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multiassets() { |
There was a problem hiding this comment.
| use xcm_simulator::TestExt; | ||
|
|
||
| #[test] | ||
| fn transact_through_derivative_multilocation() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 992 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_derivative_with_custom_fee_weight() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1160 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_derivative_with_custom_fee_weight_refund() { |
There was a problem hiding this comment.
| use xcm_simulator::TestExt; | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 3901 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_custom_fee_and_weight() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4008 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_custom_fee_and_weight_refund() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4194 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_refund() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum_no_proxy_fails() { |
There was a problem hiding this comment.
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum_proxy_succeeds() { |
There was a problem hiding this comment.
| use xcm_simulator::{Encode, TestExt}; | ||
|
|
||
| #[test] | ||
| fn transact_through_sovereign() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1467 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_sovereign_fee_payer_none() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1633 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_sovereign_with_custom_fee_weight() { |
There was a problem hiding this comment.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1738 in fb5af73
| } | ||
|
|
||
| #[test] | ||
| fn transact_through_sovereign_with_custom_fee_weight_refund() { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Lets just refactor "test setup" things, like registration of assets, xcm version configuration, etc...
For functions that are only used once, and are not duplicated elsewhere, we should just move them to the specific test.
Over-refactoring testing code can lead to loss of clarity, harder debugging, and less maintainable tests. Unlike production code, tests should prioritize readability and explicitness over DRY (Don't Repeat Yourself). Over-abstraction in tests can make it difficult to understand what a test does at a glance or what it’s verifying when it fails.
| pub fn assert_asset_balance_para_b( | ||
| account: &[u8; 20], | ||
| asset_id: parachain::AssetId, | ||
| expected: u128, | ||
| ) { | ||
| use crate::xcm_mock::ParaB; | ||
| ParaB::execute_with(|| { | ||
| let account_id = parachain::AccountId::from(*account); | ||
| assert_eq!(Assets::balance(asset_id, &account_id), expected); | ||
| }); | ||
| } |
There was a problem hiding this comment.
This seems to only be used once and uses Assets, which is going to be removed. Every function that is only used once should either be moved to the specific test that uses it or we should update other tests to also use it.
What does it do?
What important points should reviewers know?
The test cases should remain the same, these are not meant to change in this PR.
Reviewers might need to compare current tests with the original ones at https://github.com/moonbeam-foundation/moonbeam/blob/fb5af73509cccc5ecef95de28896679b7cef11c5/runtime/moonbase/tests/xcm_tests.rs (current commit at
masterwhen writing this)I've commented on the new tests with links to the original ones.
Is there something left for follow-up PRs?
To make this PR smaller, only Moonbase tests were changed. Once merged, a new PR is needed to update Moonriver and Moonbeam tests.
What alternative implementations were considered?
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?