From c7d14cb149c4fdcd10558efef2875e3fa98fe9e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 12 Oct 2024 00:47:08 +0200 Subject: [PATCH 1/5] Use a manual parser for `rust-timer queue` Instead of a regex. This allows the user to pass the arguments in an arbitrary order. --- Cargo.lock | 19 +++ site/Cargo.toml | 3 + site/src/request_handlers/github.rs | 172 +++++++++++++++++++++++++--- 3 files changed, 176 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 55d0441f5..9507460c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1257,6 +1257,18 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "insta" +version = "1.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6593a41c7a73841868772495db7dc1e8ecab43bb5c0b6da2059246c4b506ab60" +dependencies = [ + "console", + "lazy_static", + "linked-hash-map", + "similar", +] + [[package]] name = "instant" version = "0.1.12" @@ -1400,6 +1412,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linked-hash-map" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" + [[package]] name = "linux-raw-sys" version = "0.1.4" @@ -2430,6 +2448,7 @@ dependencies = [ "humansize", "hyper", "inferno", + "insta", "itertools", "jemalloc-ctl", "jemallocator", diff --git a/site/Cargo.toml b/site/Cargo.toml index 30b2994de..a11f90246 100644 --- a/site/Cargo.toml +++ b/site/Cargo.toml @@ -57,3 +57,6 @@ jemalloc-ctl = "0.5" serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } toml = "0.7" + +[dev-dependencies] +insta = "1.40.0" diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index fd7e97a7d..e48e2e6e1 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -5,15 +5,13 @@ use crate::github::{ }; use crate::load::SiteCtxt; -use std::sync::Arc; - +use hashbrown::HashMap; use regex::Regex; +use std::sync::Arc; lazy_static::lazy_static! { static ref BODY_TIMER_BUILD: Regex = Regex::new(r"(?:\W|^)@rust-timer\s+build\s+(\w+)(?:\W|$)(?:include=(\S+))?\s*(?:exclude=(\S+))?\s*(?:runs=(\d+))?").unwrap(); - static ref BODY_TIMER_QUEUE: Regex = - Regex::new(r"(?:\W|^)@rust-timer\s+queue(?:\W|$)(?:include=(\S+))?\s*(?:exclude=(\S+))?\s*(?:runs=(\d+))?").unwrap(); } pub async fn handle_github( @@ -118,26 +116,25 @@ async fn handle_rust_timer( return Ok(github::Response); } - if let Some(captures) = BODY_TIMER_QUEUE.captures(&comment.body) { - let include = captures.get(1).map(|v| v.as_str()); - let exclude = captures.get(2).map(|v| v.as_str()); - let runs = captures.get(3).and_then(|v| v.as_str().parse::().ok()); - { - let conn = ctxt.conn().await; - conn.queue_pr(issue.number, include, exclude, runs).await; - } - main_client - .post_comment( - issue.number, + if let Some(queue) = parse_queue_command(&comment.body) { + let msg = match queue { + Ok(cmd) => { + let conn = ctxt.conn().await; + conn.queue_pr(issue.number, cmd.include, cmd.exclude, cmd.runs) + .await; format!( "Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf {COMMENT_MARK_TEMPORARY}" - ), - ) - .await; + ) + } + Err(error) => { + format!("Error occurred while parsing comment: {error}") + } + }; + main_client.post_comment(issue.number, msg).await; return Ok(github::Response); } @@ -163,6 +160,68 @@ async fn handle_rust_timer( Ok(github::Response) } +/// Parses the first occurrence of a `@rust-timer queue <...>` command +/// in the input string. +fn parse_queue_command(body: &str) -> Option> { + let prefix = "@rust-timer"; + let bot_line = body.lines().find_map(|line| { + let line = line.trim(); + line.find(prefix) + .map(|index| line[index + prefix.len()..].trim()) + })?; + + let args = bot_line.strip_prefix("queue").map(|l| l.trim())?; + let mut args = match parse_command_arguments(args) { + Ok(args) => args, + Err(error) => return Some(Err(error)), + }; + let mut cmd = QueueCommand { + include: args.remove("include"), + exclude: args.remove("exclude"), + runs: None, + }; + if let Some(runs) = args.remove("runs") { + let Ok(runs) = runs.parse::() else { + return Some(Err(format!("Cannot parse runs {runs} as a number"))); + }; + cmd.runs = Some(runs as i32); + } + + if let Some((key, _)) = args.into_iter().next() { + return Some(Err(format!("Unknown command argument {key}"))); + } + + Some(Ok(cmd)) +} + +/// Parses command arguments from a single line of text. +/// Expects that arguments are separated by whitespace, and each argument +/// has the format `=`. +fn parse_command_arguments(args: &str) -> Result, String> { + let mut argmap = HashMap::new(); + for arg in args.split_whitespace() { + let Some((key, value)) = arg.split_once('=') else { + return Err(format!( + "Invalid command argument `{arg}` (there may be no spaces around the `=` character)" + )); + }; + let key = key.trim(); + let value = value.trim(); + if argmap.insert(key, value).is_some() { + return Err(format!("Duplicate command argument `{key}`")); + } + } + + Ok(argmap) +} + +#[derive(Debug)] +struct QueueCommand<'a> { + include: Option<&'a str>, + exclude: Option<&'a str>, + runs: Option, +} + /// Run the `@rust-timer build` regex over the comment message extracting the commit and the other captures fn build_captures(comment_body: &str) -> impl Iterator { BODY_TIMER_BUILD @@ -196,6 +255,7 @@ pub async fn get_authorized_users() -> Result, String> { #[cfg(test)] mod tests { use super::*; + #[test] fn captures_all_shas() { let comment_body = r#" @@ -215,4 +275,80 @@ Going to do perf runs for a few of these: ] ); } + + #[test] + fn command_missing() { + assert!(parse_queue_command("").is_none()); + } + + #[test] + fn unknown_command() { + assert!(parse_queue_command("@rust-timer foo").is_none()); + } + + #[test] + fn queue_command() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue"), + @"Some(Ok(QueueCommand { include: None, exclude: None, runs: None }))"); + } + + #[test] + fn queue_command_unknown_arg() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue foo=bar"), + @r###"Some(Err("Unknown command argument foo"))"###); + } + + #[test] + fn queue_command_duplicate_arg() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=a exclude=c include=b"), + @r###"Some(Err("Duplicate command argument `include`"))"###); + } + + #[test] + fn queue_command_include() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=abcd,feih"), + @r###"Some(Ok(QueueCommand { include: Some("abcd,feih"), exclude: None, runs: None }))"###); + } + + #[test] + fn queue_command_exclude() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue exclude=foo134,barzbaz41baf"), + @r###"Some(Ok(QueueCommand { include: None, exclude: Some("foo134,barzbaz41baf"), runs: None }))"###); + } + + #[test] + fn queue_command_runs() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=5"), + @"Some(Ok(QueueCommand { include: None, exclude: None, runs: Some(5) }))"); + } + + #[test] + fn queue_command_runs_nan() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=xxx"), + @r###"Some(Err("Cannot parse runs xxx as a number"))"###); + } + + #[test] + fn queue_command_combination() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=acda,13asd exclude=c13,DA runs=5"), + @r###"Some(Ok(QueueCommand { include: Some("acda,13asd"), exclude: Some("c13,DA"), runs: Some(5) }))"###); + } + + #[test] + fn queue_command_argument_spaces() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include = abcd,das"), + @r###"Some(Err("Invalid command argument `include` (there may be no spaces around the `=` character)"))"###); + } + + #[test] + fn queue_command_spaces() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=abcd,das "), + @r###"Some(Ok(QueueCommand { include: Some("abcd,das"), exclude: None, runs: None }))"###); + } + + #[test] + fn queue_command_with_bors() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@bors try @rust-timer queue include=foo,bar"), + @r###"Some(Ok(QueueCommand { include: Some("foo,bar"), exclude: None, runs: None }))"###); + } } From 1c0968bfa384d586e29d9e3f70008b3d784cb9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 12 Oct 2024 14:04:18 +0200 Subject: [PATCH 2/5] Document our usage of `insta` --- site/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/site/README.md b/site/README.md index 305845a28..aece864fc 100644 --- a/site/README.md +++ b/site/README.md @@ -46,3 +46,8 @@ the `PORT` environment variable. ```console $ ./target/release/site ``` + +## Development +We use [insta](https://github.com/mitsuhiko/insta) for snapshot testing. If any tests that use the +`insta` macros fail, you should `cargo install cargo-insta` and then run `cargo insta review` to review +the snapshot changes. From bb5711d6455705013ff405cd2e8b75b591ca056b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 12 Oct 2024 14:18:07 +0200 Subject: [PATCH 3/5] Add test and refactor parsing slightly --- site/src/request_handlers/github.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index e48e2e6e1..b02f06f49 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -165,7 +165,6 @@ async fn handle_rust_timer( fn parse_queue_command(body: &str) -> Option> { let prefix = "@rust-timer"; let bot_line = body.lines().find_map(|line| { - let line = line.trim(); line.find(prefix) .map(|index| line[index + prefix.len()..].trim()) })?; @@ -187,8 +186,11 @@ fn parse_queue_command(body: &str) -> Option> { cmd.runs = Some(runs as i32); } - if let Some((key, _)) = args.into_iter().next() { - return Some(Err(format!("Unknown command argument {key}"))); + if !args.is_empty() { + return Some(Err(format!( + "Unknown command argument(s) `{}`", + args.into_keys().collect::>().join(",") + ))); } Some(Ok(cmd)) @@ -295,7 +297,7 @@ Going to do perf runs for a few of these: #[test] fn queue_command_unknown_arg() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue foo=bar"), - @r###"Some(Err("Unknown command argument foo"))"###); + @r###"Some(Err("Unknown command argument(s) `foo`"))"###); } #[test] @@ -351,4 +353,10 @@ Going to do perf runs for a few of these: insta::assert_compact_debug_snapshot!(parse_queue_command("@bors try @rust-timer queue include=foo,bar"), @r###"Some(Ok(QueueCommand { include: Some("foo,bar"), exclude: None, runs: None }))"###); } + + #[test] + fn queue_command_parameter_order() { + insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=3 exclude=c,a include=b"), + @r###"Some(Ok(QueueCommand { include: Some("b"), exclude: Some("c,a"), runs: Some(3) }))"###); + } } From b6609023c84cfeb1588748f055848bb5aaebdeb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 13 Oct 2024 08:49:40 +0200 Subject: [PATCH 4/5] Refactor `queue` parsing --- site/src/request_handlers/github.rs | 81 ++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 26 deletions(-) diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index b02f06f49..c4ddb9191 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -120,8 +120,13 @@ async fn handle_rust_timer( let msg = match queue { Ok(cmd) => { let conn = ctxt.conn().await; - conn.queue_pr(issue.number, cmd.include, cmd.exclude, cmd.runs) - .await; + conn.queue_pr( + issue.number, + cmd.params.include, + cmd.params.exclude, + cmd.params.runs, + ) + .await; format!( "Awaiting bors try build completion. @@ -160,40 +165,59 @@ async fn handle_rust_timer( Ok(github::Response) } -/// Parses the first occurrence of a `@rust-timer queue <...>` command +/// Parses the first occurrence of a `@rust-timer queue ` command /// in the input string. fn parse_queue_command(body: &str) -> Option> { - let prefix = "@rust-timer"; - let bot_line = body.lines().find_map(|line| { - line.find(prefix) - .map(|index| line[index + prefix.len()..].trim()) - })?; - - let args = bot_line.strip_prefix("queue").map(|l| l.trim())?; - let mut args = match parse_command_arguments(args) { + let args = get_command_lines(body, "queue").next()?; + let args = match parse_command_arguments(args) { Ok(args) => args, Err(error) => return Some(Err(error)), }; - let mut cmd = QueueCommand { + let params = match parse_benchmark_parameters(args) { + Ok(params) => params, + Err(error) => return Some(Err(error)), + }; + + Some(Ok(QueueCommand { params })) +} + +fn get_command_lines<'a: 'b, 'b>( + body: &'a str, + command: &'b str, +) -> impl Iterator + 'b { + let prefix = "@rust-timer"; + body.lines() + .filter_map(move |line| { + line.find(prefix) + .map(|index| line[index + prefix.len()..].trim()) + }) + .filter_map(move |line| line.strip_prefix(command)) + .map(move |l| l.trim()) +} + +fn parse_benchmark_parameters<'a>( + mut args: HashMap<&'a str, &'a str>, +) -> Result, String> { + let mut params = BenchmarkParameters { include: args.remove("include"), exclude: args.remove("exclude"), runs: None, }; if let Some(runs) = args.remove("runs") { let Ok(runs) = runs.parse::() else { - return Some(Err(format!("Cannot parse runs {runs} as a number"))); + return Err(format!("Cannot parse runs {runs} as a number")); }; - cmd.runs = Some(runs as i32); + params.runs = Some(runs as i32); } if !args.is_empty() { - return Some(Err(format!( + Err(format!( "Unknown command argument(s) `{}`", args.into_keys().collect::>().join(",") - ))); + )) + } else { + Ok(params) } - - Some(Ok(cmd)) } /// Parses command arguments from a single line of text. @@ -219,6 +243,11 @@ fn parse_command_arguments(args: &str) -> Result, String> { #[derive(Debug)] struct QueueCommand<'a> { + params: BenchmarkParameters<'a>, +} + +#[derive(Debug)] +struct BenchmarkParameters<'a> { include: Option<&'a str>, exclude: Option<&'a str>, runs: Option, @@ -291,7 +320,7 @@ Going to do perf runs for a few of these: #[test] fn queue_command() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue"), - @"Some(Ok(QueueCommand { include: None, exclude: None, runs: None }))"); + @"Some(Ok(QueueCommand { params: BenchmarkParameters { include: None, exclude: None, runs: None } }))"); } #[test] @@ -309,19 +338,19 @@ Going to do perf runs for a few of these: #[test] fn queue_command_include() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=abcd,feih"), - @r###"Some(Ok(QueueCommand { include: Some("abcd,feih"), exclude: None, runs: None }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("abcd,feih"), exclude: None, runs: None } }))"###); } #[test] fn queue_command_exclude() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue exclude=foo134,barzbaz41baf"), - @r###"Some(Ok(QueueCommand { include: None, exclude: Some("foo134,barzbaz41baf"), runs: None }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: None, exclude: Some("foo134,barzbaz41baf"), runs: None } }))"###); } #[test] fn queue_command_runs() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=5"), - @"Some(Ok(QueueCommand { include: None, exclude: None, runs: Some(5) }))"); + @"Some(Ok(QueueCommand { params: BenchmarkParameters { include: None, exclude: None, runs: Some(5) } }))"); } #[test] @@ -333,7 +362,7 @@ Going to do perf runs for a few of these: #[test] fn queue_command_combination() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=acda,13asd exclude=c13,DA runs=5"), - @r###"Some(Ok(QueueCommand { include: Some("acda,13asd"), exclude: Some("c13,DA"), runs: Some(5) }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("acda,13asd"), exclude: Some("c13,DA"), runs: Some(5) } }))"###); } #[test] @@ -345,18 +374,18 @@ Going to do perf runs for a few of these: #[test] fn queue_command_spaces() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue include=abcd,das "), - @r###"Some(Ok(QueueCommand { include: Some("abcd,das"), exclude: None, runs: None }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("abcd,das"), exclude: None, runs: None } }))"###); } #[test] fn queue_command_with_bors() { insta::assert_compact_debug_snapshot!(parse_queue_command("@bors try @rust-timer queue include=foo,bar"), - @r###"Some(Ok(QueueCommand { include: Some("foo,bar"), exclude: None, runs: None }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("foo,bar"), exclude: None, runs: None } }))"###); } #[test] fn queue_command_parameter_order() { insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=3 exclude=c,a include=b"), - @r###"Some(Ok(QueueCommand { include: Some("b"), exclude: Some("c,a"), runs: Some(3) }))"###); + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("b"), exclude: Some("c,a"), runs: Some(3) } }))"###); } } From f3bcb8b7d6faca0f3a2d4a966fa39fb901750b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 13 Oct 2024 09:17:42 +0200 Subject: [PATCH 5/5] Refactor `build` command parsing --- site/src/request_handlers/github.rs | 160 +++++++++++++++++++--------- 1 file changed, 112 insertions(+), 48 deletions(-) diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index c4ddb9191..9034bbb79 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -6,14 +6,8 @@ use crate::github::{ use crate::load::SiteCtxt; use hashbrown::HashMap; -use regex::Regex; use std::sync::Arc; -lazy_static::lazy_static! { - static ref BODY_TIMER_BUILD: Regex = - Regex::new(r"(?:\W|^)@rust-timer\s+build\s+(\w+)(?:\W|$)(?:include=(\S+))?\s*(?:exclude=(\S+))?\s*(?:runs=(\d+))?").unwrap(); -} - pub async fn handle_github( request: github::Request, ctxt: Arc, @@ -143,13 +137,30 @@ async fn handle_rust_timer( return Ok(github::Response); } - for captures in build_captures(&comment.body).map(|(_, captures)| captures) { - let include = captures.get(2).map(|v| v.as_str()); - let exclude = captures.get(3).map(|v| v.as_str()); - let runs = captures.get(4).and_then(|v| v.as_str().parse::().ok()); - { - let conn = ctxt.conn().await; - conn.queue_pr(issue.number, include, exclude, runs).await; + let build_cmds: Vec<_> = parse_build_commands(&comment.body).collect(); + let mut valid_build_cmds = vec![]; + let mut errors = String::new(); + for cmd in build_cmds { + match cmd { + Ok(cmd) => valid_build_cmds.push(cmd), + Err(error) => errors.push_str(&format!("Cannot parse build command: {error}\n")), + } + } + if !errors.is_empty() { + main_client.post_comment(issue.number, errors).await; + return Ok(github::Response); + } + + { + let conn = ctxt.conn().await; + for command in &valid_build_cmds { + conn.queue_pr( + issue.number, + command.params.include, + command.params.exclude, + command.params.runs, + ) + .await; } } @@ -158,7 +169,7 @@ async fn handle_rust_timer( main_client, ci_client, issue.number, - build_captures(&comment.body).map(|(commit, _)| commit), + valid_build_cmds.iter().map(|c| c.sha), ) .await?; @@ -181,6 +192,22 @@ fn parse_queue_command(body: &str) -> Option> { Some(Ok(QueueCommand { params })) } +/// Parses all occurrences of a `@rust-timer build ` command in the input string. +fn parse_build_commands(body: &str) -> impl Iterator> { + get_command_lines(body, "build").map(|line| { + let mut iter = line.splitn(2, ' '); + let Some(sha) = iter.next().filter(|s| !s.is_empty() && !s.contains('=')) else { + return Err("Missing SHA in build command".to_string()); + }; + + let sha = sha.trim_start_matches("https://github.com/rust-lang/rust/commit/"); + let args = iter.next().unwrap_or(""); + let args = parse_command_arguments(args)?; + let params = parse_benchmark_parameters(args)?; + Ok(BuildCommand { sha, params }) + }) +} + fn get_command_lines<'a: 'b, 'b>( body: &'a str, command: &'b str, @@ -192,7 +219,7 @@ fn get_command_lines<'a: 'b, 'b>( .map(|index| line[index + prefix.len()..].trim()) }) .filter_map(move |line| line.strip_prefix(command)) - .map(move |l| l.trim()) + .map(move |l| l.trim_start()) } fn parse_benchmark_parameters<'a>( @@ -246,6 +273,12 @@ struct QueueCommand<'a> { params: BenchmarkParameters<'a>, } +#[derive(Debug)] +struct BuildCommand<'a> { + sha: &'a str, + params: BenchmarkParameters<'a>, +} + #[derive(Debug)] struct BenchmarkParameters<'a> { include: Option<&'a str>, @@ -253,20 +286,6 @@ struct BenchmarkParameters<'a> { runs: Option, } -/// Run the `@rust-timer build` regex over the comment message extracting the commit and the other captures -fn build_captures(comment_body: &str) -> impl Iterator { - BODY_TIMER_BUILD - .captures_iter(comment_body) - .filter_map(|captures| { - captures.get(1).map(|m| { - let commit = m - .as_str() - .trim_start_matches("https://github.com/rust-lang/rust/commit/"); - (commit, captures) - }) - }) -} - pub async fn get_authorized_users() -> Result, String> { let url = format!("{}/permissions/perf.json", ::rust_team_data::v1::BASE_URL); let client = reqwest::Client::new(); @@ -288,32 +307,62 @@ mod tests { use super::*; #[test] - fn captures_all_shas() { - let comment_body = r#" -Going to do perf runs for a few of these: - -@rust-timer build 5832462aa1d9373b24ace96ad2c50b7a18af9952 (https://github.com/rust-lang/rust/pull/100307) -@rust-timer build 23936af287657fa4148aeab40cc2a780810fae52 (https://github.com/rust-lang/rust/pull/100392) - "#; - let shas = build_captures(comment_body) - .map(|(c, _)| c) - .collect::>(); - assert_eq!( - shas, - &[ - "5832462aa1d9373b24ace96ad2c50b7a18af9952", - "23936af287657fa4148aeab40cc2a780810fae52" - ] - ); + fn build_command_missing() { + assert!(get_build_commands("").is_empty()); + } + + #[test] + fn build_unknown_command() { + assert!(get_build_commands("@rust-timer foo").is_empty()); } #[test] - fn command_missing() { + fn build_command_missing_sha() { + insta::assert_compact_debug_snapshot!(get_build_commands("@rust-timer build"), + @r###"[Err("Missing SHA in build command")]"###); + } + + #[test] + fn build_command() { + insta::assert_compact_debug_snapshot!(get_build_commands("@rust-timer build 5832462aa1d9373b24ace96ad2c50b7a18af9952"), + @r###"[Ok(BuildCommand { sha: "5832462aa1d9373b24ace96ad2c50b7a18af9952", params: BenchmarkParameters { include: None, exclude: None, runs: None } })]"###); + } + + #[test] + fn build_command_multiple() { + insta::assert_compact_debug_snapshot!(get_build_commands(r#" +@rust-timer build 5832462aa1d9373b24ace96ad2c50b7a18af9952 +@rust-timer build 23936af287657fa4148aeab40cc2a780810fae52 +"#), + @r###"[Ok(BuildCommand { sha: "5832462aa1d9373b24ace96ad2c50b7a18af9952", params: BenchmarkParameters { include: None, exclude: None, runs: None } }), Ok(BuildCommand { sha: "23936af287657fa4148aeab40cc2a780810fae52", params: BenchmarkParameters { include: None, exclude: None, runs: None } })]"###); + } + + #[test] + fn build_command_unknown_arg() { + insta::assert_compact_debug_snapshot!(get_build_commands("@rust-timer build foo=bar"), + @r###"[Err("Missing SHA in build command")]"###); + } + + #[test] + fn build_command_complex() { + insta::assert_compact_debug_snapshot!(get_build_commands(" @rust-timer build sha123456 exclude=baz include=foo,bar runs=4"), + @r###"[Ok(BuildCommand { sha: "sha123456", params: BenchmarkParameters { include: Some("foo,bar"), exclude: Some("baz"), runs: Some(4) } })]"###); + } + + #[test] + fn build_command_link() { + insta::assert_compact_debug_snapshot!(get_build_commands(r#" +@rust-timer build https://github.com/rust-lang/rust/commit/323f521bc6d8f2b966ba7838a3f3ee364e760b7e"#), + @r###"[Ok(BuildCommand { sha: "323f521bc6d8f2b966ba7838a3f3ee364e760b7e", params: BenchmarkParameters { include: None, exclude: None, runs: None } })]"###); + } + + #[test] + fn queue_command_missing() { assert!(parse_queue_command("").is_none()); } #[test] - fn unknown_command() { + fn queue_unknown_command() { assert!(parse_queue_command("@rust-timer foo").is_none()); } @@ -388,4 +437,19 @@ Going to do perf runs for a few of these: insta::assert_compact_debug_snapshot!(parse_queue_command("@rust-timer queue runs=3 exclude=c,a include=b"), @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("b"), exclude: Some("c,a"), runs: Some(3) } }))"###); } + + #[test] + fn queue_command_multiline() { + insta::assert_compact_debug_snapshot!(parse_queue_command(r#"Ok, this looks good now. +Let's do a perf run quickly and then we can merge it. + +@bors try @rust-timer queue include=foo,bar + +Otherwise LGTM."#), + @r###"Some(Ok(QueueCommand { params: BenchmarkParameters { include: Some("foo,bar"), exclude: None, runs: None } }))"###); + } + + fn get_build_commands(body: &str) -> Vec> { + parse_build_commands(body).collect() + } }