fix(pallet-benchmarking): split test functions in v2 (#3574)

Closes #376

---------

Co-authored-by: command-bot <>

cumulus/pallets/collator-selection/src/benchmarking.rs

@@ -22,9 +22,7 @@ use super::*;
 #[allow(unused)]
 use crate::Pallet as CollatorSelection;
 use codec::Decode;
-use frame_benchmarking::{
-	account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError,
-};
+use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError};
 use frame_support::traits::{Currency, EnsureOrigin, Get, ReservableCurrency};
 use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, RawOrigin};
 use pallet_authorship::EventHandler;

cumulus/parachains/pallets/collective-content/src/benchmarking.rs

@@ -16,7 +16,7 @@
 //! The pallet benchmarks.
 
 use super::{Pallet as CollectiveContent, *};
-use frame_benchmarking::{impl_benchmark_test_suite, v2::*};
+use frame_benchmarking::v2::*;
 use frame_support::traits::EnsureOrigin;
 
 fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::RuntimeEvent) {

polkadot/runtime/common/src/identity_migrator.rs

@@ -31,7 +31,7 @@ use pallet_identity;
 use sp_core::Get;
 
 #[cfg(feature = "runtime-benchmarks")]
-use frame_benchmarking::{account, impl_benchmark_test_suite, v2::*, BenchmarkError};
+use frame_benchmarking::{account, v2::*, BenchmarkError};
 
 pub trait WeightInfo {
 	fn reap_identity(r: u32, s: u32) -> Weight;

polkadot/runtime/parachains/src/hrmp/benchmarking.rs

@@ -22,7 +22,7 @@ use crate::{
 	paras::{Pallet as Paras, ParaKind, ParachainsCache},
 	shared::Pallet as Shared,
 };
-use frame_benchmarking::{impl_benchmark_test_suite, v2::*, whitelisted_caller};
+use frame_benchmarking::{v2::*, whitelisted_caller};
 use frame_support::{assert_ok, traits::Currency};
 
 type BalanceOf<T> =

prdoc/pr_3574.prdoc

@@ -0,0 +1,23 @@
+title: Generate test functions for each benchmark with benchmarking v2
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR fixes an issue where using `impl_benchmark_test_suite` macro
+      within modules that use the benchmarking v2 macros (`#[benchmarks]`
+      and `#[instance_benchmarks]`) always produced a single test called
+      `test_benchmarks` instead of a separate benchmark test for every
+      benchmark (noted with the `#[benchmark]` macro).
+
+      By using this macro from now on, new tests will be created named
+      `test_benchmark_{name}` where `name` is the name of the benchmark
+      function. Those tests will be nested inside the module intended for
+      benchmark functions.
+
+      Also, when using `impl_benchmark_test_suite` inside the module,
+      the import of such marco will not be necessary, so any explicit
+      import of it will be marked as unused, the same way it works for
+      v1 macros so far.
+
+crates:
+  - name: frame-support-procedural

substrate/frame/alliance/src/benchmarking.rs

@@ -26,7 +26,7 @@ use core::{
 };
 use sp_runtime::traits::{Bounded, Hash, StaticLookup};
 
-use frame_benchmarking::{account, impl_benchmark_test_suite, v2::*, BenchmarkError};
+use frame_benchmarking::{account, v2::*, BenchmarkError};
 use frame_support::traits::{EnsureOrigin, Get, UnfilteredDispatchable};
 use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin as SystemOrigin};
 

substrate/frame/identity/src/benchmarking.rs

@@ -23,9 +23,7 @@ use super::*;
 
 use crate::Pallet as Identity;
 use codec::Encode;
-use frame_benchmarking::{
-	account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError,
-};
+use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError};
 use frame_support::{
 	assert_ok, ensure,
 	traits::{EnsureOrigin, Get, OnFinalize, OnInitialize},

substrate/frame/lottery/src/benchmarking.rs

@@ -23,7 +23,6 @@ use super::*;
 
 use crate::Pallet as Lottery;
 use frame_benchmarking::{
-	impl_benchmark_test_suite,
 	v1::{account, whitelisted_caller, BenchmarkError},
 	v2::*,
 };

substrate/frame/support/procedural/src/benchmark.rs

@@ -431,7 +431,7 @@ pub fn benchmarks(
 	let mut benchmarks_by_name_mappings: Vec<TokenStream2> = Vec::new();
 	let test_idents: Vec<Ident> = benchmark_names_str
 		.iter()
-		.map(|n| Ident::new(format!("test_{}", n).as_str(), Span::call_site()))
+		.map(|n| Ident::new(format!("test_benchmark_{}", n).as_str(), Span::call_site()))
 		.collect();
 	for i in 0..benchmark_names.len() {
 		let name_ident = &benchmark_names[i];
@@ -441,6 +441,37 @@ pub fn benchmarks(
 		benchmarks_by_name_mappings.push(quote!(#name_str => Self::#test_ident()))
 	}
 
+	let impl_test_function = content
+		.iter_mut()
+		.find_map(|item| {
+			let Item::Macro(item_macro) = item else {
+				return None;
+			};
+
+			if !item_macro
+				.mac
+				.path
+				.segments
+				.iter()
+				.any(|s| s.ident == "impl_benchmark_test_suite")
+			{
+				return None;
+			}
+
+			let tokens = item_macro.mac.tokens.clone();
+			*item = Item::Verbatim(quote! {});
+
+			Some(quote! {
+				impl_test_function!(
+					(#( {} #benchmark_names )*)
+					(#( #extra_benchmark_names )*)
+					(#( #skip_meta_benchmark_names )*)
+					#tokens
+				);
+			})
+		})
+		.unwrap_or(quote! {});
+
 	// emit final quoted tokens
 	let res = quote! {
 		#(#mod_attrs)
@@ -676,6 +707,8 @@ pub fn benchmarks(
 					}
 				}
 			}
+
+			#impl_test_function
 		}
 		#mod_vis use #mod_name::*;
 	};
@@ -733,7 +766,8 @@ fn expand_benchmark(
 	let setup_stmts = benchmark_def.setup_stmts;
 	let verify_stmts = benchmark_def.verify_stmts;
 	let last_stmt = benchmark_def.last_stmt;
-	let test_ident = Ident::new(format!("test_{}", name.to_string()).as_str(), Span::call_site());
+	let test_ident =
+		Ident::new(format!("test_benchmark_{}", name.to_string()).as_str(), Span::call_site());
 
 	// unroll params (prepare for quoting)
 	let unrolled = UnrolledParams::from(&benchmark_def.params);

substrate/frame/system/benchmarking/src/extensions.rs

@@ -19,7 +19,7 @@
 
 #![cfg(feature = "runtime-benchmarks")]
 
-use frame_benchmarking::{account, impl_benchmark_test_suite, v2::*, BenchmarkError};
+use frame_benchmarking::{account, v2::*, BenchmarkError};
 use frame_support::{
 	dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo},
 	weights::Weight,

substrate/frame/system/benchmarking/src/lib.rs

@@ -21,7 +21,7 @@
 #![cfg(feature = "runtime-benchmarks")]
 
 use codec::Encode;
-use frame_benchmarking::{impl_benchmark_test_suite, v2::*};
+use frame_benchmarking::v2::*;
 use frame_support::{dispatch::DispatchClass, storage, traits::Get};
 use frame_system::{Call, Pallet as System, RawOrigin};
 use sp_core::storage::well_known_keys;