paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.89k stars 692 forks source link

benchmarking macro v2: tests don't seem to execute the verify logic. #6464

Closed gui1117 closed 22 hours ago

gui1117 commented 23 hours ago

I modify a benchmark to return an error or to panic in the verify branch, the tests are still successful.

diff --git a/substrate/frame/broker/src/benchmarking.rs b/substrate/frame/broker/src/benchmarking.rs
index 9ef9b125443..fa578f611b1 100644
--- a/substrate/frame/broker/src/benchmarking.rs
+++ b/substrate/frame/broker/src/benchmarking.rs
@@ -1063,6 +1063,7 @@ mod benches {
                #[extrinsic_call]
                _(RawOrigin::Signed(caller), region.core, 2001, None);

+               return Err(BenchmarkError::Stop("aaaah"));
                assert_last_event::<T>(Event::AutoRenewalEnabled { core: region.core, task: 2001 }.into());
                // Make sure we indeed renewed:
                assert!(PotentialRenewals::<T>::get(PotentialRenewalId {

result: test benchmarking::benches::bench_enable_auto_renew ... ok

diff --git a/substrate/frame/broker/src/benchmarking.rs b/substrate/frame/broker/src/benchmarking.rs
index 9ef9b125443..f4e5ffcc977 100644
--- a/substrate/frame/broker/src/benchmarking.rs
+++ b/substrate/frame/broker/src/benchmarking.rs
@@ -1063,6 +1063,7 @@ mod benches {
                #[extrinsic_call]
                _(RawOrigin::Signed(caller), region.core, 2001, None);

+               panic!("aah");
                assert_last_event::<T>(Event::AutoRenewalEnabled { core: region.core, task: 2001 }.into());
                // Make sure we indeed renewed:
                assert!(PotentialRenewals::<T>::get(PotentialRenewalId {

result: test benchmarking::benches::bench_enable_auto_renew ... ok

gui1117 commented 22 hours ago

This is because BenchmarkError::Weightless is consider a successful result and it is used in some setup of the benchmark