Redesign windsock CLI

windsock/src/cli.rs

@@ -1,5 +1,56 @@
 use anyhow::{anyhow, Error};
-use clap::Parser;
+use clap::{Parser, Subcommand};
+
+#[derive(Subcommand, Clone)]
+pub enum Command {
+    /// List the name of every bench.
+    #[clap(verbatim_doc_comment)]
+    List,
+
+    /// The results of the last benchmarks run becomes the new baseline from which future benchmark runs will be compared.
+    #[clap(verbatim_doc_comment)]
+    BaselineSet,
+
+    /// Removes the stored baseline. Following runs will no longer compare against a baseline.
+    #[clap(verbatim_doc_comment)]
+    BaselineClear,
+
+    /// Generate graphs webpage from the last benchmarks run.
+    #[clap(verbatim_doc_comment)]
+    GenerateWebpage,
+
+    /// Display results from the last benchmark run by:
+    ///     Listing bench results matching tag filters.
+    ///
+    /// Usage: Provide tag filters
+    #[clap(verbatim_doc_comment)]
+    // TODO: get trailing_var_arg(true) working so user can avoid having to wrap in ""
+    Results {
+        /// Do not compare against the set baseline.
+        #[clap(long, verbatim_doc_comment)]
+        ignore_baseline: bool,
+
+        /// e.g. "db=kafka OPS=10000"
+        #[clap(verbatim_doc_comment)]
+        filter: Option<String>,
+    },
+
+    /// Display results from the last benchmark run by:
+    ///     Comparing various benches against a specific base bench.
+    ///
+    /// Usage: First provide the base benchmark name then provide benchmark names to compare against the base.
+    ///     --compare_by_name "base_name other_name1 other_name2"
+    #[clap(verbatim_doc_comment)]
+    CompareByName { filter: String },
+
+    /// Display results from the last benchmark run by:
+    ///     Comparing benches matching tag filters against a specific base bench.
+    ///
+    /// Usage: First provide the base benchmark name then provide tag filters
+    ///     --compare_by_tags "base_name db=kafka OPS=10000"
+    #[clap(verbatim_doc_comment)]
+    CompareByTags { filter: String },
+}
 
 const ABOUT: &str = r#"Bench Names:
     Each benchmark has a unique name, this name is used by many options listed below.
