Proper shell overhead computationrefactor-benchmark-mod

5 files changed, 40 insertions, 22 deletions

diff --git a/src/benchmark/executor.rs b/src/benchmark/executor.rs
index 9022cad..11978b4 100644
--- a/src/benchmark/executor.rs
+++ b/src/benchmark/executor.rs
@@ -165,12 +165,12 @@ impl<'a> Executor for ShellExecutor<'a> {
 }
 
 #[derive(Clone)]
-pub struct MockExecutor<'a> {
-    shell: &'a Shell,
+pub struct MockExecutor {
+    shell: Option<String>,
 }
 
-impl<'a> MockExecutor<'a> {
-    pub fn new(shell: &'a Shell) -> Self {
+impl MockExecutor {
+    pub fn new(shell: Option<String>) -> Self {
         MockExecutor { shell }
     }
 
@@ -184,7 +184,7 @@ impl<'a> MockExecutor<'a> {
     }
 }
 
-impl<'a> Executor for MockExecutor<'a> {
+impl Executor for MockExecutor {
     fn time_command(
         &self,
         command: &Command<'_>,
@@ -217,9 +217,9 @@ impl<'a> Executor for MockExecutor<'a> {
     }
 
     fn time_overhead(&self) -> Second {
-        match self.shell {
-            Shell::Default(_) => 0.0,
-            Shell::Custom(shell) => Self::extract_time(&shell[0]),
+        match &self.shell {
+            None => 0.0,
+            Some(shell) => Self::extract_time(shell),
         }
     }
 }
diff --git a/src/benchmark/mod.rs b/src/benchmark/mod.rs
index 59f07fb..d26dc39 100644
--- a/src/benchmark/mod.rs
+++ b/src/benchmark/mod.rs
@@ -141,11 +141,10 @@ impl<'a> Benchmark<'a> {
             )
         });
         let run_preparation_command = || {
-            if let Some(ref cmd) = preparation_command {
-                self.run_preparation_command(cmd)
-            } else {
-                Ok(TimingResult::default())
-            }
+            preparation_command
+                .as_ref()
+                .map(|cmd| self.run_preparation_command(cmd))
+                .transpose()
         };
 
         self.run_setup_command(self.command.get_parameters().iter().cloned())?;
@@ -186,6 +185,8 @@ impl<'a> Benchmark<'a> {
         };
 
         let preparation_result = run_preparation_command()?;
+        let preparation_overhead =
+            preparation_result.map_or(0.0, |res| res.time_real + self.executor.time_overhead());
 
         // Initial timing run
         let (res, status) = self.executor.time_command(self.command, None)?;
@@ -193,7 +194,7 @@ impl<'a> Benchmark<'a> {
 
         // Determine number of benchmark runs
         let runs_in_min_time = (self.options.min_benchmarking_time
-            / (res.time_real + preparation_result.time_real + self.executor.time_overhead()))
+            / (res.time_real + self.executor.time_overhead() + preparation_overhead))
             as u64;
 
         let count = {
diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs
index c4e3603..9b259f8 100644
--- a/src/benchmark/scheduler.rs
+++ b/src/benchmark/scheduler.rs
@@ -33,7 +33,7 @@ impl<'a> Scheduler<'a> {
 
     pub fn run_benchmarks(&mut self) -> Result<()> {
         let mut executor: Box<dyn Executor> = match self.options.executor_kind {
-            ExecutorKind::Mock(ref shell) => Box::new(MockExecutor::new(shell)),
+            ExecutorKind::Mock(ref shell) => Box::new(MockExecutor::new(shell.clone())),
             ExecutorKind::Shell(ref shell) => Box::new(ShellExecutor::new(shell, &self.options)),
         };
 
diff --git a/src/options.rs b/src/options.rs
index 7d82e39..08aa38e 100644
--- a/src/options.rs
+++ b/src/options.rs
@@ -126,7 +126,7 @@ impl Default for CommandOutputPolicy {
 
 pub enum ExecutorKind {
     Shell(Shell),
-    Mock(Shell),
+    Mock(Option<String>),
 }
 
 impl Default for ExecutorKind {
@@ -276,8 +276,8 @@ impl Options {
         {
             (false, Some(shell)) => ExecutorKind::Shell(Shell::from_str(shell)?),
             (false, None) => ExecutorKind::Shell(Shell::default()),
-            (true, Some(shell)) => ExecutorKind::Mock(Shell::from_str(shell)?),
-            (true, None) => ExecutorKind::Mock(Shell::default()),
+            (true, Some(shell)) => ExecutorKind::Mock(Some(shell.into())),
+            (true, None) => ExecutorKind::Mock(None),
         };
 
         if matches.is_present("ignore-failure") {
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 42aa787..67ebf25 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -189,7 +189,6 @@ fn returns_mean_time_in_correct_unit() {
 #[test]
 fn performs_ten_runs_for_slow_commands() {
     hyperfine_debug()
-        .arg("--time-unit=millisecond")
         .arg("sleep 0.5")
         .assert()
         .success()
@@ -199,7 +198,6 @@ fn performs_ten_runs_for_slow_commands() {
 #[test]
 fn performs_three_seconds_of_benchmarking_for_fast_commands() {
     hyperfine_debug()
-        .arg("--time-unit=millisecond")
         .arg("sleep 0.01")
         .assert()
         .success()
@@ -207,14 +205,33 @@ fn performs_three_seconds_of_benchmarking_for_fast_commands() {
 }
 
 #[test]
-fn takes_time_of_preparation_command_into_account() {
+fn takes_shell_spawning_time_into_account_for_computing_number_of_runs() {
+    hyperfine_debug()
+        .arg("--shell=sleep 0.02")
+        .arg("sleep 0.01")
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("100 runs"));
+}
+
+#[test]
+fn takes_preparation_command_into_account_for_computing_number_of_runs() {
     hyperfine_debug()
-        .arg("--time-unit=millisecond")
         .arg("--prepare=sleep 0.02")
         .arg("sleep 0.01")
         .assert()
         .success()
         .stdout(predicate::str::contains("100 runs"));
+
+    // Shell overhead needs to be added to both the prepare command and the actual command,
+    // leading to a total benchmark time of (prepare + shell + cmd + shell = 0.1 s)
+    hyperfine_debug()
+        .arg("--shell=sleep 0.01")
+        .arg("--prepare=sleep 0.03")
+        .arg("sleep 0.05")
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("30 runs"));
 }
 
 #[test]