Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 69 additions & 9 deletions src/bors/command/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub enum CommandParseError {
MissingCommand,
UnknownCommand(String),
MissingArgValue { arg: String },
UnknownArg(String),
UnknownArg { arg: String, did_you_mean: String },
DuplicateArg(String),
ValidationError(String),
}
Expand Down Expand Up @@ -270,7 +270,10 @@ fn parser_try(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResu
for part in parts {
match part {
CommandPart::Bare(key) => {
return Some(Err(CommandParseError::UnknownArg(key.to_string())));
return Some(Err(CommandParseError::UnknownArg {
arg: key.to_string(),
did_you_mean: "jobs=<jobs>|parent=<parent>".to_string(),
}));
}
CommandPart::KeyValue { key, value } => match (*key, *value) {
("parent", "last") => parent = Some(Parent::Last),
Expand Down Expand Up @@ -299,7 +302,10 @@ fn parser_try(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResu
jobs = raw_jobs;
}
_ => {
return Some(Err(CommandParseError::UnknownArg(key.to_string())));
return Some(Err(CommandParseError::UnknownArg {
arg: key.to_string(),
did_you_mean: "jobs=<jobs>|parent=<parent>".to_string(),
}));
}
},
}
Expand All @@ -318,11 +324,24 @@ fn parser_try_cancel(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> Pa
}

/// Parses `@bors delegate=<try|review>` or `@bors delegate+`.
fn parser_delegate(command: &CommandPart<'_>, _parts: &[CommandPart<'_>]) -> ParseResult {
fn parser_delegate(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResult {
match command {
CommandPart::Bare("delegate+") => {
Some(Ok(BorsCommand::SetDelegate(DelegatedPermission::Review)))
}
CommandPart::Bare("delegate") => {
if !parts.is_empty() {
Some(Err(CommandParseError::UnknownArg {
arg: match parts[0] {
CommandPart::Bare(arg) => arg.to_owned(),
CommandPart::KeyValue { key, .. } => key.to_owned(),
},
did_you_mean: "delegate=<try|review>".to_string(),
}))
} else {
Some(Ok(BorsCommand::SetDelegate(DelegatedPermission::Review)))
}
}
CommandPart::KeyValue {
key: "delegate",
value,
Expand Down Expand Up @@ -1163,15 +1182,31 @@ for the crater",
#[test]
fn parse_try_unknown_arg() {
let cmds = parse_commands("@bors try a");
assert_eq!(cmds.len(), 1);
assert_eq!(cmds[0], Err(CommandParseError::UnknownArg("a".to_string())));
insta::assert_debug_snapshot!(cmds, @r#"
[
Err(
UnknownArg {
arg: "a",
did_you_mean: "jobs=<jobs>|parent=<parent>",
},
),
]
"#);
}

#[test]
fn parse_try_unknown_kv_arg() {
let cmds = parse_commands("@bors try a=b");
assert_eq!(cmds.len(), 1);
assert_eq!(cmds[0], Err(CommandParseError::UnknownArg("a".to_string())));
insta::assert_debug_snapshot!(cmds, @r#"
[
Err(
UnknownArg {
arg: "a",
did_you_mean: "jobs=<jobs>|parent=<parent>",
},
),
]
"#);
}

#[test]
Expand Down Expand Up @@ -1228,7 +1263,7 @@ for the crater",
}

#[test]
fn parse_delegate_author() {
fn parse_delegate_plus_author() {
let cmds = parse_commands("@bors delegate+");
assert_eq!(cmds.len(), 1);
assert!(matches!(
Expand All @@ -1237,6 +1272,31 @@ for the crater",
));
}

#[test]
fn parse_delegate_author() {
let cmds = parse_commands("@bors delegate");
assert_eq!(cmds.len(), 1);
assert!(matches!(
cmds[0],
Ok(BorsCommand::SetDelegate(DelegatedPermission::Review))
));
}

#[test]
fn parse_delegate_unknown_arg() {
let cmds = parse_commands("@bors delegate try");
insta::assert_debug_snapshot!(cmds, @r#"
[
Err(
UnknownArg {
arg: "try",
did_you_mean: "delegate=<try|review>",
},
),
]
"#);
}

#[test]
fn parse_delegate_review_author() {
let cmds = parse_commands("@bors delegate=review");
Expand Down
2 changes: 1 addition & 1 deletion src/bors/handlers/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mod tests {
- `delegate=<try|review>`: Delegate permissions for running try builds or approving to the PR author
- `try` allows the PR author to start try builds.
- `review` allows the PR author to both start try builds and approve the PR.
- `delegate+`: Delegate approval permissions to the PR author
- `delegate` | `delegate+`: Delegate approval permissions to the PR author
- Shortcut for `delegate=review`
- `delegate-`: Remove any previously granted permission delegation
- `try [parent=<parent>] [job|jobs=<jobs>]`: Start a try build.
Expand Down
7 changes: 5 additions & 2 deletions src/bors/handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,11 @@ async fn handle_comment(
CommandParseError::MissingArgValue { arg } => {
format!(r#"Unknown value for argument "{arg}"."#)
}
CommandParseError::UnknownArg(arg) => {
format!(r#"Unknown argument "{arg}"."#)
CommandParseError::UnknownArg { arg, did_you_mean } => {
format!(
r#"Unknown argument "{arg}". Did you mean to use `{} {did_you_mean}`?"#,
ctx.parser.prefix()
)
}
CommandParseError::DuplicateArg(arg) => {
format!(r#"Argument "{arg}" found multiple times."#)
Expand Down
13 changes: 13 additions & 0 deletions src/bors/handlers/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,19 @@ approved = { modifications = ["+foo", "+baz"], unless = ["label1", "label2"] }
.await;
}

#[sqlx::test]
async fn delegate_error_message(pool: sqlx::PgPool) {
BorsBuilder::new(pool)
.github(GitHub::unauthorized_pr_author())
.run_test(async |ctx: &mut BorsTester| {
ctx.post_comment(review_comment("@bors delegate try"))
.await?;
insta::assert_snapshot!(ctx.get_next_comment_text(()).await?, @r#"Unknown argument "try". Did you mean to use `@bors delegate=<try|review>`? Run `@bors help` to see available commands."#);
Ok(())
})
.await;
}

#[sqlx::test]
async fn delegatee_can_try(pool: sqlx::PgPool) {
let gh = BorsBuilder::new(pool)
Expand Down
2 changes: 1 addition & 1 deletion src/bors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ You can use the following commands:
- `delegate=<try|review>`: Delegate permissions for running try builds or approving to the PR author
- `try` allows the PR author to start try builds.
- `review` allows the PR author to both start try builds and approve the PR.
- `delegate+`: Delegate approval permissions to the PR author
- `delegate` | `delegate+`: Delegate approval permissions to the PR author
- Shortcut for `delegate=review`
- `delegate-`: Remove any previously granted permission delegation
- `try [parent=<parent>] [job|jobs=<jobs>]`: Start a try build.
Expand Down