@@ -22,9 +73,8 @@ pub struct Args {
     #[clap(verbatim_doc_comment)]
     pub filter: Option<String>,
 
-    /// List the name of every bench.
-    #[clap(long, verbatim_doc_comment)]
-    pub list: bool,
+    #[command(subcommand)]
+    pub command: Option<Command>,
 
     /// Run a specific bench with the name produced via `--list`.
     #[clap(long, verbatim_doc_comment)]
@@ -45,19 +95,6 @@ pub struct Args {
     #[clap(long, verbatim_doc_comment)]
     pub operations_per_second: Option<u64>,
 
-    /// The results of the last benchmarks run becomes the new baseline from which following benchmark runs will be compared.
-    /// Baseline bench results are merged with the results of following results under a `baseline=true` tag.
-    #[clap(long, verbatim_doc_comment)]
-    pub set_baseline: bool,
-
-    /// Removes the stored baseline. Following runs will no longer compare against a baseline.
-    #[clap(long, verbatim_doc_comment)]
-    pub clear_baseline: bool,
-
-    /// Generate graphs webpage from the last benchmarks run.
-    #[clap(long, verbatim_doc_comment)]
-    pub generate_webpage: bool,
-
     /// By default windsock will run benches on your local machine.
     /// Set this flag to have windsock run the benches in your configured cloud.
     #[clap(long, verbatim_doc_comment)]
@@ -80,64 +117,23 @@ pub struct Args {
     #[clap(long, verbatim_doc_comment)]
     pub load_cloud_resources_file: bool,
 
-    /// Display results from the last benchmark run by:
-    ///     Comparing various benches against a specific base bench.
-    ///
-    /// Usage: First provide the base benchmark name then provide benchmark names to compare against the base.
-    ///     --compare_by_name "base_name other_name1 other_name2"
-    #[clap(long, verbatim_doc_comment)]
-    pub compare_by_name: Option<String>,
-
-    /// Display results from the last benchmark run by:
-    ///     Comparing benches matching tag filters against a specific base bench.
-    ///
-    /// Usage: First provide the base benchmark name then provide tag filters
-    ///     --compare_by_tags "base_name db=kafka OPS=10000"
-    #[clap(long, verbatim_doc_comment)]
-    pub compare_by_tags: Option<String>,
-
-    /// Display results from the last benchmark run by:
-    ///     Comparing benches with specified bench names.
-    ///
-    /// Usage: Provide benchmark names to include
-    ///     --results-by-name "name1 name2 name3"
-    #[clap(long, verbatim_doc_comment)]
-    pub results_by_name: Option<String>,
-
-    /// Display results from the last benchmark run by:
-    ///     Listing bench results matching tag filters.
-    ///
-    /// Usage: Provide tag filters
-    ///     --results-by-tags "db=kafka OPS=10000"
-    #[clap(long, verbatim_doc_comment)]
-    pub results_by_tags: Option<String>,
-
-    /// Display results from the last benchmark run by:
-    ///     Comparing all benches matching tag filters against their results in the stored baseline from `--set-baseline`.
-    ///
-    /// Usage: Provide tag filters
-    ///     --baseline-compare-by-tags "db=kafka OPS=10000"
-    #[clap(long, verbatim_doc_comment)]
-    pub baseline_compare_by_tags: Option<String>,
-
     /// Not for human use. Call this from your bench orchestration method to launch your bencher.
     #[clap(long, verbatim_doc_comment)]
     pub internal_run: Option<String>,
 
-    /// Not for human use. For nextest compatibility only.
-    #[clap(long, verbatim_doc_comment)]
+    #[clap(long, hide(true))]
+    list: bool,
+
+    #[clap(long, hide(true))]
     format: Option<NextestFormat>,
 
-    /// Not for human use. For nextest compatibility only.
-    #[clap(long, verbatim_doc_comment)]
+    #[clap(long, hide(true))]
     ignored: bool,
 
-    /// Not for human use. For nextest compatibility only.
-    #[clap(long, verbatim_doc_comment)]
+    #[clap(long, hide(true))]
     exact: bool,
 
-    /// Not for human use. For nextest compatibility only.
-    #[clap(long, verbatim_doc_comment)]
+    #[clap(long, hide(true))]
     nocapture: bool,
 }
 
@@ -147,6 +143,10 @@ enum NextestFormat {
 }
 
 impl Args {
+    pub fn nextest_list(&self) -> bool {
+        self.list
+    }
+
     pub fn nextest_list_all(&self) -> bool {
         self.list && matches!(&self.format, Some(NextestFormat::Terse)) && !self.ignored
     }

windsock/src/lib.rs

@@ -18,7 +18,7 @@ pub use tables::Goal;
 use anyhow::{anyhow, Result};
 use bench::BenchState;
 use clap::Parser;
-use cli::Args;
+use cli::{Args, Command};
 use cloud::{BenchInfo, Cloud};
 use filter::Filter;
 use std::{path::Path, process::exit};
@@ -73,27 +73,34 @@ impl<ResourcesRequired: Clone, Resources: Clone> Windsock<ResourcesRequired, Res
         let args = cli::Args::parse();
 
         let running_in_release = self.running_in_release;
+        if let Some(command) = args.command {
+            match command {
+                Command::List => list::list(&self.benches),
+                Command::BaselineSet => {
+                    ReportArchive::set_baseline();
+                    println!("Baseline set");
+                }
+                Command::BaselineClear => {
+                    ReportArchive::clear_baseline();
+                    println!("Baseline cleared");
+                }
+                Command::GenerateWebpage => {
+                    println!("Webpage generation is not implemented yet!")
+                }
+                Command::Results {
+                    ignore_baseline,
+                    filter,
+                } => tables::results(ignore_baseline, filter.as_deref().unwrap_or(""))?,
+                Command::CompareByName { filter } => tables::compare_by_name(&filter)?,
+                Command::CompareByTags { filter } => tables::compare_by_tags(&filter)?,
+            }
+            return Ok(());
+        }
         if args.cleanup_cloud_resources {
             let rt = create_runtime(None);
             rt.block_on(self.cloud.cleanup_resources());
-        } else if let Some(compare_by_name) = &args.compare_by_name {
-            tables::compare_by_name(compare_by_name)?;
-        } else if let Some(compare_by_name) = &args.results_by_name {
-            tables::results_by_name(compare_by_name)?;
-        } else if let Some(compare_by_tags) = &args.compare_by_tags {
-            tables::compare_by_tags(compare_by_tags)?;
-        } else if let Some(results_by_tags) = &args.results_by_tags {
-            tables::results_by_tags(results_by_tags)?;
-        } else if let Some(filter) = &args.baseline_compare_by_tags {
-            tables::baseline_compare_by_tags(filter)?;
-        } else if args.set_baseline {
-            ReportArchive::set_baseline();
-            println!("Baseline set");
-        } else if args.clear_baseline {
-            ReportArchive::clear_baseline();
-            println!("Baseline cleared");
-        } else if args.list {
-            list::list(&args, &self.benches);
+        } else if args.nextest_list() {
+            list::nextest_list(&args, &self.benches);
         } else if args.nextest_run_by_name() {
             create_runtime(None).block_on(self.run_nextest(args, running_in_release))?;
         } else if let Some(err) = args.nextest_invalid_args() {

windsock/src/list.rs

@@ -1,6 +1,14 @@
 use crate::{bench::BenchState, cli::Args};
 
-pub fn list<ResourcesRequired, Resources>(
+pub fn list<ResourcesRequired, Resources>(benches: &[BenchState<ResourcesRequired, Resources>]) {
+    // regular usage
+    println!("Benches:");
+    for bench in benches {
+        println!("{}", bench.tags.get_name());
+    }
+}
+
+pub fn nextest_list<ResourcesRequired, Resources>(
     args: &Args,
     benches: &[BenchState<ResourcesRequired, Resources>],
 ) {
@@ -12,10 +20,7 @@ pub fn list<ResourcesRequired, Resources>(
     } else if args.nextest_list_ignored() {
         // windsock does not support ignored tests
     } else {
-        // regular usage
-        println!("Benches:");
-        for bench in benches {
-            println!("{}", bench.tags.get_name());
-        }
+        // in case the user accidentally runs `--list` just give them the regular `list` output.
+        list(benches);
     }
 }

windsock/src/tables.rs

@@ -45,26 +45,6 @@ pub fn compare_by_name(names: &str) -> Result<()> {
     Ok(())
 }
 
-pub fn results_by_name(names: &str) -> Result<()> {
-    let archives: Result<Vec<ReportColumn>> =
-        names.split_whitespace().map(ReportColumn::load).collect();
-    display_results_table(&archives?);
-    Ok(())
-}
-
-pub fn baseline_compare_by_tags(arg: &str) -> Result<()> {
-    let filter = Filter::from_query(arg)
-        .with_context(|| format!("Failed to parse tag filter from {:?}", arg))?;
-    let archives: Result<Vec<ReportColumn>> = ReportArchive::reports_in_last_run()
-        .iter()
-        .filter(|name| filter.matches(&Tags::from_name(name)))
-        .map(|x| ReportColumn::load_with_baseline(x))
-        .collect();
-    display_baseline_compare_table(&archives?);
-
-    Ok(())
-}
-
 pub fn compare_by_tags(arg: &str) -> Result<()> {
     let mut split = arg.split_whitespace();
     let base_name = split.next().unwrap().to_owned();
@@ -100,15 +80,28 @@ pub fn compare_by_tags(arg: &str) -> Result<()> {
     Ok(())
 }
 
-pub fn results_by_tags(arg: &str) -> Result<()> {
+pub fn results(ignore_baseline: bool, arg: &str) -> Result<()> {
     let filter = Filter::from_query(arg)
         .with_context(|| format!("Failed to parse tag filter from {:?}", arg))?;
     let archives: Result<Vec<ReportColumn>> = ReportArchive::reports_in_last_run()
         .iter()
         .filter(|name| filter.matches(&Tags::from_name(name)))
-        .map(|x| ReportColumn::load(x))
+        .map(|x| {
+            if ignore_baseline {
+                ReportColumn::load(x)
+            } else {
+                ReportColumn::load_with_baseline(x)
+            }
+        })
         .collect();
-    display_results_table(&archives?);
+    let archives = archives?;
+    if archives.iter().any(|x| x.baseline.is_some()) {
+        // If there any baselines then compare against baselines
+        display_baseline_compare_table(&archives);
+    } else {
+        // Otherwise display just results without any comparison
+        display_results_table(&archives);
+    }
 
     Ok(())
 }