From 622dff9ca705a6d3065c2d18a33cdab404b7830a Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere <gui.thiolliere@gmail.com> Date: Tue, 16 Jun 2020 13:10:10 +0200 Subject: [PATCH] Impl integrity test for runtime (#6356) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * impl integrity test for runtime * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/procedural/src/construct_runtime/mod.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * use thread local * update doc * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Gavin Wood <gavin@parity.io> --- substrate/Cargo.lock | 1 + .../procedural/src/construct_runtime/mod.rs | 17 +++ substrate/frame/support/src/dispatch.rs | 129 +++++++++++++++++- substrate/frame/support/src/traits.rs | 11 ++ substrate/frame/support/test/Cargo.toml | 2 + .../{decl_error.rs => construct_runtime.rs} | 20 ++- 6 files changed, 175 insertions(+), 5 deletions(-) rename substrate/frame/support/test/tests/{decl_error.rs => construct_runtime.rs} (88%) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 8948337d584..18290e7748c 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -1515,6 +1515,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-state-machine", + "sp-std", "trybuild", ] diff --git a/substrate/frame/support/procedural/src/construct_runtime/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/mod.rs index d7529cd272d..cac75490621 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/mod.rs @@ -89,6 +89,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream let outer_config = decl_outer_config(&name, modules.iter(), &scrate); let inherent = decl_outer_inherent(&block, &unchecked_extrinsic, modules.iter(), &scrate); let validate_unsigned = decl_validate_unsigned(&name, modules.iter(), &scrate); + let integrity_test = decl_integrity_test(&scrate); let res = quote!( #scrate_decl @@ -120,6 +121,8 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream #inherent #validate_unsigned + + #integrity_test ); Ok(res.into()) @@ -406,3 +409,17 @@ fn find_system_module<'a>( .find(|decl| decl.name == SYSTEM_MODULE_NAME) .map(|decl| &decl.module) } + +fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { + quote!( + #[cfg(test)] + mod __construct_runtime_integrity_test { + use super::*; + + #[test] + pub fn runtime_integrity_tests() { + <AllModules as #scrate::traits::IntegrityTest>::integrity_test(); + } + } + ) +} diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 70a552ce3a5..094cbce2634 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -269,8 +269,11 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// * `fn on_finalize() -> frame_support::weights::Weight` /// /// * `offchain_worker`: Executes at the beginning of a block and produces extrinsics for a future block -/// upon completion. Using this function will implement the -/// [`OffchainWorker`](./traits/trait.OffchainWorker.html) trait. +/// upon completion. Using this function will implement the +/// [`OffchainWorker`](./traits/trait.OffchainWorker.html) trait. +/// * `integrity_test`: Executes in a test generated by `construct_runtime`, note it doesn't +/// execute in an externalities-provided environment. Implement +/// [`IntegrityTest`](./trait.IntegrityTest.html) trait. #[macro_export] macro_rules! decl_module { // Entry point #1. @@ -298,6 +301,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -331,6 +335,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -349,6 +354,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event() = default; @@ -366,6 +372,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -382,6 +389,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event @@ -404,6 +412,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_finalize( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -423,6 +432,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -440,6 +450,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -466,6 +477,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -490,6 +502,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -516,6 +529,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -535,6 +549,48 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } + [ $( $dispatchables )* ] + $($rest)* + ); + }; + // Add integrity_test + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + {} + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn integrity_test() { $( $impl:tt )* } + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type<$trait_instance: $trait_name$(<I>, I: $instantiable $(= $module_default_instance)?)?> + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { $( $error_type )* } + { + $(#[doc = $doc_attr])* + fn integrity_test() { $( $impl)* } + } [ $( $dispatchables )* ] $($rest)* ); @@ -554,6 +610,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -578,6 +635,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -604,6 +662,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -623,6 +682,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -642,6 +702,7 @@ macro_rules! decl_module { { } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn offchain_worker( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -661,6 +722,7 @@ macro_rules! decl_module { { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -682,6 +744,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $( #[doc = $doc_attr:tt] )* const $name:ident: $ty:ty = $value:expr; @@ -706,6 +769,7 @@ macro_rules! decl_module { $name: $ty = $value; } { $( $error_type )* } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -727,6 +791,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* type Error = $error_type:ty; @@ -746,6 +811,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $error_type } + { $( $integrity_test)* } [ $( $dispatchables )* ] $($rest)* ); @@ -766,6 +832,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { } + { $( $integrity_test:tt )* } [ $($t:tt)* ] $($rest:tt)* ) => { @@ -783,6 +850,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { &'static str } + { $( $integrity_test)* } [ $($t)* ] $($rest)* ); @@ -804,6 +872,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $error_type:ty } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -826,6 +895,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $error_type } + { $( $integrity_test)* } [ $( $dispatchables )* $(#[doc = $doc_attr])* @@ -854,6 +924,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $fn_vis:vis fn $fn_name:ident( @@ -880,6 +951,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -906,6 +978,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -932,6 +1005,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -959,6 +1033,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $( $error_type:tt )* } + { $( $integrity_test:tt )* } [ $( $dispatchables:tt )* ] ) => { $crate::decl_module!(@imp @@ -975,6 +1050,7 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } + { $( $integrity_test)* } ); }; @@ -1081,6 +1157,32 @@ macro_rules! decl_module { {} }; + (@impl_integrity_test + $module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + $(#[doc = $doc_attr:tt])* + fn integrity_test() { $( $impl:tt )* } + ) => { + impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?> + $crate::traits::IntegrityTest + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + { + $(#[doc = $doc_attr])* + fn integrity_test() { + $( $impl )* + } + } + }; + + (@impl_integrity_test + $module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + ) => { + impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?> + $crate::traits::IntegrityTest + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + {} + }; (@impl_on_finalize $module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>; @@ -1340,6 +1442,7 @@ macro_rules! decl_module { { $( $offchain:tt )* } { $( $constants:tt )* } { $error_type:ty } + { $( $integrity_test:tt )* } ) => { $crate::__check_reserved_fn_name! { $( $fn_name )* } @@ -1366,7 +1469,6 @@ macro_rules! decl_module { $( $on_runtime_upgrade )* } - $crate::decl_module! { @impl_on_finalize $mod_type<$trait_instance: $trait_name $(<I>, $instance: $instantiable)?>; @@ -1388,6 +1490,13 @@ macro_rules! decl_module { $( $deposit_event )* } + $crate::decl_module! { + @impl_integrity_test + $mod_type<$trait_instance: $trait_name $(<I>, $instance: $instantiable)?>; + { $( $other_where_bounds )* } + $( $integrity_test )* + } + /// Can also be called using [`Call`]. /// /// [`Call`]: enum.Call.html @@ -2015,6 +2124,9 @@ macro_rules! __check_reserved_fn_name { (offchain_worker $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error offchain_worker); }; + (integrity_test $( $rest:ident )*) => { + $crate::__check_reserved_fn_name!(@compile_error integrity_test); + }; ($t:ident $( $rest:ident )*) => { $crate::__check_reserved_fn_name!($( $rest )*); }; @@ -2050,7 +2162,8 @@ mod tests { use super::*; use crate::weights::{DispatchInfo, DispatchClass, Pays}; use crate::traits::{ - CallMetadata, GetCallMetadata, GetCallName, OnInitialize, OnFinalize, OnRuntimeUpgrade + CallMetadata, GetCallMetadata, GetCallName, OnInitialize, OnFinalize, OnRuntimeUpgrade, + IntegrityTest, }; pub trait Trait: system::Trait + Sized where Self::AccountId: From<u32> { @@ -2112,6 +2225,8 @@ mod tests { fn on_finalize(n: T::BlockNumber,) { if n.into() == 42 { panic!("on_finalize") } } fn on_runtime_upgrade() -> Weight { 10 } fn offchain_worker() {} + /// Some doc + fn integrity_test() { panic!("integrity_test") } } } @@ -2303,4 +2418,10 @@ mod tests { let module_names = OuterCall::get_module_names(); assert_eq!(["Test"], module_names); } + + #[test] + #[should_panic(expected = "integrity_test")] + fn integrity_test_should_work() { + <Module<TraitImpl> as IntegrityTest>::integrity_test(); + } } diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 833160b61cf..9a2dbf2b299 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -192,6 +192,17 @@ macro_rules! impl_filter_stack { } } +/// Type that provide some integrity tests. +/// +/// This implemented for modules by `decl_module`. +#[impl_for_tuples(30)] +pub trait IntegrityTest { + /// Run integrity test. + /// + /// The test is not executed in a externalities provided environment. + fn integrity_test() {} +} + #[cfg(test)] mod test_impl_filter_stack { use super::*; diff --git a/substrate/frame/support/test/Cargo.toml b/substrate/frame/support/test/Cargo.toml index 65933929a5f..a68edc62381 100644 --- a/substrate/frame/support/test/Cargo.toml +++ b/substrate/frame/support/test/Cargo.toml @@ -20,6 +20,7 @@ frame-support = { version = "2.0.0-rc3", default-features = false, path = "../" sp-inherents = { version = "2.0.0-rc3", default-features = false, path = "../../../primitives/inherents" } sp-runtime = { version = "2.0.0-rc3", default-features = false, path = "../../../primitives/runtime" } sp-core = { version = "2.0.0-rc3", default-features = false, path = "../../../primitives/core" } +sp-std = { version = "2.0.0-rc3", default-features = false, path = "../../../primitives/std" } trybuild = "1.0.17" pretty_assertions = "0.6.1" rustversion = "1.0.0" @@ -33,6 +34,7 @@ std = [ "frame-support/std", "sp-inherents/std", "sp-core/std", + "sp-std/std", "sp-runtime/std", "sp-state-machine", ] diff --git a/substrate/frame/support/test/tests/decl_error.rs b/substrate/frame/support/test/tests/construct_runtime.rs similarity index 88% rename from substrate/frame/support/test/tests/decl_error.rs rename to substrate/frame/support/test/tests/construct_runtime.rs index 937e27873b0..10fc3319fb0 100644 --- a/substrate/frame/support/test/tests/decl_error.rs +++ b/substrate/frame/support/test/tests/construct_runtime.rs @@ -15,16 +15,24 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! General tests for construct_runtime macro, test for: +//! * error declareed with decl_error works +//! * integrity test is generated + #![recursion_limit="128"] use sp_runtime::{generic, traits::{BlakeTwo256, Block as _, Verify}, DispatchError}; use sp_core::{H256, sr25519}; - +use sp_std::cell::RefCell; mod system; pub trait Currency {} +thread_local! { + pub static INTEGRITY_TEST_EXEC: RefCell<u32> = RefCell::new(0); +} + mod module1 { use super::*; @@ -65,6 +73,10 @@ mod module2 { pub fn fail(_origin) -> frame_support::dispatch::DispatchResult { Err(Error::<T>::Something.into()) } + + fn integrity_test() { + INTEGRITY_TEST_EXEC.with(|i| *i.borrow_mut() += 1); + } } } @@ -139,3 +151,9 @@ fn check_module2_error_type() { Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), ); } + +#[test] +fn integrity_test_works() { + __construct_runtime_integrity_test::runtime_integrity_tests(); + assert_eq!(INTEGRITY_TEST_EXEC.with(|i| *i.borrow()), 1); +} -- GitLab