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/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 <database>
```
+
+## 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.
diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs
index fd7e97a7d..9034bbb79 100644
--- a/site/src/request_handlers/github.rs
+++ b/site/src/request_handlers/github.rs
@@ -5,17 +5,9 @@ use crate::github::{
};
use crate::load::SiteCtxt;
+use hashbrown::HashMap;
use std::sync::Arc;
-use regex::Regex;
-
-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(
request: github::Request,
ctxt: Arc<SiteCtxt>,
@@ -118,36 +110,57 @@ 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::<i32>().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.params.include,
+ cmd.params.exclude,
+ cmd.params.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);
}
- 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::<i32>().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;
}
}
@@ -156,25 +169,121 @@ 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?;
Ok(github::Response)
}
-/// 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<Item = (&str, regex::Captures)> {
- 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)
- })
+/// Parses the first occurrence of a `@rust-timer queue <shared-args>` command
+/// in the input string.
+fn parse_queue_command(body: &str) -> Option<Result<QueueCommand, String>> {
+ 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 params = match parse_benchmark_parameters(args) {
+ Ok(params) => params,
+ Err(error) => return Some(Err(error)),
+ };
+
+ Some(Ok(QueueCommand { params }))
+}
+
+/// Parses all occurrences of a `@rust-timer build <shared-args>` command in the input string.
+fn parse_build_commands(body: &str) -> impl Iterator<Item = Result<BuildCommand, String>> {
+ 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,
+) -> impl Iterator<Item = &'a str> + '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_start())
+}
+
+fn parse_benchmark_parameters<'a>(
+ mut args: HashMap<&'a str, &'a str>,
+) -> Result<BenchmarkParameters<'a>, 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::<u32>() else {
+ return Err(format!("Cannot parse runs {runs} as a number"));
+ };
+ params.runs = Some(runs as i32);
+ }
+
+ if !args.is_empty() {
+ Err(format!(
+ "Unknown command argument(s) `{}`",
+ args.into_keys().collect::<Vec<_>>().join(",")
+ ))
+ } else {
+ Ok(params)
+ }
+}
+
+/// Parses command arguments from a single line of text.
+/// Expects that arguments are separated by whitespace, and each argument
+/// has the format `<key>=<value>`.
+fn parse_command_arguments(args: &str) -> Result<HashMap<&str, &str>, 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> {
+ params: BenchmarkParameters<'a>,
+}
+
+#[derive(Debug)]
+struct BuildCommand<'a> {
+ sha: &'a str,
+ params: BenchmarkParameters<'a>,
+}
+
+#[derive(Debug)]
+struct BenchmarkParameters<'a> {
+ include: Option<&'a str>,
+ exclude: Option<&'a str>,
+ runs: Option<i32>,
}
pub async fn get_authorized_users() -> Result<Vec<u64>, String> {
@@ -196,23 +305,151 @@ pub async fn get_authorized_users() -> Result<Vec<u64>, String> {
#[cfg(test)]
mod tests {
use super::*;
+
+ #[test]
+ 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 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 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::<Vec<_>>();
- assert_eq!(
- shas,
- &[
- "5832462aa1d9373b24ace96ad2c50b7a18af9952",
- "23936af287657fa4148aeab40cc2a780810fae52"
- ]
- );
+ 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 queue_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 { params: BenchmarkParameters { 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(s) `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 { 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 { 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 { params: BenchmarkParameters { 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 { params: BenchmarkParameters { 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 { 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 { 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 { 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<Result<BuildCommand, String>> {
+ parse_build_commands(body).collect()
}
}