diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 27f70dea8c..4649c12018 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -937,7 +937,7 @@ mod tests { use super::*; use crate::{ gas::GasMeter, tests::{ExtBuilder, Test, Event as MetaEvent}, - storage::Storage, + storage::{Storage, ContractAbsentError}, tests::{ ALICE, BOB, CHARLIE, test_utils::{place_contract, set_balance, get_balance}, @@ -945,6 +945,7 @@ mod tests { exec::ExportedFunction::*, Error, Weight, CurrentSchedule, }; + use frame_support::assert_noop; use sp_runtime::DispatchError; use assert_matches::assert_matches; use std::{cell::RefCell, collections::HashMap, rc::Rc}; @@ -1571,7 +1572,10 @@ mod tests { ); // Check that the account has not been created. - assert!(Storage::::code_hash(&instantiated_contract_address).is_err()); + assert_noop!( + Storage::::code_hash(&instantiated_contract_address), + ContractAbsentError, + ); assert!(events().is_empty()); }); } diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index 40bc99ec12..78bf9863fd 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -21,8 +21,9 @@ use super::*; use frame_benchmarking::{benchmarks, account, whitelist_account, impl_benchmark_test_suite}; use frame_support::{ - IterableStorageMap, - traits::{Currency, Get, EnsureOrigin, OnInitialize, UnfilteredDispatchable, schedule::DispatchTime}, + assert_noop, assert_ok, IterableStorageMap, + traits::{Currency, Get, EnsureOrigin, OnInitialize, UnfilteredDispatchable, + schedule::DispatchTime}, }; use frame_system::{RawOrigin, Pallet as System, self, EventRecord}; use sp_runtime::traits::{Bounded, One}; @@ -206,11 +207,14 @@ benchmarks! { let origin = T::CancellationOrigin::successful_origin(); let referendum_index = add_referendum::(0)?; let call = Call::::emergency_cancel(referendum_index); - assert!(Democracy::::referendum_status(referendum_index).is_ok()); + assert_ok!(Democracy::::referendum_status(referendum_index)); }: { call.dispatch_bypass_filter(origin)? } verify { // Referendum has been canceled - assert!(Democracy::::referendum_status(referendum_index).is_err()); + assert_noop!( + Democracy::::referendum_status(referendum_index), + Error::::ReferendumInvalid, + ); } blacklist { @@ -224,18 +228,23 @@ benchmarks! { // Place our proposal in the external queue, too. let hash = T::Hashing::hash_of(&0); - assert!(Democracy::::external_propose(T::ExternalOrigin::successful_origin(), hash.clone()).is_ok()); + assert_ok!( + Democracy::::external_propose(T::ExternalOrigin::successful_origin(), hash.clone()) + ); // Add a referendum of our proposal. let referendum_index = add_referendum::(0)?; - assert!(Democracy::::referendum_status(referendum_index).is_ok()); + assert_ok!(Democracy::::referendum_status(referendum_index)); let call = Call::::blacklist(hash, Some(referendum_index)); let origin = T::BlacklistOrigin::successful_origin(); }: { call.dispatch_bypass_filter(origin)? } verify { // Referendum has been canceled - assert!(Democracy::::referendum_status(referendum_index).is_err()); + assert_noop!( + Democracy::::referendum_status(referendum_index), + Error::::ReferendumInvalid + ); } // Worst case scenario, we external propose a previously blacklisted proposal diff --git a/substrate/frame/democracy/src/tests/decoders.rs b/substrate/frame/democracy/src/tests/decoders.rs index 52b61d8d9e..0331ea3934 100644 --- a/substrate/frame/democracy/src/tests/decoders.rs +++ b/substrate/frame/democracy/src/tests/decoders.rs @@ -58,12 +58,12 @@ fn pre_image() { let key = Default::default(); let missing = PreimageStatus::Missing(0); Preimages::::insert(key, missing); - assert!(Democracy::pre_image_data_len(key).is_err()); + assert_noop!(Democracy::pre_image_data_len(key), Error::::PreimageMissing); assert_eq!(Democracy::check_pre_image_is_missing(key), Ok(())); Preimages::::remove(key); - assert!(Democracy::pre_image_data_len(key).is_err()); - assert!(Democracy::check_pre_image_is_missing(key).is_err()); + assert_noop!(Democracy::pre_image_data_len(key), Error::::PreimageMissing); + assert_noop!(Democracy::check_pre_image_is_missing(key), Error::::NotImminent); for l in vec![0, 10, 100, 1000u32] { let available = PreimageStatus::Available{ @@ -76,7 +76,8 @@ fn pre_image() { Preimages::::insert(key, available); assert_eq!(Democracy::pre_image_data_len(key), Ok(l)); - assert!(Democracy::check_pre_image_is_missing(key).is_err()); + assert_noop!(Democracy::check_pre_image_is_missing(key), + Error::::DuplicatePreimage); } }) } diff --git a/substrate/frame/democracy/src/tests/external_proposing.rs b/substrate/frame/democracy/src/tests/external_proposing.rs index ff1a7a87da..37654a5e91 100644 --- a/substrate/frame/democracy/src/tests/external_proposing.rs +++ b/substrate/frame/democracy/src/tests/external_proposing.rs @@ -93,7 +93,7 @@ fn external_blacklisting_should_work() { assert_ok!(Democracy::blacklist(Origin::root(), hash, None)); fast_forward_to(2); - assert!(Democracy::referendum_status(0).is_err()); + assert_noop!(Democracy::referendum_status(0), Error::::ReferendumInvalid); assert_noop!( Democracy::external_propose( diff --git a/substrate/frame/democracy/src/tests/public_proposals.rs b/substrate/frame/democracy/src/tests/public_proposals.rs index 4785ef0a89..4a4827ac7e 100644 --- a/substrate/frame/democracy/src/tests/public_proposals.rs +++ b/substrate/frame/democracy/src/tests/public_proposals.rs @@ -129,9 +129,9 @@ fn blacklisting_should_work() { fast_forward_to(2); let hash = set_balance_proposal_hash(4); - assert!(Democracy::referendum_status(0).is_ok()); + assert_ok!(Democracy::referendum_status(0)); assert_ok!(Democracy::blacklist(Origin::root(), hash, Some(0))); - assert!(Democracy::referendum_status(0).is_err()); + assert_noop!(Democracy::referendum_status(0), Error::::ReferendumInvalid); }); } diff --git a/substrate/frame/democracy/src/tests/voting.rs b/substrate/frame/democracy/src/tests/voting.rs index 207085ceb5..13072ebf87 100644 --- a/substrate/frame/democracy/src/tests/voting.rs +++ b/substrate/frame/democracy/src/tests/voting.rs @@ -80,12 +80,12 @@ fn single_proposal_should_work() { fast_forward_to(3); // referendum still running - assert!(Democracy::referendum_status(0).is_ok()); + assert_ok!(Democracy::referendum_status(0)); // referendum runs during 2 and 3, ends @ start of 4. fast_forward_to(4); - assert!(Democracy::referendum_status(0).is_err()); + assert_noop!(Democracy::referendum_status(0), Error::::ReferendumInvalid); assert!(pallet_scheduler::Agenda::::get(6)[0].is_some()); // referendum passes and wait another two blocks for enactment. diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs index b54a7d75a0..6b0d237c6e 100644 --- a/substrate/frame/election-provider-multi-phase/src/unsigned.rs +++ b/substrate/frame/election-provider-multi-phase/src/unsigned.rs @@ -832,20 +832,20 @@ mod tests { assert!(MultiPhase::try_acquire_offchain_lock(25).is_ok()); // next block: rejected. - assert!(MultiPhase::try_acquire_offchain_lock(26).is_err()); + assert_noop!(MultiPhase::try_acquire_offchain_lock(26), "recently executed."); // allowed after `OFFCHAIN_REPEAT` assert!(MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT).into()).is_ok()); // a fork like situation: re-execute last 3. - assert!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 3).into()).is_err() + assert_noop!( + MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 3).into()), "fork." ); - assert!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 2).into()).is_err() + assert_noop!( + MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 2).into()), "fork." ); - assert!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 1).into()).is_err() + assert_noop!( + MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 1).into()), "fork." ); }) } diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 0102fdea7c..bc2783f76b 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -535,7 +535,7 @@ mod tests { }, }; use frame_support::{ - parameter_types, + assert_err, parameter_types, weights::{Weight, RuntimeDbWeight, IdentityFee, WeightToFeePolynomial}, traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons}, }; @@ -889,7 +889,9 @@ mod tests { [69u8; 32].into(), Digest::default(), )); - assert!(Executive::apply_extrinsic(xt).is_err()); + assert_err!(Executive::apply_extrinsic(xt), + TransactionValidityError::Invalid(InvalidTransaction::Future) + ); assert_eq!(>::extrinsic_index(), Some(0)); }); } diff --git a/substrate/frame/grandpa/src/tests.rs b/substrate/frame/grandpa/src/tests.rs index 50462d3347..92d2c6c751 100644 --- a/substrate/frame/grandpa/src/tests.rs +++ b/substrate/frame/grandpa/src/tests.rs @@ -24,7 +24,7 @@ use crate::mock::*; use codec::{Decode, Encode}; use fg_primitives::ScheduledChange; use frame_support::{ - assert_err, assert_ok, + assert_err, assert_ok, assert_noop, traits::{Currency, OnFinalize, OneSessionHandler}, weights::{GetDispatchInfo, Pays}, }; @@ -100,21 +100,27 @@ fn cannot_schedule_change_when_one_pending() { initialize_block(1, Default::default()); Grandpa::schedule_change(to_authorities(vec![(4, 1), (5, 1), (6, 1)]), 1, None).unwrap(); assert!(>::exists()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None), + Error::::ChangePending + ); Grandpa::on_finalize(1); let header = System::finalize(); initialize_block(2, header.hash()); assert!(>::exists()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None), + Error::::ChangePending + ); Grandpa::on_finalize(2); let header = System::finalize(); initialize_block(3, header.hash()); assert!(!>::exists()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_ok()); + assert_ok!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None)); Grandpa::on_finalize(3); let _header = System::finalize(); @@ -148,7 +154,10 @@ fn dispatch_forced_change() { ).unwrap(); assert!(>::exists()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, Some(0)).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, Some(0)), + Error::::ChangePending + ); Grandpa::on_finalize(1); let mut header = System::finalize(); @@ -157,8 +166,14 @@ fn dispatch_forced_change() { initialize_block(i, header.hash()); assert!(>::get().unwrap().forced.is_some()); assert_eq!(Grandpa::next_forced(), Some(11)); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_err()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, Some(0)).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None), + Error::::ChangePending + ); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, Some(0)), + Error::::ChangePending + ); Grandpa::on_finalize(i); header = System::finalize(); @@ -170,7 +185,7 @@ fn dispatch_forced_change() { initialize_block(7, header.hash()); assert!(!>::exists()); assert_eq!(Grandpa::grandpa_authorities(), to_authorities(vec![(4, 1), (5, 1), (6, 1)])); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_ok()); + assert_ok!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None)); Grandpa::on_finalize(7); header = System::finalize(); } @@ -180,7 +195,10 @@ fn dispatch_forced_change() { initialize_block(8, header.hash()); assert!(>::exists()); assert_eq!(Grandpa::grandpa_authorities(), to_authorities(vec![(4, 1), (5, 1), (6, 1)])); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1)]), 1, None), + Error::::ChangePending + ); Grandpa::on_finalize(8); header = System::finalize(); } @@ -192,7 +210,10 @@ fn dispatch_forced_change() { assert!(!>::exists()); assert_eq!(Grandpa::grandpa_authorities(), to_authorities(vec![(5, 1)])); assert_eq!(Grandpa::next_forced(), Some(11)); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1), (6, 1)]), 5, Some(0)).is_err()); + assert_noop!( + Grandpa::schedule_change(to_authorities(vec![(5, 1), (6, 1)]), 5, Some(0)), + Error::::TooSoon + ); Grandpa::on_finalize(i); header = System::finalize(); } @@ -200,7 +221,7 @@ fn dispatch_forced_change() { { initialize_block(11, header.hash()); assert!(!>::exists()); - assert!(Grandpa::schedule_change(to_authorities(vec![(5, 1), (6, 1), (7, 1)]), 5, Some(0)).is_ok()); + assert_ok!(Grandpa::schedule_change(to_authorities(vec![(5, 1), (6, 1), (7, 1)]), 5, Some(0))); assert_eq!(Grandpa::next_forced(), Some(21)); Grandpa::on_finalize(11); header = System::finalize(); @@ -231,7 +252,7 @@ fn schedule_pause_only_when_live() { initialize_block(2, Default::default()); // signaling a pause now should fail - assert!(Grandpa::schedule_pause(1).is_err()); + assert_noop!(Grandpa::schedule_pause(1), Error::::PauseFailed); Grandpa::on_finalize(2); let _ = System::finalize(); @@ -250,7 +271,7 @@ fn schedule_resume_only_when_paused() { initialize_block(1, Default::default()); // the set is currently live, resuming it is an error - assert!(Grandpa::schedule_resume(1).is_err()); + assert_noop!(Grandpa::schedule_resume(1), Error::::ResumeFailed); assert_eq!( Grandpa::state(), diff --git a/substrate/frame/system/src/extensions/check_nonce.rs b/substrate/frame/system/src/extensions/check_nonce.rs index 3cb74a7ed9..cb25c3c027 100644 --- a/substrate/frame/system/src/extensions/check_nonce.rs +++ b/substrate/frame/system/src/extensions/check_nonce.rs @@ -120,6 +120,7 @@ impl SignedExtension for CheckNonce where mod tests { use super::*; use crate::mock::{Test, new_test_ext, CALL}; + use frame_support::{assert_noop, assert_ok}; #[test] fn signed_ext_check_nonce_works() { @@ -134,14 +135,23 @@ mod tests { let info = DispatchInfo::default(); let len = 0_usize; // stale - assert!(CheckNonce::(0).validate(&1, CALL, &info, len).is_err()); - assert!(CheckNonce::(0).pre_dispatch(&1, CALL, &info, len).is_err()); + assert_noop!( + CheckNonce::(0).validate(&1, CALL, &info, len), + InvalidTransaction::Stale + ); + assert_noop!( + CheckNonce::(0).pre_dispatch(&1, CALL, &info, len), + InvalidTransaction::Stale + ); // correct - assert!(CheckNonce::(1).validate(&1, CALL, &info, len).is_ok()); - assert!(CheckNonce::(1).pre_dispatch(&1, CALL, &info, len).is_ok()); + assert_ok!(CheckNonce::(1).validate(&1, CALL, &info, len)); + assert_ok!(CheckNonce::(1).pre_dispatch(&1, CALL, &info, len)); // future - assert!(CheckNonce::(5).validate(&1, CALL, &info, len).is_ok()); - assert!(CheckNonce::(5).pre_dispatch(&1, CALL, &info, len).is_err()); + assert_ok!(CheckNonce::(5).validate(&1, CALL, &info, len)); + assert_noop!( + CheckNonce::(5).pre_dispatch(&1, CALL, &info, len), + InvalidTransaction::Future + ); }) } } diff --git a/substrate/frame/system/src/extensions/check_weight.rs b/substrate/frame/system/src/extensions/check_weight.rs index a4ebeaea30..e01c913176 100644 --- a/substrate/frame/system/src/extensions/check_weight.rs +++ b/substrate/frame/system/src/extensions/check_weight.rs @@ -26,7 +26,7 @@ use sp_runtime::{ DispatchResult, }; use frame_support::{ - traits::{Get}, + traits::Get, weights::{PostDispatchInfo, DispatchInfo, DispatchClass, priority::FrameTransactionPriority}, }; @@ -281,8 +281,7 @@ mod tests { use crate::{BlockWeight, AllExtrinsicsLen}; use crate::mock::{Test, CALL, new_test_ext, System}; use sp_std::marker::PhantomData; - use frame_support::{assert_ok, assert_noop}; - use frame_support::weights::{Weight, Pays}; + use frame_support::{assert_err, assert_ok, weights::{Weight, Pays}}; fn block_weights() -> crate::limits::BlockWeights { ::BlockWeights::get() @@ -335,11 +334,7 @@ mod tests { ..Default::default() }; let len = 0_usize; - - assert_noop!( - CheckWeight::::do_validate(&max, len), - InvalidTransaction::ExhaustsResources - ); + assert_err!(CheckWeight::::do_validate(&max, len), InvalidTransaction::ExhaustsResources); }); } @@ -371,10 +366,7 @@ mod tests { ..Default::default() }) ); - assert_noop!( - CheckWeight::::do_validate(&max, len), - InvalidTransaction::ExhaustsResources - ); + assert_err!(CheckWeight::::do_validate(&max, len), InvalidTransaction::ExhaustsResources); }); } @@ -437,15 +429,13 @@ mod tests { let dispatch_operational = DispatchInfo { weight: 251, class: DispatchClass::Operational, ..Default::default() }; let len = 0_usize; - assert_noop!( - CheckWeight::::do_pre_dispatch(&dispatch_normal, len), + assert_err!( CheckWeight::::do_pre_dispatch(&dispatch_normal, len), InvalidTransaction::ExhaustsResources ); // Thank goodness we can still do an operational transaction to possibly save the blockchain. assert_ok!(CheckWeight::::do_pre_dispatch(&dispatch_operational, len)); // Not too much though - assert_noop!( - CheckWeight::::do_pre_dispatch(&dispatch_operational, len), + assert_err!(CheckWeight::::do_pre_dispatch(&dispatch_operational, len), InvalidTransaction::ExhaustsResources ); // Even with full block, validity of single transaction should be correct. @@ -466,15 +456,19 @@ mod tests { current_weight.set(normal_limit, DispatchClass::Normal) }); // will not fit. - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &normal, len).is_err()); + assert_err!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &normal, len), + InvalidTransaction::ExhaustsResources + ); // will fit. - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &op, len).is_ok()); + assert_ok!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &op, len)); // likewise for length limit. let len = 100_usize; AllExtrinsicsLen::::put(normal_length_limit()); - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &normal, len).is_err()); - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &op, len).is_ok()); + assert_err!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &normal, len), + InvalidTransaction::ExhaustsResources + ); + assert_ok!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &op, len)); }) } @@ -575,10 +569,7 @@ mod tests { let pre = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap(); assert_eq!(BlockWeight::::get().total(), info.weight + 256); - assert!( - CheckWeight::::post_dispatch(pre, &info, &post_info, len, &Ok(())) - .is_ok() - ); + assert_ok!( CheckWeight::::post_dispatch(pre, &info, &post_info, len, &Ok(()))); assert_eq!( BlockWeight::::get().total(), post_info.actual_weight.unwrap() + 256, @@ -607,10 +598,7 @@ mod tests { info.weight + 128 + block_weights().get(DispatchClass::Normal).base_extrinsic, ); - assert!( - CheckWeight::::post_dispatch(pre, &info, &post_info, len, &Ok(())) - .is_ok() - ); + assert_ok!(CheckWeight::::post_dispatch(pre, &info, &post_info, len, &Ok(()))); assert_eq!( BlockWeight::::get().total(), info.weight + 128 + block_weights().get(DispatchClass::Normal).base_extrinsic, @@ -630,8 +618,7 @@ mod tests { System::block_weight().total(), weights.base_block ); - let r = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &free, len); - assert!(r.is_ok()); + assert_ok!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &free, len)); assert_eq!( System::block_weight().total(), weights.get(DispatchClass::Normal).base_extrinsic + weights.base_block @@ -687,15 +674,14 @@ mod tests { let mandatory2 = DispatchInfo { weight: 6, class: DispatchClass::Mandatory, ..Default::default() }; // when - let result1 = calculate_consumed_weight::<::Call>( - maximum_weight.clone(), all_weight.clone(), &mandatory1 + assert_ok!( + calculate_consumed_weight::<::Call>( + maximum_weight.clone(), all_weight.clone(), &mandatory1 + ) ); - let result2 = calculate_consumed_weight::<::Call>( - maximum_weight, all_weight, &mandatory2 + assert_err!( + calculate_consumed_weight::<::Call>( maximum_weight, all_weight, &mandatory2), + InvalidTransaction::ExhaustsResources ); - - // then - assert!(result2.is_err()); - assert!(result1.is_ok()); } } diff --git a/substrate/frame/system/src/tests.rs b/substrate/frame/system/src/tests.rs index 9f500e5a3b..7ad4344ae5 100644 --- a/substrate/frame/system/src/tests.rs +++ b/substrate/frame/system/src/tests.rs @@ -19,7 +19,9 @@ use crate::*; use mock::{*, Origin}; use sp_core::H256; use sp_runtime::{DispatchError, DispatchErrorWithPostInfo, traits::{Header, BlakeTwo256}}; -use frame_support::{assert_noop, weights::WithPostDispatchInfo, dispatch::PostDispatchInfo}; +use frame_support::{ + assert_noop, assert_ok, weights::WithPostDispatchInfo, dispatch::PostDispatchInfo +}; #[test] fn origin_works() { @@ -31,7 +33,7 @@ fn origin_works() { #[test] fn stored_map_works() { new_test_ext().execute_with(|| { - assert!(System::insert(&0, 42).is_ok()); + assert_ok!(System::insert(&0, 42)); assert!(!System::is_provider_required(&0)); assert_eq!(Account::::get(0), AccountInfo { @@ -42,17 +44,17 @@ fn stored_map_works() { data: 42, }); - assert!(System::inc_consumers(&0).is_ok()); + assert_ok!(System::inc_consumers(&0)); assert!(System::is_provider_required(&0)); - assert!(System::insert(&0, 69).is_ok()); + assert_ok!(System::insert(&0, 69)); assert!(System::is_provider_required(&0)); System::dec_consumers(&0); assert!(!System::is_provider_required(&0)); assert!(KILLED.with(|r| r.borrow().is_empty())); - assert!(System::remove(&0).is_ok()); + assert_ok!(System::remove(&0)); assert_eq!(KILLED.with(|r| r.borrow().clone()), vec![0u64]); }); } @@ -122,7 +124,7 @@ fn sufficient_cannot_support_consumer() { assert_noop!(System::inc_consumers(&0), IncRefError::NoProviders); assert_eq!(System::inc_providers(&0), IncRefStatus::Existed); - assert!(System::inc_consumers(&0).is_ok()); + assert_ok!(System::inc_consumers(&0)); assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining); }); } @@ -140,7 +142,7 @@ fn provider_required_to_support_consumer() { assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Exists); assert_eq!(System::account_nonce(&0), 1); - assert!(System::inc_consumers(&0).is_ok()); + assert_ok!(System::inc_consumers(&0)); assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining); System::dec_consumers(&0); @@ -516,7 +518,7 @@ fn ensure_one_of_works() { assert_eq!(ensure_root_or_signed(RawOrigin::Root).unwrap(), Either::Left(())); assert_eq!(ensure_root_or_signed(RawOrigin::Signed(0)).unwrap(), Either::Right(0)); - assert!(ensure_root_or_signed(RawOrigin::None).is_err()) + assert!(ensure_root_or_signed(RawOrigin::None).is_err()); } #[test] diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs index 278cabc400..ff69386838 100644 --- a/substrate/frame/transaction-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/src/lib.rs @@ -604,7 +604,7 @@ mod tests { use frame_system as system; use codec::Encode; use frame_support::{ - parameter_types, + assert_noop, assert_ok, parameter_types, weights::{ DispatchClass, DispatchInfo, PostDispatchInfo, GetDispatchInfo, Weight, WeightToFeePolynomial, WeightToFeeCoefficients, WeightToFeeCoefficient, @@ -616,6 +616,7 @@ mod tests { use sp_runtime::{ testing::{Header, TestXt}, traits::{BlakeTwo256, IdentityLookup}, + transaction_validity::InvalidTransaction, Perbill, }; use std::cell::RefCell; @@ -826,10 +827,9 @@ mod tests { .unwrap(); assert_eq!(Balances::free_balance(1), 100 - 5 - 5 - 10); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &info_from_weight(5), &default_post_info(), len, &Ok(())) - .is_ok() ); assert_eq!(Balances::free_balance(1), 100 - 5 - 5 - 10); @@ -838,10 +838,9 @@ mod tests { .unwrap(); assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 100 - 5); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &info_from_weight(100), &post_info_from_weight(50), len, &Ok(())) - .is_ok() ); assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 50 - 5); }); @@ -864,10 +863,9 @@ mod tests { // 5 base fee, 10 byte fee, 3/2 * 100 weight fee, 5 tip assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 150 - 5); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &info_from_weight(100), &post_info_from_weight(50), len, &Ok(())) - .is_ok() ); // 75 (3/2 of the returned 50 units of weight) is refunded assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 75 - 5); @@ -883,10 +881,9 @@ mod tests { .execute_with(|| { // maximum weight possible - assert!( + assert_ok!( ChargeTransactionPayment::::from(0) .pre_dispatch(&1, CALL, &info_from_weight(Weight::max_value()), 10) - .is_ok() ); // fee will be proportional to what is the actual maximum weight in the runtime. assert_eq!( @@ -915,10 +912,9 @@ mod tests { class: DispatchClass::Operational, pays_fee: Pays::No, }; - assert!( + assert_ok!( ChargeTransactionPayment::::from(0) .validate(&1, CALL, &operational_transaction , len) - .is_ok() ); // like a InsecureFreeNormal @@ -927,10 +923,10 @@ mod tests { class: DispatchClass::Normal, pays_fee: Pays::Yes, }; - assert!( + assert_noop!( ChargeTransactionPayment::::from(0) - .validate(&1, CALL, &free_transaction , len) - .is_err() + .validate(&1, CALL, &free_transaction , len), + TransactionValidityError::Invalid(InvalidTransaction::Payment), ); }); } @@ -947,10 +943,9 @@ mod tests { NextFeeMultiplier::put(Multiplier::saturating_from_rational(3, 2)); let len = 10; - assert!( + assert_ok!( ChargeTransactionPayment::::from(10) // tipped .pre_dispatch(&1, CALL, &info_from_weight(3), len) - .is_ok() ); assert_eq!( Balances::free_balance(1), @@ -1146,13 +1141,12 @@ mod tests { assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 100 - 5); // kill the account between pre and post dispatch - assert!(Balances::transfer(Some(2).into(), 3, Balances::free_balance(2)).is_ok()); + assert_ok!(Balances::transfer(Some(2).into(), 3, Balances::free_balance(2))); assert_eq!(Balances::free_balance(2), 0); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &info_from_weight(100), &post_info_from_weight(50), len, &Ok(())) - .is_ok() ); assert_eq!(Balances::free_balance(2), 0); // Transfer Event @@ -1180,10 +1174,9 @@ mod tests { .unwrap(); assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 100 - 5); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &info_from_weight(100), &post_info_from_weight(101), len, &Ok(())) - .is_ok() ); assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 100 - 5); }); @@ -1210,10 +1203,9 @@ mod tests { .pre_dispatch(&user, CALL, &dispatch_info, len) .unwrap(); assert_eq!(Balances::total_balance(&user), 0); - assert!( + assert_ok!( ChargeTransactionPayment:: ::post_dispatch(pre, &dispatch_info, &default_post_info(), len, &Ok(())) - .is_ok() ); assert_eq!(Balances::total_balance(&user), 0); // No events for such a scenario