Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
enable optional keyword argument: exec_name
Browse files Browse the repository at this point in the history 
Took a _lot_ of macro-wrangling to get the functionality that I want,
but now you have the option to pass in

```rust
impl_benchmark_test_suite!(
	Elections,
	crate::tests::ExtBuilder::default().desired_members(13).desired_runners_up(7),
	crate::tests::Test,
	exec_name = build_and_execute,
);
```

and have it expand out properly. A selected fragment of the expansion:

```rust
        fn test_benchmarks() {
            crate::tests::ExtBuilder::default()
                .desired_members(13)
                .desired_runners_up(7)
                .build_and_execute(|| {
```
  • Loading branch information
@coriolinus
coriolinus committed Feb 15, 2021
1 parent 1732671 commit f1859ee
Show file tree
Hide file tree
Showing 2 changed files with 205 additions and 112 deletions.
143 changes: 116 additions & 27 deletions frame/benchmarking/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ macro_rules! impl_benchmark_test {
/// The first argument, `module`, must be the path to this crate's module.
///
/// The second argument, `new_test_ext`, must be a function call which returns either a
/// `sp_io::TestExternalities`, or some other type with an identical interface.
/// `sp_io::TestExternalities`, or some other type with a similar interface.
///
/// Note that this function call is _not_ evaluated at compile time, but is instead copied textually
/// into each appropriate invocation site.
Expand All @@ -977,11 +977,11 @@ macro_rules! impl_benchmark_test {
/// );
/// ```
///
/// There is an optional fourth argument: `path_to_benchmarks_invocation`. In the typical case in
/// which this macro is in the same module as the `benchmarks!` invocation, you don't need to supply
/// this. However, if the `impl_benchmark_test_suite!`
/// invocation is in a different module than the `benchmarks!` invocation, then you should provide
/// the path to the module containing the `benchmarks!` invocation:
/// There is an optional fourth argument, with keyword syntax: `benchmarks_path = path_to_benchmarks_invocation`.
/// In the typical case in which this macro is in the same module as the `benchmarks!` invocation,
/// you don't need to supply this. However, if the `impl_benchmark_test_suite!` invocation is in a
/// different module than the `benchmarks!` invocation, then you should provide the path to the
/// module containing the `benchmarks!` invocation:
///
/// ```rust,ignore
/// mod benches {
Expand All @@ -995,7 +995,7 @@ macro_rules! impl_benchmark_test {
/// // to be idents in the scope of `impl_benchmark_test_suite`.
/// use crate::{benches, Module};
///
/// impl_benchmark_test_suite!(Module, new_test_ext(), Test, benches);
/// impl_benchmark_test_suite!(Module, new_test_ext(), Test, benchmarks_path = benches);
///
/// // new_test_ext and the Test item are defined later in this module
/// }
Expand All @@ -1005,6 +1005,13 @@ macro_rules! impl_benchmark_test {
/// By default, this generates a test suite which iterates over all benchmarks, including those
/// marked with the `#[extra]` annotation. Setting `extra = false` excludes those.
///
/// There is an optional sixth argument, with keyword syntax: `exec_name = custom_exec_name`.
/// By default, this macro uses `execute_with` for this parameter. This argument, if set, is subject
/// to these restrictions:
///
/// - It must be the name of a method applied to the output of the `new_test_ext` argument.
/// - That method must have a signature capable of receiving a single argument of the form `impl FnOnce()`.
///
// ## Notes (not for rustdoc)
//
// The biggest challenge for this macro is communicating the actual test functions to be run. We
Expand All @@ -1021,31 +1028,113 @@ macro_rules! impl_benchmark_test {
// just iterate over the `Benchmarking::benchmarks` list to run the actual implementations.
#[macro_export]
macro_rules! impl_benchmark_test_suite {
// no options set
($bench_module:ident, $new_test_ext:expr, $test:path $(,)?) => {
impl_benchmark_test_suite!($bench_module, $new_test_ext, $test, super, extra = true);
// user might or might not have set some keyword arguments; set the defaults
//
// The weird syntax indicates that `rest` comes only after a comma, which is otherwise optional
(
$bench_module:ident,
$new_test_ext:expr,
$test:path
$(, $( $rest:tt )* )?
) => {
impl_benchmark_test_suite!(
@selected:
$bench_module,
$new_test_ext,
$test,
benchmarks_path = super,
extra = true,
exec_name = execute_with,
@user:
$( $( $rest )* )?
);
};
// set path to benchmarks invocation but not extra
($bench_module:ident, $new_test_ext:expr, $test:path, $path_to_benchmarks_invocation:ident $(,)?) => {
// pick off the benchmarks_path keyword argument
(
@selected:
$bench_module:ident,
$new_test_ext:expr,
$test:path,
benchmarks_path = $old:ident,
extra = $extra:expr,
exec_name = $exec_name:ident,
@user:
benchmarks_path = $benchmarks_path:ident
$(, $( $rest:tt )* )?
) => {
impl_benchmark_test_suite!(
$bench_module,
$new_test_ext,
$test,
$path_to_benchmarks_invocation,
extra = true,
@selected:
$bench_module,
$new_test_ext,
$test,
benchmarks_path = $benchmarks_path,
extra = $extra,
exec_name = $exec_name,
@user:
$( $( $rest )* )?
);
};
// set extra but not path to benchmarks invocation
($bench_module:ident, $new_test_ext:expr, $test:path, extra = $extra:expr $(,)?) => {
impl_benchmark_test_suite!($bench_module, $new_test_ext, $test, super, extra = $extra);
// pick off the extra keyword argument
(
@selected:
$bench_module:ident,
$new_test_ext:expr,
$test:path,
benchmarks_path = $benchmarks_path:ident,
extra = $old:expr,
exec_name = $exec_name:ident,
@user:
extra = $extra:expr
$(, $( $rest:tt )* )?
) => {
impl_benchmark_test_suite!(
@selected:
$bench_module,
$new_test_ext,
$test,
benchmarks_path = $benchmarks_path,
extra = $extra,
exec_name = $exec_name,
@user:
$( $( $rest )* )?
);
};
// all options set
// pick off the exec_name keyword argument
(
$bench_module:ident,
$new_test_ext:expr,
$test:path,
$path_to_benchmarks_invocation:ident,
extra = $extra:expr $(,)?
@selected:
$bench_module:ident,
$new_test_ext:expr,
$test:path,
benchmarks_path = $benchmarks_path:ident,
extra = $extra:expr,
exec_name = $old:ident,
@user:
exec_name = $exec_name:ident
$(, $( $rest:tt )* )?
) => {
impl_benchmark_test_suite!(
@selected:
$bench_module,
$new_test_ext,
$test,
benchmarks_path = $benchmarks_path,
extra = $extra,
exec_name = $exec_name,
@user:
$( $( $rest )* )?
);
};
// all options set; nothing else in user-provided keyword arguments
(
@selected:
$bench_module:ident,
$new_test_ext:expr,
$test:path,
benchmarks_path = $path_to_benchmarks_invocation:ident,
extra = $extra:expr,
exec_name = $exec_name:ident,
@user:
$(,)?
) => {
#[cfg(test)]
mod benchmark_tests {
Expand All @@ -1054,7 +1143,7 @@ macro_rules! impl_benchmark_test_suite {

#[test]
fn test_benchmarks() {
$new_test_ext.execute_with(|| {
$new_test_ext.$exec_name(|| {
use $crate::Benchmarking;

let mut anything_failed = false;
Expand Down
174 changes: 89 additions & 85 deletions frame/elections-phragmen/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use super::*;

use frame_system::RawOrigin;
use frame_benchmarking::{benchmarks, account};
use frame_benchmarking::{benchmarks, account, impl_benchmark_test_suite};
use frame_support::traits::OnInitialize;

use crate::Module as Elections;
Expand Down Expand Up @@ -536,87 +536,91 @@ benchmarks! {
}
}

// can't use impl_benchmark_test_suite for this module because the tests depend on
// `build_and_execute`, which checks pre- and post-conditions.

#[cfg(test)]
mod tests {
use super::*;
use crate::tests::{ExtBuilder, Test};
use frame_support::assert_ok;

#[test]
fn test_benchmarks_elections_phragmen() {
ExtBuilder::default()
.desired_members(13)
.desired_runners_up(7)
.build_and_execute(|| {
assert_ok!(test_benchmark_vote_equal::<Test>());
});

ExtBuilder::default()
.desired_members(13)
.desired_runners_up(7)
.build_and_execute(|| {
assert_ok!(test_benchmark_vote_more::<Test>());
});

ExtBuilder::default()
.desired_members(13)
.desired_runners_up(7)
.build_and_execute(|| {
assert_ok!(test_benchmark_vote_less::<Test>());
});

ExtBuilder::default()
.desired_members(13)
.desired_runners_up(7)
.build_and_execute(|| {
assert_ok!(test_benchmark_remove_voter::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_submit_candidacy::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_renounce_candidacy_candidate::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_renounce_candidacy_runners_up::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_renounce_candidacy_members::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_remove_member_without_replacement::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_remove_member_with_replacement::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_clean_defunct_voters::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_election_phragmen::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_election_phragmen::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_election_phragmen_c_e::<Test>());
});

ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
assert_ok!(test_benchmark_election_phragmen_v::<Test>());
});
}
}
impl_benchmark_test_suite!(
Elections,
crate::tests::ExtBuilder::default().desired_members(13).desired_runners_up(7),
crate::tests::Test,
exec_name = build_and_execute,
);

// #[cfg(test)]
// mod tests {
// use super::*;
// use crate::tests::{ExtBuilder, Test};
// use frame_support::assert_ok;

// #[test]
// fn test_benchmarks_elections_phragmen() {
// ExtBuilder::default()
// .desired_members(13)
// .desired_runners_up(7)
// .build_and_execute(|| {
// assert_ok!(test_benchmark_vote_equal::<Test>());
// });

// ExtBuilder::default()
// .desired_members(13)
// .desired_runners_up(7)
// .build_and_execute(|| {
// assert_ok!(test_benchmark_vote_more::<Test>());
// });

// ExtBuilder::default()
// .desired_members(13)
// .desired_runners_up(7)
// .build_and_execute(|| {
// assert_ok!(test_benchmark_vote_less::<Test>());
// });

// ExtBuilder::default()
// .desired_members(13)
// .desired_runners_up(7)
// .build_and_execute(|| {
// assert_ok!(test_benchmark_remove_voter::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_submit_candidacy::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_renounce_candidacy_candidate::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_renounce_candidacy_runners_up::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_renounce_candidacy_members::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_remove_member_without_replacement::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_remove_member_with_replacement::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_clean_defunct_voters::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_election_phragmen::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_election_phragmen::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_election_phragmen_c_e::<Test>());
// });

// ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
// assert_ok!(test_benchmark_election_phragmen_v::<Test>());
// });
// }
// }

0 comments on commit f1859ee

Please sign in to comment.