From 6e57431d33b0c40a80a30854940bb503b33a3758 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 13 Sep 2022 14:43:15 +0200 Subject: [PATCH 1/4] Use temporary db for benchmarking If no db option was given benchmarks shall use temporary database. Otherwise the test can use locally stored database which maybe out-of-date causing test to fail. --- utils/frame/benchmarking-cli/src/lib.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 6e4f092084ef5..b86dab46ca91a 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -30,6 +30,7 @@ pub use extrinsic::{ExtrinsicBuilder, ExtrinsicCmd, ExtrinsicFactory}; pub use machine::{MachineCmd, Requirements, SUBSTRATE_REFERENCE_HARDWARE}; pub use overhead::OverheadCmd; pub use pallet::PalletCmd; +pub use sc_service::BasePath; pub use storage::StorageCmd; use sc_cli::{CliConfiguration, DatabaseParams, ImportParams, PruningParams, Result, SharedParams}; @@ -87,6 +88,20 @@ impl CliConfiguration for BenchmarkCmd { } } + fn base_path(&self) -> Result> { + let inner = unwrap_cmd! { + self, cmd, cmd.base_path() + }; + + if let Ok(ref path) = inner { + if path.is_none() { + return Some(BasePath::new_temp_dir()).transpose().map_err(|e| e.into()) + } + } + + inner + } + fn pruning_params(&self) -> Option<&PruningParams> { unwrap_cmd! { self, cmd, cmd.pruning_params() From 30d391ee0031cd24ccb0bda68d3f6f05c71ce6e0 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 13 Sep 2022 15:02:59 +0200 Subject: [PATCH 2/4] nicer syntax --- utils/frame/benchmarking-cli/src/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index b86dab46ca91a..3fd7fb81cb54a 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -93,13 +93,10 @@ impl CliConfiguration for BenchmarkCmd { self, cmd, cmd.base_path() }; - if let Ok(ref path) = inner { - if path.is_none() { - return Some(BasePath::new_temp_dir()).transpose().map_err(|e| e.into()) - } + match inner { + Ok(None) => Some(BasePath::new_temp_dir()).transpose().map_err(|e| e.into()), + e => e, } - - inner } fn pruning_params(&self) -> Option<&PruningParams> { From 58b2a8bc63b4bb96ca37005ca7294490dbb1a436 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 13 Sep 2022 17:02:22 +0200 Subject: [PATCH 3/4] explanatory comment added --- utils/frame/benchmarking-cli/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 3fd7fb81cb54a..10bff016517f2 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -93,6 +93,8 @@ impl CliConfiguration for BenchmarkCmd { self, cmd, cmd.base_path() }; + //If the base path was not provided, benchmark command shall use temporary path. Otherwise + //we may end up using shared path, which may be inappropriate for benchmarking. match inner { Ok(None) => Some(BasePath::new_temp_dir()).transpose().map_err(|e| e.into()), e => e, From 5e3763515a58ca94b3c094f29f1c4d2666dca652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 20 Sep 2022 22:58:55 +0200 Subject: [PATCH 4/4] Update utils/frame/benchmarking-cli/src/lib.rs --- utils/frame/benchmarking-cli/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 10bff016517f2..a44a208b16ae9 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -93,8 +93,8 @@ impl CliConfiguration for BenchmarkCmd { self, cmd, cmd.base_path() }; - //If the base path was not provided, benchmark command shall use temporary path. Otherwise - //we may end up using shared path, which may be inappropriate for benchmarking. + // If the base path was not provided, benchmark command shall use temporary path. Otherwise + // we may end up using shared path, which may be inappropriate for benchmarking. match inner { Ok(None) => Some(BasePath::new_temp_dir()).transpose().map_err(|e| e.into()), e => e,