fix(cli): drop the value of unsupported go test flags

go-runner/src/cli.rs

@@ -1,3 +1,5 @@
+use std::iter::Peekable;
+
 #[derive(Debug)]
 pub enum CliExit {
     Help,
@@ -6,6 +8,159 @@ pub enum CliExit {
     UnknownFlag,
 }
 
+/// A `go test` flag, classified by whether it takes a value.
+///
+/// Flag names follow cmd/go/internal/{base/flag.go, work/build.go, test/testflag.go}.
+enum Flag<'a> {
+    /// Takes no value.
+    Boolean,
+    /// Takes one value, attached (`-p=1`) or as the next argument (`-p 1`).
+    Value(Option<&'a str>),
+    /// Not a `go test` flag, e.g. a custom flag of the test binary.
+    Unknown(Option<&'a str>),
+}
+
+impl<'a> Flag<'a> {
+    /// Classifies `arg`, or returns `None` if it is not a flag.
+    fn from_arg(arg: &'a str) -> Option<Self> {
+        if Self::parse_bool_flag(arg).is_some() {
+            return Some(Self::Boolean);
+        }
+        if let Some(value) = Self::parse_value_flag(arg) {
+            return Some(Self::Value(value));
+        }
+        let (_, value) = Self::split(arg)?;
+        Some(Self::Unknown(value))
+    }
+
+    /// Parses a `go test` flag that takes a value and returns its attached value (`-p=1`), or
+    /// `Some(None)` when the value is the next argument (`-p 1`). Returns `None` if `arg` is
+    /// not a known value flag. Like [`Self::parse_bool_flag`], it accepts `--` and `test.`.
+    fn parse_value_flag(arg: &str) -> Option<Option<&str>> {
+        const VALUE_FLAGS: &[&str] = &[
+            "asmflags",
+            "bench",
+            "benchtime",
+            "blockprofile",
+            "blockprofilerate",
+            "buildmode",
+            "C",
+            "compiler",
+            "count",
+            "covermode",
+            "coverpkg",
+            "coverprofile",
+            "cpu",
+            "cpuprofile",
+            "debug-actiongraph",
+            "debug-runtime-trace",
+            "debug-trace",
+            "exec",
+            "fuzz",
+            "fuzzminimizetime",
+            "fuzztime",
+            "gccgoflags",
+            "gcflags",
+            "installsuffix",
+            "ldflags",
+            "list",
+            "memprofile",
+            "memprofilerate",
+            "mod",
+            "modfile",
+            "mutexprofile",
+            "mutexprofilefraction",
+            "o",
+            "outputdir",
+            "overlay",
+            "p",
+            "parallel",
+            "pgo",
+            "pkgdir",
+            "run",
+            "shuffle",
+            "skip",
+            "tags",
+            "timeout",
+            "toolexec",
+            "trace",
+            "vet",
+        ];
+        let (name, value) = Self::split(arg)?;
+        VALUE_FLAGS.contains(&name).then_some(value)
+    }
+
+    /// Parses a boolean `go test` flag and returns its value, or `None` if `arg` is not a
+    /// known boolean flag or has an invalid value. Accepted spellings:
+    ///
+    /// - `-v`, `--v`: bare, means true
+    /// - `-v=true`, `--v=true`: also `1`, `t`, `T`, `TRUE`, `True`
+    /// - `-v=false`, `--v=false`: also `0`, `f`, `F`, `FALSE`, `False`
+    /// - `-test.v`, `--test.v`, `-test.v=true`: the same flag under the test binary's name
+    ///
+    /// `-v true` is not a form: Go reads `true` as a separate argument, not as the value.
+    fn parse_bool_flag(arg: &str) -> Option<bool> {
+        const BOOL_FLAGS: &[&str] = &[
+            "a",
+            "artifacts",
+            "asan",
+            "benchmem",
+            "buildvcs",
+            "c",
+            "cover",
+            "failfast",
+            "fullpath",
+            "json",
+            "linkshared",
+            "modcacherw",
+            "msan",
+            "n",
+            "race",
+            "short",
+            "trimpath",
+            "v",
+            "work",
+            "x",
+        ];
+        // Spellings accepted by Go's `strconv.ParseBool`.
+        const TRUE_VALUES: &[&str] = &["1", "t", "T", "TRUE", "true", "True"];
+        const FALSE_VALUES: &[&str] = &["0", "f", "F", "FALSE", "false", "False"];
+
+        let (name, value) = Self::split(arg)?;
+        if !BOOL_FLAGS.contains(&name) {
+            return None;
+        }
+        match value {
+            None => Some(true),
+            Some(value) if TRUE_VALUES.contains(&value) => Some(true),
+            Some(value) if FALSE_VALUES.contains(&value) => Some(false),
+            Some(_) => None,
+        }
+    }
+
+    /// Consumes the flag's value from `args` when it is a separate argument.
+    fn consume_value(self, args: &mut Peekable<impl Iterator<Item = String>>) -> Option<String> {
+        match self {
+            Self::Boolean | Self::Value(Some(_)) | Self::Unknown(Some(_)) => None,
+            Self::Value(None) => args.next(),
+            // Like Go, treat a dash-leading argument after an unknown flag as a flag.
+            Self::Unknown(None) => args.next_if(|arg| !arg.starts_with('-')),
+        }
+    }
+
+    /// Splits `-name`, `--name` or `-test.name`, with an optional `=value`, into name and value.
+    fn split(arg: &str) -> Option<(&str, Option<&str>)> {
+        let name = arg.strip_prefix('-')?;
+        let name = name.strip_prefix('-').unwrap_or(name);
+        let (name, value) = match name.split_once('=') {
+            Some((name, value)) => (name, Some(value)),
+            None => (name, None),
+        };
+        let name = name.strip_prefix("test.").unwrap_or(name);
+        Some((name, value))
+    }
+}
+
 #[derive(Debug)]
 pub struct Cli {
     /// Run only benchmarks matching regexp
@@ -48,7 +203,8 @@ impl Cli {
     ///
     /// We can't do this with clap/structopt, because they only support single dashes for single-letter
     /// flags (e.g., `-h`), and double dashes for multi-letter flags (e.g., `--help`).
-    fn parse_args(mut args: impl Iterator<Item = String>) -> Result<Self, CliExit> {
+    fn parse_args(args: impl Iterator<Item = String>) -> Result<Self, CliExit> {
+        let mut args = args.peekable();
         let mut instance = Self::default();
 
         // We currently only support the `test` subcommand.
@@ -106,9 +262,11 @@ UNSUPPORTED FLAGS (will be warned about):
                     instance.benchtime = s.split_once('=').unwrap().1.to_string();
                 }
                 s if s.starts_with('-') => {
-                    eprintln!(
-                        "warning: flag '{s}' is not supported by CodSpeed Go runner, ignoring"
-                    );
+                    log::warn!("flag '{s}' is not supported by CodSpeed Go runner, ignoring");
+                    let value = Flag::from_arg(s).and_then(|flag| flag.consume_value(&mut args));
+                    if let Some(value) = value {
+                        log::warn!("ignoring value '{value}' of unsupported flag '{s}'");
+                    }
                 }
                 _ => {
                     // Collect package arguments for filtering
@@ -128,6 +286,7 @@ UNSUPPORTED FLAGS (will be warned about):
 #[cfg(test)]
 mod tests {
     use super::*;
+    use rstest::rstest;
 
     fn str_to_iter(cmd: &str) -> Result<Cli, CliExit> {
         let args: Vec<String> = if cmd.trim().is_empty() {
@@ -215,4 +374,53 @@ mod tests {
         let result = str_to_iter("go-runner test -unknown");
         assert!(result.is_ok());
     }
+
+    #[rstest]
+    #[case::separate_p("-p 1 ./... -bench Foo", &["./...", "-bench", "Foo"])]
+    #[case::attached_p("-p=1 ./...", &["./..."])]
+    #[case::path_value("-cpuprofile ./cpu.out ./...", &["./..."])]
+    #[case::subtest_value("-run TestX/sub ./...", &["./..."])]
+    #[case::bool_then_package("-v fmt", &["fmt"])]
+    #[case::test_prefixed_bool("-test.v fmt", &["fmt"])]
+    #[case::bool_then_flag("-v -bench Foo", &["./..."])]
+    #[case::trailing_flag("-p", &["./..."])]
+    #[case::unknown_then_flag("-custom -timeout 5m ./pkg", &["./pkg"])]
+    fn test_cli_drops_unsupported_flags(#[case] args: &str, #[case] packages: &[&str]) {
+        let cli = str_to_iter(&format!("go-runner test {args}")).unwrap();
+        assert_eq!(cli.packages, packages, "{args}");
+    }
+
+    #[rstest]
+    #[case::bare("-v", Some(true))]
+    #[case::double_dash("--v", Some(true))]
+    #[case::true_value("-v=true", Some(true))]
+    #[case::one_value("-v=1", Some(true))]
+    #[case::upper_true_value("-v=TRUE", Some(true))]
+    #[case::false_value("--v=false", Some(false))]
+    #[case::short_false_value("-short=f", Some(false))]
+    #[case::test_prefix("-test.v", Some(true))]
+    #[case::test_prefix_double_dash("--test.v", Some(true))]
+    #[case::test_prefix_value("-test.short=false", Some(false))]
+    #[case::invalid_value("-v=maybe", None)]
+    #[case::empty_value("-v=", None)]
+    #[case::triple_dash("---v", None)]
+    #[case::value_flag("-p", None)]
+    #[case::value_flag_with_value("-p=1", None)]
+    #[case::unknown("-integration", None)]
+    #[case::not_a_flag("v", None)]
+    fn test_parse_bool_flag(#[case] arg: &str, #[case] expected: Option<bool>) {
+        assert_eq!(Flag::parse_bool_flag(arg), expected, "{arg}");
+    }
+
+    // Whole-token arguments: `str_to_iter` would split "-s -w" in two.
+    #[rstest]
+    #[case::ldflags("-ldflags", "-s -w")]
+    #[case::double_dash_gcflags("--gcflags", "-N")]
+    #[case::run("-run", "-x")]
+    #[case::test_prefixed_run("-test.run", "-bench")]
+    fn test_cli_drops_dash_leading_value(#[case] flag: &str, #[case] value: &str) {
+        let args = ["test", flag, value, "./pkg"].map(String::from);
+        let cli = Cli::parse_args(args.into_iter()).unwrap();
+        assert_eq!(cli.packages, ["./pkg"], "{flag} {value}");
+    }
 }