diff --git a/Cargo.lock b/Cargo.lock index e7bab24fd..8155aaca5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -265,6 +265,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" +[[package]] +name = "equivalent" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" + [[package]] name = "errno" version = "0.3.14" @@ -362,6 +368,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "hashbrown" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" + [[package]] name = "homedir" version = "0.3.6" @@ -631,6 +643,16 @@ dependencies = [ "quote", ] +[[package]] +name = "indexmap" +version = "2.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "ipnet" version = "2.12.2" @@ -1285,6 +1307,15 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + [[package]] name = "shlex" version = "2.0.1" @@ -1428,6 +1459,7 @@ dependencies = [ "serde_json", "signal-hook", "tempfile", + "toml", ] [[package]] @@ -1504,6 +1536,45 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "1.1.6+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "920602543f0911ab71da12c50d59701da54c196d1a2bf5cb4b75667f137a406a" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned", + "toml_datetime", + "toml_parser", + "toml_writer", + "winnow", +] + +[[package]] +name = "toml_datetime" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3165f65f62e28e0115a00b2ebdd37eb6f3b641855f9d636d3cd4103767159ad7" +dependencies = [ + "serde_core", +] + +[[package]] +name = "toml_parser" +version = "1.1.3+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" +dependencies = [ + "winnow", +] + +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" + [[package]] name = "tower" version = "0.5.3" @@ -1976,6 +2047,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winnow" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" + [[package]] name = "writeable" version = "0.6.4" diff --git a/Cargo.toml b/Cargo.toml index 87395271f..9322b4cd5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ libc = "0.2.189" serde_json = "1.0.151" signal-hook = "0.4.4" tempfile = "3.27.0" +toml = "1.1.6" # The profile that 'dist' will build with [profile.dist] diff --git a/README.md b/README.md index b30c41fbf..9e9c8ad6b 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ To have the Run merge its pull request instead of leaving it for your review, st thirdshift merge https://github.com/acme/widgets/issues/7 ``` -`thirdshift --merge ` does the same. The agent gets the same implement prompt either way; the differences are the Self-merge at the end and the review of [Foreign commits](#foreign-commits-in-a-merge-run). The keyword goes before the URL: anything after the URL, `--merge` included, is an argument error. +`thirdshift --merge ` does the same, and either goes before or after the URL. The agent gets the same implement prompt either way; the differences are the Self-merge at the end and the review of [Foreign commits](#foreign-commits-in-a-merge-run). To make every Run on a machine a Merge run, set `merge.always` in the [User config](#user-config); `no-merge` (or `--no-merge`), before or after the URL, then makes one Run end ready for review instead. Giving a flag twice, or `merge` together with `no-merge`, is an argument error. A Merge run ends in one of three ways: @@ -114,7 +114,7 @@ A Merge run ends in one of three ways: - **stdout** carries only the pull request's URL: on success, and on a Failed run that leaves an open pull request, a draft or, after a policy refusal, one ready for review. The exit code tells the two apart, so script it as `url=$(thirdshift "$issue") && echo "ready: $url"`. - **stderr** carries everything else: errors, cleanup problems, and progress lines while sessions run. A successful Run's last line names the pull request too: `PR is ready for review`, or `PR is merged` after a Merge run. -- **Exit code** `0` means the Run ended with a pull request the factory stands behind, merged in a Merge run. Once the Self-merge has merged, the Run succeeds even if deleting the Issue branch on `origin` or closing the issue then fails: the merge can't be undone, so each failed step is a `warning:` line on stderr naming the command to run by hand, and the Run still exits `0` with the URL on stdout. Ctrl-C likewise: before the merge it makes a Failed run, after it thirdshift finishes these steps and exits as merged. `2` means the Issue URL is missing or isn't a GitHub Issue URL, or something follows it; the error and the help text go to stderr. Any other failure exits `1`. +- **Exit code** `0` means the Run ended with a pull request the factory stands behind, merged in a Merge run. Once the Self-merge has merged, the Run succeeds even if deleting the Issue branch on `origin` or closing the issue then fails: the merge can't be undone, so each failed step is a `warning:` line on stderr naming the command to run by hand, and the Run still exits `0` with the URL on stdout. Ctrl-C likewise: before the merge it makes a Failed run, after it thirdshift finishes these steps and exits as merged. `2` means the Issue URL is missing or isn't a GitHub Issue URL, there is an argument other than the URL and the Run flags, or a Run flag is repeated or contradicts another; the error and the help text go to stderr, before any work. A [User config](#user-config) thirdshift can't use exits `1`, also before any work. Any other failure exits `1`. The other commands: @@ -128,6 +128,19 @@ thirdshift help # print every form of the command, each with a one-line des Uncommitted changes in your clone are fine: the Run works in its own worktree from `origin`, so they are simply left out. Unpushed commits on the Base branch are not: push them first, or the Run stops. +### User config + +A **User config** at `~/.thirdshift/config.toml` sets this machine's defaults for every Run. It is optional: with no file, or one that says nothing about a setting, a Run does only what its command asks for. + +```toml +[merge] +always = true # every Run is a Merge run, without the merge word +``` + +With `merge.always = true`, `thirdshift ` is a Merge run, and `thirdshift --no-merge ` (or `no-merge`, before or after the URL) leaves that one Run's pull request ready for review. + +A Run reads the file before any work. One that isn't valid TOML, or that has a key or section thirdshift doesn't know, such as `alway` for `always`, stops the Run with an error naming the file and the offending key, so a typo can't silently leave a setting off. `update`, `version` and `help` never read it, so a broken User config can't block them. + ### Logs Each session's full transcript, as Claude Code's `stream-json` output, is written to its own file under `~/.thirdshift/logs/` (created if missing): diff --git a/src/args.rs b/src/args.rs new file mode 100644 index 000000000..1e9473ef3 --- /dev/null +++ b/src/args.rs @@ -0,0 +1,58 @@ +//! The command line: which command, and for a Run, its Issue URL and flags. + +use anyhow::{Result, bail}; + +use crate::issue::IssueUrl; +use crate::run::Goal; + +/// What thirdshift was asked to do. +pub enum Command { + Help, + Version, + Update, + Run(RunArgs), +} + +/// A Run's arguments. +pub struct RunArgs { + pub issue: IssueUrl, + /// The goal `merge` or `no-merge` asked for, if either was given; without + /// one, the User config decides. + pub goal: Option, +} + +/// Parse the arguments after the program name. `help`, `version` and +/// `update` are commands only as the first argument. Otherwise it is a Run: +/// one Issue URL, with each Run flag at most once, before or after it. +pub fn parse(args: &[String]) -> Result { + match args.first().map(String::as_str) { + Some("help" | "--help" | "-h") => return Ok(Command::Help), + Some("version" | "--version" | "-V") => return Ok(Command::Version), + Some("update") => return Ok(Command::Update), + _ => {} + } + let mut issue = None; + let mut goal = None; + for arg in args { + let asked = match arg.as_str() { + "merge" | "--merge" => Goal::Merged, + "no-merge" | "--no-merge" => Goal::ReadyForReview, + _ => { + if issue.is_some() { + bail!("unexpected argument after the Issue URL: {arg}"); + } + issue = Some(IssueUrl::parse(arg)?); + continue; + } + }; + match goal { + None => goal = Some(asked), + Some(given) if given == asked => bail!("repeated argument: {arg}"), + Some(_) => bail!("merge and no-merge can't be used together"), + } + } + let Some(issue) = issue else { + bail!("missing Issue URL"); + }; + Ok(Command::Run(RunArgs { issue, goal })) +} diff --git a/src/config.rs b/src/config.rs new file mode 100644 index 000000000..92c915a45 --- /dev/null +++ b/src/config.rs @@ -0,0 +1,108 @@ +//! The User config: `~/.thirdshift/config.toml`, this machine's defaults for +//! every Run. Only a Run reads it, so a broken one can't block `update`, +//! `version` or `help`. + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result, anyhow, bail}; +use toml::{Table, Value}; + +use crate::run::Goal; + +/// The settings a User config can hold. Each is what a Run does when its +/// command says nothing about it. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct UserConfig { + /// `merge.always`: every Run is a Merge run unless told `no-merge`. + pub merge_always: bool, +} + +impl UserConfig { + /// The User config under `$HOME`, or the defaults if there is none. + pub fn load() -> Result { + let Some(home) = std::env::var_os("HOME").filter(|home| !home.is_empty()) else { + return Ok(UserConfig::default()); + }; + let path = PathBuf::from(home).join(".thirdshift/config.toml"); + match std::fs::read_to_string(&path) { + Ok(text) => UserConfig::parse(&text, &path), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(UserConfig::default()), + Err(error) => Err(error).with_context(|| format!("can't read {}", path.display())), + } + } + + /// Parse `text`, the contents of the User config at `path`. Any key or + /// section thirdshift doesn't know is an error, so a typo can't silently + /// leave a setting unset. + fn parse(text: &str, path: &Path) -> Result { + let file = format!("the User config {}", path.display()); + let table: Table = text + .parse() + .map_err(|error| anyhow!("{error}")) + .with_context(|| format!("can't parse {file}"))?; + let mut config = UserConfig::default(); + for (section, value) in &table { + let settings = match (section.as_str(), value) { + ("merge", Value::Table(settings)) => settings, + ("merge", _) => bail!("merge must be the section [merge] in {file}"), + (_, Value::Table(_)) => bail!("unknown section [{section}] in {file}"), + _ => bail!("unknown key {section} in {file}"), + }; + for (key, value) in settings { + match (key.as_str(), value) { + ("always", Value::Boolean(always)) => config.merge_always = *always, + ("always", _) => bail!("merge.always must be true or false in {file}"), + _ => bail!("unknown key merge.{key} in {file}"), + } + } + } + Ok(config) + } + + /// The goal of a Run whose command gave no `merge` or `no-merge`. + pub fn default_goal(&self) -> Goal { + if self.merge_always { + Goal::Merged + } else { + Goal::ReadyForReview + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(text: &str) -> Result { + UserConfig::parse(text, Path::new("/home/me/.thirdshift/config.toml")) + } + + #[test] + fn merge_always_is_read() { + assert!(parse("[merge]\nalways = true\n").unwrap().merge_always); + assert!(!parse("[merge]\nalways = false\n").unwrap().merge_always); + assert!(!parse("").unwrap().merge_always); + } + + #[test] + fn errors_name_the_file_and_the_offending_key() { + for (text, named) in [ + ("[merge\n", "can't parse the User config"), + ("[merge]\nalway = true\n", "unknown key merge.alway"), + ("[logs]\n", "unknown section [logs]"), + ("merge = true\n", "merge must be the section [merge]"), + ("colour = true\n", "unknown key colour "), + ( + "[merge]\nalways = 1\n", + "merge.always must be true or false", + ), + ] { + let error = format!("{:#}", parse(text).unwrap_err()); + assert!(error.contains(named), "{text:?}: {error}"); + assert!( + error.contains("/home/me/.thirdshift/config.toml"), + "{text:?}: {error}" + ); + } + } +} diff --git a/src/main.rs b/src/main.rs index 24ebf459a..dbad53239 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,7 @@ +mod args; mod branch; mod ci; +mod config; mod failed_run; mod git; mod github; @@ -19,31 +21,40 @@ mod worktree; use std::process::ExitCode; -use issue::IssueUrl; -use run::Goal; +use args::{Command, RunArgs}; +use config::UserConfig; const HELP: &str = "\ thirdshift turns a GitHub issue into a ready-for-review pull request, or a merged one, unattended. -usage: thirdshift Run the factory on the issue, from the clone on the Base branch - thirdshift merge Run the factory on the issue, then merge its pull request - thirdshift update Update thirdshift to the latest release - thirdshift version Print thirdshift's version - thirdshift help Print this help +usage: thirdshift Run the factory on the issue, from the clone on the Base branch + thirdshift merge Run the factory on the issue, then merge its pull request + thirdshift --no-merge Run the factory on the issue and leave its pull request for review + thirdshift update Update thirdshift to the latest release + thirdshift version Print thirdshift's version + thirdshift help Print this help + +merge and --no-merge go before or after the Issue URL. + +The User config, ~/.thirdshift/config.toml, sets defaults for every Run on this machine. +With merge.always set, every Run is a Merge run unless given --no-merge: + + [merge] + always = true "; fn main() -> ExitCode { let args: Vec = std::env::args().skip(1).collect(); - let (goal, rest) = match args.first().map(String::as_str) { - Some("help" | "--help" | "-h") => { + let RunArgs { issue, goal } = match args::parse(&args) { + Ok(Command::Help) => { print!("{HELP}"); return ExitCode::SUCCESS; } - Some("version" | "--version" | "-V") => { + Ok(Command::Version) => { println!("thirdshift {}", env!("CARGO_PKG_VERSION")); return ExitCode::SUCCESS; } - Some("update") => { + Ok(Command::Update) => { return match update::update() { Ok(outcome) => { progress::step(outcome); @@ -55,21 +66,16 @@ fn main() -> ExitCode { } }; } - Some("merge" | "--merge") => (Goal::Merged, &args[1..]), - _ => (Goal::ReadyForReview, &args[..]), - }; - let Some((url, after)) = rest.split_first() else { - return argument_error(format_args!("missing Issue URL")); - }; - let issue = match IssueUrl::parse(url) { - Ok(issue) => issue, + Ok(Command::Run(run_args)) => run_args, Err(error) => return argument_error(format_args!("{error:#}")), }; - if let Some(extra) = after.first() { - return argument_error(format_args!( - "unexpected argument after the Issue URL: {extra}" - )); - } + let goal = match UserConfig::load() { + Ok(config) => goal.unwrap_or(config.default_goal()), + Err(error) => { + progress::step(format_args!("{error:#}")); + return ExitCode::FAILURE; + } + }; if let Err(error) = interrupt::install() { progress::step(format_args!("{error:#}")); return ExitCode::FAILURE; diff --git a/tests/command_surface.rs b/tests/command_surface.rs index 1e6e5934d..806b6b4f2 100644 --- a/tests/command_surface.rs +++ b/tests/command_surface.rs @@ -137,17 +137,16 @@ fn merge_without_an_issue_url_prints_an_error_and_the_help_to_stderr() { } #[test] -fn anything_after_the_issue_url_prints_an_error_and_the_help_to_stderr() { +fn an_extra_argument_prints_an_error_and_the_help_to_stderr() { let scenario = Scenario::new(); let url = scenario.issue_url(7); - for args in [ - vec![url.as_str(), "--merge"], - vec![url.as_str(), "merge"], - vec!["merge", url.as_str(), "extra"], - vec!["--merge", url.as_str(), "--merge"], + for (args, extra) in [ + (vec![url.as_str(), "extra"], "extra"), + (vec!["merge", url.as_str(), "extra"], "extra"), + (vec![url.as_str(), url.as_str()], url.as_str()), + (vec![url.as_str(), "--no-merge", "frobnicate"], "frobnicate"), ] { - let extra = args.last().unwrap(); let result = scenario.run(&args); assert_argument_error( @@ -158,3 +157,68 @@ fn anything_after_the_issue_url_prints_an_error_and_the_help_to_stderr() { assert!(scenario.claude_calls().is_empty(), "{args:?} started a Run"); } } + +#[test] +fn a_repeated_flag_prints_an_error_and_the_help_to_stderr() { + let scenario = Scenario::new(); + let url = scenario.issue_url(7); + + for (args, repeated) in [ + (vec!["merge", url.as_str(), "merge"], "merge"), + (vec!["--merge", url.as_str(), "--merge"], "--merge"), + (vec!["merge", "--merge", url.as_str()], "--merge"), + (vec![url.as_str(), "no-merge", "--no-merge"], "--no-merge"), + ] { + let result = scenario.run(&args); + + assert_argument_error( + &scenario, + &result, + &format!("repeated argument: {repeated}"), + ); + assert!(scenario.claude_calls().is_empty(), "{args:?} started a Run"); + } +} + +#[test] +fn merge_with_no_merge_prints_an_error_and_the_help_to_stderr() { + let scenario = Scenario::new(); + let url = scenario.issue_url(7); + + for args in [ + vec!["merge", url.as_str(), "no-merge"], + vec!["--no-merge", "--merge", url.as_str()], + vec![url.as_str(), "--merge", "no-merge"], + ] { + let result = scenario.run(&args); + + assert_argument_error( + &scenario, + &result, + "merge and no-merge can't be used together", + ); + assert!(scenario.claude_calls().is_empty(), "{args:?} started a Run"); + } +} + +#[test] +fn no_merge_without_an_issue_url_prints_an_error_and_the_help_to_stderr() { + let scenario = Scenario::new(); + + for no_merge in ["no-merge", "--no-merge"] { + let result = scenario.run(&[no_merge]); + + assert_argument_error(&scenario, &result, "missing Issue URL"); + } +} + +#[test] +fn help_lists_no_merge_and_the_user_config() { + let scenario = Scenario::new(); + + let help = scenario.run(&["help"]).stdout; + + for mention in ["--no-merge", "~/.thirdshift/config.toml", "merge.always"] { + assert!(help.contains(mention), "help lacks {mention:?}: {help}"); + } +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 62fcb6265..a41ecf21c 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -8,6 +8,7 @@ //! it rejects non-fast-forward pushes, so no rebase or //! force-push can reach it //! home/ $HOME: .gitconfig with identity and the insteadOf rule +//! home/.thirdshift/ the User config, config.toml, if the test writes one //! home/.config/ $XDG_CONFIG_HOME, where an install receipt would be //! bin/ fake gh and claude, first on PATH //! tmp/ $TMPDIR, so leftover temp directories are visible @@ -198,6 +199,16 @@ impl Scenario { command } + /// Write `toml` as the User config, `~/.thirdshift/config.toml` under the + /// scenario's `$HOME`, and return its path. + pub fn user_config_is(&self, toml: &str) -> PathBuf { + let dir = self.path("home/.thirdshift"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config.toml"); + fs::write(&path, toml).unwrap(); + path + } + /// Set issue `number`'s state on the fake GitHub: `"OPEN"` or `"CLOSED"`. pub fn issue_is(&self, number: u32, state: &str) { let mut gh = self.gh_state(); diff --git a/tests/update.rs b/tests/update.rs index b7e0a3e81..74c0651c4 100644 --- a/tests/update.rs +++ b/tests/update.rs @@ -286,3 +286,19 @@ fn when_github_is_unreachable_update_fails_with_an_explanation() { result.stderr ); } + +#[test] +fn update_never_reads_the_user_config_so_a_broken_one_cannot_block_it() { + let scenario = installed_scenario(); + scenario.user_config_is("[merge\nalway = true\n"); + let github = FakeReleases::start(); + github.publish(VERSION, "true"); + + let result = github.update(&scenario); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!( + result.stderr, + format!("thirdshift: thirdshift {VERSION} is the latest release\n") + ); +} diff --git a/tests/user_config.rs b/tests/user_config.rs new file mode 100644 index 000000000..5463d5e19 --- /dev/null +++ b/tests/user_config.rs @@ -0,0 +1,198 @@ +//! The User config, `~/.thirdshift/config.toml`: `merge.always` makes every +//! Run a Merge run unless the command says `no-merge`, and a config thirdshift +//! can't use stops the Run before any work. + +mod support; + +use support::{RunResult, Scenario}; + +/// The agent commits its work and opens a PR that closes issue #7, into +/// `main`. +const AGENT_OPENS_PR: &str = r#" +echo "feature" > feature.txt +git add feature.txt +git commit -q -m "Add feature" +gh pr create --base main --head issue-7 --title "Add feature" --body "Closes #7" +"#; + +const PR_URL: &str = "https://github.com/acme/widgets/pull/1"; + +/// Assert the Run ended with PR #1 merged, as a Merge run does. +fn assert_merged(scenario: &Scenario, result: &RunResult) { + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{PR_URL}\n")); + assert_eq!( + result.stderr.lines().last(), + Some(format!("thirdshift: PR {PR_URL} is merged").as_str()), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["state"], "MERGED"); +} + +/// Assert the Run ended with PR #1 open and ready for review, never merged. +fn assert_ready_for_review(scenario: &Scenario, result: &RunResult) { + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{PR_URL}\n")); + assert_eq!( + result.stderr.lines().last(), + Some(format!("thirdshift: PR {PR_URL} is ready for review").as_str()), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["state"], "OPEN"); + assert!( + !scenario + .gh_calls() + .iter() + .any(|call| call.starts_with(&["pr".to_string(), "merge".to_string()])), + "thirdshift ran gh pr merge" + ); +} + +#[test] +fn merge_always_makes_a_run_without_the_merge_word_a_merge_run() { + let scenario = Scenario::new(); + scenario.user_config_is("[merge]\nalways = true\n"); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_merged(&scenario, &result); +} + +#[test] +fn no_merge_overrides_merge_always_for_one_run() { + for flag in ["no-merge", "--no-merge"] { + for flag_first in [true, false] { + let scenario = Scenario::new(); + scenario.user_config_is("[merge]\nalways = true\n"); + scenario.agent_does(AGENT_OPENS_PR); + let url = scenario.issue_url(7); + let args = if flag_first { + [flag, url.as_str()] + } else { + [url.as_str(), flag] + }; + + let result = scenario.run(&args); + + assert_ready_for_review(&scenario, &result); + } + } +} + +#[test] +fn without_a_config_file_a_run_is_not_a_merge_run() { + let scenario = Scenario::new(); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_ready_for_review(&scenario, &result); +} + +#[test] +fn a_config_that_says_nothing_about_merging_leaves_a_run_not_a_merge_run() { + for config in [ + "", + "# nothing set yet\n", + "[merge]\n", + "[merge]\nalways = false\n", + ] { + let scenario = Scenario::new(); + scenario.user_config_is(config); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_ready_for_review(&scenario, &result); + } +} + +#[test] +fn the_merge_word_goes_before_or_after_the_issue_url() { + for merge in ["merge", "--merge"] { + let scenario = Scenario::new(); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7), merge]); + + assert_merged(&scenario, &result); + } +} + +#[test] +fn an_unparsable_config_stops_the_run_naming_the_file() { + let scenario = Scenario::new(); + let path = scenario.user_config_is("[merge\nalways = true\n"); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); + scenario.assert_rejected_before_any_work(&result, &path.display().to_string()); + assert!(scenario.gh_calls().is_empty(), "thirdshift called gh"); +} + +#[test] +fn an_unknown_key_or_section_stops_the_run_naming_it_and_the_file() { + for (config, named) in [ + ("[merge]\nalway = true\n", "merge.alway"), + ("[merges]\nalways = true\n", "[merges]"), + ("colour = true\n", "colour"), + ] { + let scenario = Scenario::new(); + let path = scenario.user_config_is(config); + scenario.agent_does(AGENT_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(1), "{config}: {}", result.stderr); + scenario.assert_rejected_before_any_work(&result, &path.display().to_string()); + assert!( + result.stderr.contains(named), + "expected {named:?} in stderr for {config:?}: {}", + result.stderr + ); + } +} + +#[test] +fn a_value_of_the_wrong_type_stops_the_run_naming_the_key() { + for config in ["merge = true\n", "[merge]\nalways = \"yes\"\n"] { + let scenario = Scenario::new(); + let path = scenario.user_config_is(config); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(1), "{config}: {}", result.stderr); + scenario.assert_rejected_before_any_work(&result, &path.display().to_string()); + assert!(result.stderr.contains("merge"), "stderr: {}", result.stderr); + } +} + +#[test] +fn argument_errors_come_before_the_config_is_read() { + let scenario = Scenario::new(); + scenario.user_config_is("[merge\n"); + + let result = scenario.run(&["merge", "no-merge", &scenario.issue_url(7)]); + + assert_eq!(result.code, Some(2), "stderr: {}", result.stderr); +} + +#[test] +fn help_and_version_succeed_with_a_broken_config() { + for config in ["[merge\n", "[merge]\nalway = true\n"] { + let scenario = Scenario::new(); + scenario.user_config_is(config); + + for command in ["help", "version"] { + let result = scenario.run(&[command]); + + assert_eq!(result.code, Some(0), "{command}: {}", result.stderr); + assert_eq!(result.stderr, "", "{command}"); + } + } +}