diff --git a/go-runner/src/cli.rs b/go-runner/src/cli.rs index c10a589..e72470a 100644 --- a/go-runner/src/cli.rs +++ b/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) -> Result { + fn parse_args(args: impl Iterator) -> Result { + 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 { let args: Vec = 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}"); } }