- 
                Notifications
    
You must be signed in to change notification settings  - Fork 375
 
Refactor 🔧: Add helpers & split Moonbase XCM tests #3367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
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) 🚨  | 
    
          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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4798 in fb5af73
| fn hrmp_init_accept_through_root() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn hrmp_close_works() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4884 in fb5af73
| fn hrmp_close_works() { | 
| use xcm_simulator::TestExt; | ||
| 
               | 
          ||
| #[test] | ||
| fn test_automatic_versioning_on_runtime_upgrade_with_relay() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2043 in fb5af73
| fn test_automatic_versioning_on_runtime_upgrade_with_relay() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn test_automatic_versioning_on_runtime_upgrade_with_para_b() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2169 in fb5af73
| fn test_automatic_versioning_on_runtime_upgrade_with_para_b() { | 
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2445 in fb5af73
| fn evm_account_receiving_assets_should_handle_sufficients_ref_count() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn empty_account_should_not_be_reset() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 2513 in fb5af73
| fn empty_account_should_not_be_reset() { | 
| use xcm_simulator::TestExt; | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_to_para_b() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_from_para_b_to_para_c() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 406 in fb5af73
| fn send_para_a_asset_from_para_b_to_para_c() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_to_para_b_and_back_to_para_a() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 520 in fb5af73
| fn send_para_a_asset_to_para_b_and_back_to_para_a() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_to_para_b_and_back_to_para_a_with_new_reanchoring() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 623 in fb5af73
| fn send_para_a_asset_to_para_b_and_back_to_para_a_with_new_reanchoring() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_to_para_b_with_trader() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 792 in fb5af73
| fn send_para_a_asset_to_para_b_with_trader() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_para_a_asset_to_para_b_with_trader_and_fee() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 867 in fb5af73
| fn send_para_a_asset_to_para_b_with_trader_and_fee() { | 
| 
               | 
          ||
| // Send a relay asset (like DOT) to a parachain A | ||
| #[test] | ||
| fn receive_relay_asset_from_relay() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 100 in fb5af73
| fn receive_relay_asset_from_relay() { | 
| 
               | 
          ||
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 148 in fb5af73
| fn send_relay_asset_to_relay() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_relay_asset_to_para_b() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 239 in fb5af73
| fn send_relay_asset_to_para_b() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_with_fee() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multiasset() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 3235 in fb5af73
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multiasset() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multicurrencies() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn send_dot_from_moonbeam_to_statemint_via_xtokens_transfer_multiassets() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| use xcm_simulator::TestExt; | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_derivative_multilocation() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 992 in fb5af73
| fn transact_through_derivative_multilocation() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_derivative_with_custom_fee_weight() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1160 in fb5af73
| fn transact_through_derivative_with_custom_fee_weight() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_derivative_with_custom_fee_weight_refund() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| use xcm_simulator::TestExt; | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 3901 in fb5af73
| fn transact_through_signed_multilocation() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_custom_fee_and_weight() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4008 in fb5af73
| fn transact_through_signed_multilocation_custom_fee_and_weight() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_custom_fee_and_weight_refund() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 4194 in fb5af73
| fn transact_through_signed_multilocation_para_to_para() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_refund() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum_no_proxy_fails() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_signed_multilocation_para_to_para_ethereum_proxy_succeeds() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| use xcm_simulator::{Encode, TestExt}; | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_sovereign() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1467 in fb5af73
| fn transact_through_sovereign() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_sovereign_fee_payer_none() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1633 in fb5af73
| fn transact_through_sovereign_fee_payer_none() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_sovereign_with_custom_fee_weight() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original
moonbeam/runtime/moonbase/tests/xcm_tests.rs
Line 1738 in fb5af73
| fn transact_through_sovereign_with_custom_fee_weight() { | 
| } | ||
| 
               | 
          ||
| #[test] | ||
| fn transact_through_sovereign_with_custom_fee_weight_refund() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left comments with links to the original tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?