fix(cli): reject unsupported go test flags

go-runner/src/cli.rs

@@ -6,6 +6,26 @@ pub enum CliExit {
     UnknownFlag,
 }
 
+/// Whether an argument is a package pattern rather than a flag value.
+///
+/// Go flag values can be anything, so this is a heuristic based on `go help packages`:
+/// relative or absolute paths, `...` wildcards, the reserved words, and domain-style
+/// import paths (`github.com/x/y`). A bare `/` is not enough, since subtest patterns
+/// such as `-run TestX/sub` use it too, and a bare dot is not enough either (`cpu.out`).
+fn looks_like_package(arg: &str) -> bool {
+    if matches!(arg, "." | ".." | "all" | "std" | "cmd") {
+        return true;
+    }
+    if arg.starts_with("./") || arg.starts_with("../") || arg.starts_with('/') {
+        return true;
+    }
+    if arg.contains("...") {
+        return true;
+    }
+    arg.split_once('/')
+        .is_some_and(|(host, _)| host.contains('.'))
+}
+
 #[derive(Debug)]
 pub struct Cli {
     /// Run only benchmarks matching regexp
@@ -48,7 +68,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.
@@ -109,6 +130,15 @@ UNSUPPORTED FLAGS (will be warned about):
                     eprintln!(
                         "warning: flag '{s}' is not supported by CodSpeed Go runner, ignoring"
                     );
+                    if s.contains('=') {
+                        continue;
+                    }
+                    // The flag's arity is unknown. A bare token that does not look like a
+                    // package is the flag's value, and must not be collected as a package.
+                    let is_flag_value = |a: &String| !a.starts_with('-') && !looks_like_package(a);
+                    if let Some(value) = args.next_if(is_flag_value) {
+                        eprintln!("warning: ignoring value '{value}' of unsupported flag '{s}'");
+                    }
                 }
                 _ => {
                     // Collect package arguments for filtering
@@ -128,6 +158,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() {
@@ -210,9 +241,36 @@ mod tests {
 
         let result = str_to_iter("go-runner test -benchtime");
         assert!(matches!(result, Err(CliExit::MissingArgument)));
+    }
 
-        // Unknown flags now generate warnings but don't cause errors
-        let result = str_to_iter("go-runner test -unknown");
-        assert!(result.is_ok());
+    #[rstest]
+    // Value flags: the flag and its separate value are dropped.
+    #[case::separate_p("-p 1 ./... -bench Foo", &["./...", "-bench", "Foo"])]
+    #[case::count("-count 5 ./pkg", &["./pkg"])]
+    #[case::timeout("-timeout 10m ./...", &["./..."])]
+    #[case::cpu_list("-cpu 1,2,4 ./...", &["./..."])]
+    #[case::tags("-tags integration ./...", &["./..."])]
+    #[case::cpuprofile("-cpuprofile cpu.out ./...", &["./..."])]
+    #[case::run_regex("-run ^$ ./...", &["./..."])]
+    #[case::run_subtest("-run TestX/sub ./...", &["./..."])]
+    #[case::unknown_with_value("-unknown value ./...", &["./..."])]
+    // Inline values: only the single token is dropped.
+    #[case::attached_p("-p=1 ./...", &["./..."])]
+    // Boolean flags followed by a package: the package is kept.
+    #[case::benchmem("-benchmem ./...", &["./..."])]
+    #[case::verbose_then_relative("-v ./pkg", &["./pkg"])]
+    #[case::verbose_then_parent("-v ../pkg", &["../pkg"])]
+    #[case::verbose_then_absolute("-v /abs/pkg", &["/abs/pkg"])]
+    #[case::verbose_then_dot("-v .", &["."])]
+    #[case::verbose_then_wildcard("-v pkg/...", &["pkg/..."])]
+    #[case::verbose_then_import_path("-v github.com/x/y", &["github.com/x/y"])]
+    #[case::verbose_then_std("-v std", &["std"])]
+    // Boolean flag followed by another flag: nothing extra is consumed.
+    #[case::flag_then_flag("-v -bench Foo", &["./..."])]
+    // Unsupported flag at the end of the arguments.
+    #[case::trailing_flag("-p", &["./..."])]
+    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}");
     }
 }