Collectives tests: Teleports and Whitelist Call #251
Conversation
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
…achain-for-asset-rate
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
…achain-for-asset-rate
| @@ -123,6 +123,184 @@ macro_rules! test_parachain_is_trusted_teleporter { | |||
| }; | |||
| } | |||
|
|
|||
| #[macro_export] | |||
| macro_rules! test_relay_is_trusted_teleporter { | |||
There was a problem hiding this comment.
In the analogy with test_parachain_is_trusted_teleporter macro from above.
There are two different ways now to test teleports, using the macro from above and using xcm-emulator::Test. I find the macro option more clear and easy to read compered to the Test.
I propose to rename the existing test_parachain_is_trusted_teleporter to test_sibling_is_trusted_teleporter, and have two additional test_relay_is_trusted_teleporter and test_parachain_is_trusted_teleporter
There was a problem hiding this comment.
Actually, iirc, the plan is/was to remove this crate from the fellows and re-use macros from polkadot-sdk to avoid unnecessary duplications.
I don't remember exactly what was missing in 1.7.0, but @acatangiu just temporary added it here.
See the comment https://github.com/polkadot-fellows/runtimes/pull/251/files#diff-7346706278168c97dfec9e73bec1a115274a2be89a3147b914dd2eedec9f6269R31-R32,
/// TODO: when bumping to polkadot-sdk v1.8.0,
/// remove this crate altogether and get the macros from `emulated-integration-tests-common`.
also I can see test_chain_can_claim_assets was added here also... @franciscoaguirre Cisco, do we need/want to move it to polkadot-sdk and setup those tests there or maybe do we have an issue for that?
There was a problem hiding this comment.
From what I see, test_parachain_is_trusted_teleporter and test_sibling_is_trusted_teleporter do almost exactly the same. Besides multi-receiver support, the only significant difference I can see is just asset construction: let assets: Assets = (Parent, $amount).into();. I like your multi-receiver support and assets as input parameters, it makes it much more generic.
So, I would vote here for keeping just one macro (temporarily, one nice generic macro here and port it asynchronously to polkadot-sdk for later removal from fellows) with multi-receiver support + the possibility to set assets as input parameters.
There was a problem hiding this comment.
Yes, I agree. But until we have it in polkadot-sdk we can keep this, and remove when we have macro to replace it.
There was a problem hiding this comment.
Yes, so at least TODO+issue maybe?
Well, I suggest to pimp that macro to its final version here, and then backport it to polkadot-sdk. This way, when bumping with polkadot-sdk later, we'll only need to just change imports without causing much disruption. However, please feel free to disregard this suggestion.
There was a problem hiding this comment.
I tried and I do not like the result. I have to remove some event asserts and leave only the ones which relevant for all cases. Also I have to pass some argument and resolve the xcm pallet's name from it, because they are different on Relay and Paras. Probably it can be still improved, but I think it's good enough for now. Having separate macro also leaves some flexibility for later changes.
integration-tests/emulated/tests/collectives/collectives-polkadot/src/tests/teleport.rs
Outdated
Show resolved
Hide resolved
integration-tests/emulated/tests/collectives/collectives-polkadot/src/tests/teleport.rs
Outdated
Show resolved
Hide resolved
integration-tests/emulated/tests/collectives/collectives-polkadot/src/tests/teleport.rs
Outdated
Show resolved
Hide resolved
|
Review required! Latest push from author must always be reviewed |
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
| use frame_support::{assert_ok, sp_runtime::traits::Dispatchable}; | ||
|
|
||
| #[test] | ||
| fn fellows_whitelist_call() { |
|
needs a clippy fix and a CHANGELOG entry and is good to go |
|
/merge |
|
Enabled Available commands
For more information see the documentation |
c2886b9
into
polkadot-fellows:main
Introduce teleport and whitelist call tests for Polkadot Collectives.
Based on #233