From fe3d61123a676bdabd6df9d21b166d176e2b5580 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 26 May 2026 17:08:48 +0200 Subject: [PATCH 1/3] fix(cli): accept --debug on the Rust binary and actually toggle it in Python Two coupled gaps the same user-visible bug was hiding: 1. The Rust binary never declared the top-level `--debug` flag the Python CLI accepts. Invocations like `mergify --debug ci git-refs` were rejected with `error: unexpected argument '--debug' found` the moment a command was promoted from shim to native. Add `debug: bool` as a `global = true` argument on `CliRoot`; native commands accept it as a no-op (no native code path consults the flag yet) and shimmed dispatches re-inject `--debug` at the front of the forwarded argv so the Python `cli` group still receives it. 2. The Python `cli.py` parsed `--debug` into `ctx.obj["debug"]` but never called `utils.set_debug(...)`, so the module-level `_DEBUG` toggle stayed `False` and the 6+ `if is_debug():` sites in `utils.py` / `stack/*` never fired regardless of the flag. Wire `set_debug(debug=debug)` from the root group so the flag has the effect users have always expected it to have. Co-Authored-By: Claude Opus 4.7 Change-Id: I62ac941db12c55c988896086d7042c3dacf863af --- crates/mergify-cli/src/main.rs | 130 ++++++++++++++++++++++++++++++++- mergify_cli/cli.py | 5 ++ mergify_cli/tests/test_cli.py | 18 +++++ 3 files changed, 149 insertions(+), 4 deletions(-) diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index f8f0fb46..31bcf2d7 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -118,6 +118,21 @@ fn prepend_two(first: &str, second: &str, tail: Vec) -> Vec { out } +/// Re-inject the global `--debug` flag at the front of the forwarded +/// argv so Python's root group sees it. Clap consumed the flag when +/// parsing the Rust-side argv, but the Python CLI declares it at +/// root too — leaving it off would silently drop the user's intent +/// for shimmed commands. +fn inject_global_flags(debug: bool, argv: Vec) -> Vec { + if !debug { + return argv; + } + let mut out = Vec::with_capacity(argv.len() + 1); + out.push("--debug".to_string()); + out.extend(argv); + out +} + /// Single source of truth for the `(group, subcommand)` pairs the /// Rust binary handles natively. Used by [`looks_native`] for argv /// recognition and by the `--list-native-commands` hidden flag so @@ -344,17 +359,29 @@ fn detect_dispatch(argv: &[String]) -> Option { #[allow(clippy::too_many_lines)] // mostly mechanical match arms fn dispatch_from_parsed(parsed: CliRoot) -> Dispatch { + let debug = parsed.debug; match parsed.command { - Subcommands::Stack(ShimmedArgs { args }) => Dispatch::Shim(prepend_one("stack", args)), + Subcommands::Stack(ShimmedArgs { args }) => { + Dispatch::Shim(inject_global_flags(debug, prepend_one("stack", args))) + } Subcommands::Ci(CiArgs { command: CiSubcommand::Scopes(ShimmedArgs { args }), - }) => Dispatch::Shim(prepend_two("ci", "scopes", args)), + }) => Dispatch::Shim(inject_global_flags( + debug, + prepend_two("ci", "scopes", args), + )), Subcommands::Ci(CiArgs { command: CiSubcommand::JunitProcess(ShimmedArgs { args }), - }) => Dispatch::Shim(prepend_two("ci", "junit-process", args)), + }) => Dispatch::Shim(inject_global_flags( + debug, + prepend_two("ci", "junit-process", args), + )), Subcommands::Ci(CiArgs { command: CiSubcommand::JunitUpload(ShimmedArgs { args }), - }) => Dispatch::Shim(prepend_two("ci", "junit-upload", args)), + }) => Dispatch::Shim(inject_global_flags( + debug, + prepend_two("ci", "junit-upload", args), + )), Subcommands::Config(ConfigArgs { config_file, command: ConfigSubcommand::Validate(_), @@ -776,6 +803,15 @@ fn run_native(cmd: NativeCommand) -> ExitCode { #[command(name = "mergify", disable_help_subcommand = true)] #[command(disable_version_flag = true)] struct CliRoot { + /// Enable verbose debug logging. Mirrors the Python CLI's + /// top-level `--debug` flag so the same invocations work + /// against either binary; native commands accept it as a no-op + /// today (no native code path consults it yet), shimmed ones + /// re-inject it into the forwarded argv so the Python side can + /// honor it. + #[arg(long, global = true)] + debug: bool, + #[command(subcommand)] command: Subcommands, } @@ -1191,3 +1227,89 @@ struct FreezeDeleteCliArgs { fn parse_naive_datetime_arg(value: &str) -> Result { parse_naive_datetime(value).map_err(|e| e.to_string()) } + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(argv: &[&str]) -> CliRoot { + CliRoot::try_parse_from( + std::iter::once("mergify".to_string()).chain(argv.iter().map(|s| (*s).to_string())), + ) + .expect("argv parses") + } + + #[test] + fn root_debug_flag_accepted_before_native_command() { + // Without this, clap would reject `--debug` and exit before + // any dispatch — the regression we just fixed. + let parsed = parse(&["--debug", "ci", "git-refs"]); + assert!(parsed.debug, "--debug should be parsed as true"); + assert!(matches!( + parsed.command, + Subcommands::Ci(CiArgs { + command: CiSubcommand::GitRefs(_) + }) + )); + } + + #[test] + fn root_debug_flag_accepted_after_native_group() { + // `--debug` is declared `global = true`, so it's recognised + // at any point along the subcommand chain. clap's + // hand-off prefers root, but users sometimes type it after + // the group name — both must work. + let parsed = parse(&["queue", "--debug", "status"]); + assert!(parsed.debug); + } + + #[test] + fn shimmed_dispatch_reinjects_debug_at_argv_head() { + // Clap consumes the root `--debug`; without re-injection, + // the Python side (which declares its own root `--debug`) + // would never see the flag. + let parsed = parse(&["--debug", "stack", "push"]); + let Dispatch::Shim(argv) = dispatch_from_parsed(parsed) else { + panic!("stack must dispatch to the Python shim"); + }; + assert_eq!(argv, vec!["--debug", "stack", "push"]); + } + + #[test] + fn shimmed_dispatch_omits_debug_when_not_set() { + let parsed = parse(&["stack", "push"]); + let Dispatch::Shim(argv) = dispatch_from_parsed(parsed) else { + panic!("stack must dispatch to the Python shim"); + }; + // No `--debug` prefix when the user didn't pass one — we + // don't want to silently flip Python into verbose mode. + assert_eq!(argv, vec!["stack", "push"]); + } + + #[test] + fn shimmed_dispatch_reinjects_debug_for_ci_subcommand() { + // The two-token shim paths (`ci scopes`, `ci junit-process`, + // `ci junit-upload`) need the same treatment as the + // single-token `stack` shim — every shim arm must re-inject + // `--debug` so the Python side honors it. + for (group, sub, tail) in &[ + ("ci", "scopes", vec!["--base", "main"]), + ("ci", "junit-process", vec!["--files", "a.xml"]), + ("ci", "junit-upload", vec!["--files", "a.xml"]), + ] { + let mut argv_in = vec!["--debug", group, sub]; + argv_in.extend(tail.iter().copied()); + let parsed = parse(&argv_in); + let Dispatch::Shim(argv) = dispatch_from_parsed(parsed) else { + panic!("ci {sub} must dispatch to the Python shim"); + }; + let mut expected = vec![ + "--debug".to_string(), + (*group).to_string(), + (*sub).to_string(), + ]; + expected.extend(tail.iter().map(|s| (*s).to_string())); + assert_eq!(argv, expected, "ci {sub} dispatch dropped --debug"); + } + } +} diff --git a/mergify_cli/cli.py b/mergify_cli/cli.py index 9a4e07c6..fe112cfd 100644 --- a/mergify_cli/cli.py +++ b/mergify_cli/cli.py @@ -46,6 +46,11 @@ def cli( debug: bool, ) -> None: ctx.obj = {"debug": debug} + # Propagate the flag to the module-level toggle utils consults + # via `is_debug()`. Without this call the `--debug` flag is + # parsed but ignored — the 6+ `if is_debug():` sites in + # `utils.py` / `stack/*` never fire. + utils.set_debug(debug=debug) if ctx.invoked_subcommand is None: click.echo(ctx.get_help()) diff --git a/mergify_cli/tests/test_cli.py b/mergify_cli/tests/test_cli.py index 05c4197e..2315de57 100644 --- a/mergify_cli/tests/test_cli.py +++ b/mergify_cli/tests/test_cli.py @@ -40,6 +40,24 @@ def test_cli_shows_help_by_default() -> None: assert "stack" in result.output +def test_cli_debug_flag_toggles_is_debug() -> None: + """`mergify --debug` must flip `utils.is_debug()` so the 6+ + `if is_debug():` sites in `utils.py` and `stack/*` actually + fire. Pinned with an explicit pre/post check because the flag + was parsed-but-silently-ignored for a long time before the + `set_debug(...)` call was wired into the root group.""" + runner = testing.CliRunner() + # Sanity: default state. We restore it in `finally` regardless + # so a stray module-level flag doesn't leak into sibling tests. + assert not utils.is_debug() + try: + result = runner.invoke(cli_mod.cli, ["--debug"]) + assert result.exit_code == 0, result.output + assert utils.is_debug(), "expected --debug to flip is_debug() on" + finally: + utils.set_debug(debug=False) + + def test_clirunner_translates_mergify_error_to_exit_code() -> None: """CliRunner must see the typed exit code when MergifyError is raised.""" import click From 32a71f8884cd122159f947fa163fb27c203ed513 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 19 May 2026 14:05:18 +0200 Subject: [PATCH 2/3] refactor(rust): share test scaffolding via mergify-test-support crate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every command crate's test module re-rolled the same ~30 LOC of `SharedBytes` / `SharedWriter` / `Captured` / `make_output` glue — about 350 LOC of pure boilerplate across 15 files, drifting over time (some `Captured` had `stderr`, some didn't; one file even carried a `_stderr_accessor_lives` dead-code stub just to silence the resulting warning). Extract the canonical version into a new `mergify-test-support` crate that other crates pull in as a `dev-dependencies`. The new `Captured` API exposes `human()` / `new(mode)` constructors and `stdout()` / `stderr()` accessors, so the common pattern shrinks from a 12-line `String::from_utf8(cap.stdout.lock().unwrap().clone())` to `let s = cap.stdout()`. Net `-418 / +114` lines across the workspace. Behavior unchanged; all 233 tests pass. The crate is `publish = false` and only ever appears under `[dev-dependencies]`, so the test-only types never leak into a production build. Co-Authored-By: Claude Opus 4.7 Change-Id: If9d688cdaf55360ba90f386d2020b52346c19b28 --- Cargo.lock | 11 +++ crates/mergify-ci/Cargo.toml | 1 + crates/mergify-ci/src/git_refs.rs | 58 +++--------- crates/mergify-ci/src/queue_info.rs | 40 ++------- crates/mergify-ci/src/queue_metadata.rs | 42 ++------- crates/mergify-ci/src/scopes_send.rs | 56 +++--------- crates/mergify-config/Cargo.toml | 1 + crates/mergify-config/src/simulate.rs | 31 +------ crates/mergify-config/src/validate.rs | 41 ++------- crates/mergify-freeze/Cargo.toml | 1 + crates/mergify-freeze/src/create.rs | 40 +-------- crates/mergify-freeze/src/delete.rs | 38 +------- crates/mergify-freeze/src/list.rs | 55 +++--------- crates/mergify-freeze/src/update.rs | 38 +------- crates/mergify-queue/Cargo.toml | 1 + crates/mergify-queue/src/pause.rs | 36 +------- crates/mergify-queue/src/show.rs | 62 +++---------- crates/mergify-queue/src/status.rs | 60 +++---------- crates/mergify-queue/src/unpause.rs | 38 +------- crates/mergify-test-support/Cargo.toml | 16 ++++ crates/mergify-test-support/src/lib.rs | 114 ++++++++++++++++++++++++ 21 files changed, 244 insertions(+), 536 deletions(-) create mode 100644 crates/mergify-test-support/Cargo.toml create mode 100644 crates/mergify-test-support/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 0000ac92..06e95f75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1064,6 +1064,7 @@ version = "0.0.0" dependencies = [ "chrono", "mergify-core", + "mergify-test-support", "serde", "serde_json", "serde_yaml_ng", @@ -1098,6 +1099,7 @@ version = "0.0.0" dependencies = [ "jsonschema", "mergify-core", + "mergify-test-support", "serde", "serde_json", "serde_norway", @@ -1130,6 +1132,7 @@ dependencies = [ "chrono", "iana-time-zone", "mergify-core", + "mergify-test-support", "mergify-tui", "serde", "serde_json", @@ -1154,6 +1157,7 @@ dependencies = [ "chrono", "indexmap", "mergify-core", + "mergify-test-support", "mergify-tui", "serde", "serde_json", @@ -1162,6 +1166,13 @@ dependencies = [ "wiremock", ] +[[package]] +name = "mergify-test-support" +version = "0.0.0" +dependencies = [ + "mergify-core", +] + [[package]] name = "mergify-tui" version = "0.0.0" diff --git a/crates/mergify-ci/Cargo.toml b/crates/mergify-ci/Cargo.toml index c8918139..27552aef 100644 --- a/crates/mergify-ci/Cargo.toml +++ b/crates/mergify-ci/Cargo.toml @@ -20,6 +20,7 @@ url = "2" uuid = { version = "1", features = ["v4"] } [dev-dependencies] +mergify-test-support = { path = "../mergify-test-support" } tempfile = "3.14" temp-env = { version = "0.3", features = ["async_closure"] } tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } diff --git a/crates/mergify-ci/src/git_refs.rs b/crates/mergify-ci/src/git_refs.rs index 754b1610..16d0e5b7 100644 --- a/crates/mergify-ci/src/git_refs.rs +++ b/crates/mergify-ci/src/git_refs.rs @@ -437,30 +437,11 @@ fn buildkite_meta_data_set(key: &str, value: &str) -> std::io::Result<()> { #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use tempfile::TempDir; use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - fn no_notes(_branch: &str, _sha: &str) -> Option { None } @@ -473,7 +454,7 @@ mod tests { #[test] fn falls_back_to_head_pair_when_no_event() { - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars_unset( ["GITHUB_EVENT_NAME", "GITHUB_EVENT_PATH", "BUILDKITE"], || detect(&mut cap.output, &no_notes).unwrap(), @@ -495,7 +476,7 @@ mod tests { }, }), ); - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -516,7 +497,7 @@ mod tests { &dir, &serde_json::json!({"before": "old-sha", "after": "new-sha"}), ); - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("push")), @@ -543,7 +524,7 @@ mod tests { }, }), ); - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -581,7 +562,7 @@ mod tests { None } }; - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -600,7 +581,7 @@ mod tests { &dir, &serde_json::json!({"pull_request": {"head": {"sha": "h"}}}), ); - let mut cap = make_output(); + let mut cap = Captured::human(); let err = temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -614,7 +595,7 @@ mod tests { #[test] fn detects_buildkite_pull_request() { - let mut cap = make_output(); + let mut cap = Captured::human(); let refs = temp_env::with_vars( [ ("BUILDKITE", Some("true")), @@ -647,9 +628,9 @@ mod tests { head: "h".into(), source: ReferencesSource::GithubEventPush, }; - let mut cap = make_output(); + let mut cap = Captured::human(); emit(&refs, Format::Text, &mut cap.output).unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert_eq!(stdout, "Base: b\nHead: h\n"); } @@ -660,9 +641,9 @@ mod tests { head: "has space".into(), source: ReferencesSource::MergeQueue, }; - let mut cap = make_output(); + let mut cap = Captured::human(); emit(&refs, Format::Shell, &mut cap.output).unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("MERGIFY_GIT_REFS_BASE=main")); assert!(stdout.contains("MERGIFY_GIT_REFS_HEAD='has space'")); assert!(stdout.contains("MERGIFY_GIT_REFS_SOURCE=merge_queue")); @@ -675,9 +656,9 @@ mod tests { head: "HEAD".into(), source: ReferencesSource::GithubEventOther, }; - let mut cap = make_output(); + let mut cap = Captured::human(); emit(&refs, Format::Json, &mut cap.output).unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert_eq!( stdout.trim_end(), r#"{"base":null,"head":"HEAD","source":"github_event_other"}"# @@ -691,15 +672,4 @@ mod tests { assert!(matches!(Format::parse("json"), Ok(Format::Json))); assert!(Format::parse("yaml").is_err()); } - - struct SharedWriter(SharedBytes); - impl std::io::Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-ci/src/queue_info.rs b/crates/mergify-ci/src/queue_info.rs index cde4b646..f3308ff2 100644 --- a/crates/mergify-ci/src/queue_info.rs +++ b/crates/mergify-ci/src/queue_info.rs @@ -64,30 +64,11 @@ fn write_github_output(metadata: &MergeQueueMetadata) -> Result<(), CliError> { #[cfg(test)] mod tests { use mergify_core::ExitCode; - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use tempfile::TempDir; use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - fn write_event_file(dir: &TempDir, body: &str, title: &str) -> PathBuf { let path = dir.path().join("event.json"); let payload = serde_json::json!({ @@ -102,7 +83,7 @@ mod tests { #[test] fn errors_when_not_in_mq_context() { - let mut cap = make_output(); + let mut cap = Captured::human(); let err = temp_env::with_vars_unset(["GITHUB_EVENT_NAME", "GITHUB_EVENT_PATH"], || { run(&mut cap.output).unwrap_err() }); @@ -119,7 +100,7 @@ mod tests { "merge queue: batch", ); - let mut cap = make_output(); + let mut cap = Captured::human(); temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -129,7 +110,7 @@ mod tests { || run(&mut cap.output).unwrap(), ); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("\"checking_base_sha\": \"abc123\"")); assert!(stdout.contains("\"number\": 10")); } @@ -144,7 +125,7 @@ mod tests { ); let gha_output = dir.path().join("gha_output"); - let mut cap = make_output(); + let mut cap = Captured::human(); temp_env::with_vars( [ ("GITHUB_EVENT_NAME", Some("pull_request")), @@ -158,15 +139,4 @@ mod tests { assert!(written.starts_with("queue_metadata< std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-ci/src/queue_metadata.rs b/crates/mergify-ci/src/queue_metadata.rs index 1ddd5c10..3406c7e8 100644 --- a/crates/mergify-ci/src/queue_metadata.rs +++ b/crates/mergify-ci/src/queue_metadata.rs @@ -106,29 +106,10 @@ pub fn detect(output: &mut dyn Output) -> std::io::Result>>; - - struct Captured { - output: StdioOutput, - stderr: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stderr } - } - #[test] fn parse_yaml_block_extracts_metadata() { let body = "prelude\n\n```yaml\nchecking_base_sha: abc\npull_requests:\n - number: 1\n```\ntrailing"; @@ -152,10 +133,10 @@ mod tests { }), ..Default::default() }; - let mut cap = make_output(); + let mut cap = Captured::human(); let result = extract_from_event(&ev, &mut cap.output).unwrap(); assert!(result.is_none()); - assert!(cap.stderr.lock().unwrap().is_empty()); + assert!(cap.stderr().is_empty()); } #[test] @@ -168,10 +149,10 @@ mod tests { }), ..Default::default() }; - let mut cap = make_output(); + let mut cap = Captured::human(); let result = extract_from_event(&ev, &mut cap.output).unwrap(); assert!(result.is_none()); - let stderr = String::from_utf8(cap.stderr.lock().unwrap().clone()).unwrap(); + let stderr = cap.stderr(); assert!(stderr.contains("without body"), "got: {stderr:?}"); } @@ -186,19 +167,8 @@ mod tests { }), ..Default::default() }; - let mut cap = make_output(); + let mut cap = Captured::human(); let meta = extract_from_event(&ev, &mut cap.output).unwrap().unwrap(); assert_eq!(meta.checking_base_sha, "deadbeef"); } - - struct SharedWriter(SharedBytes); - impl std::io::Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-ci/src/scopes_send.rs b/crates/mergify-ci/src/scopes_send.rs index 6ca7fa31..52bb3666 100644 --- a/crates/mergify-ci/src/scopes_send.rs +++ b/crates/mergify-ci/src/scopes_send.rs @@ -146,8 +146,7 @@ struct SendScopesRequest<'a> { mod tests { use std::fs; - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -158,30 +157,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - #[allow(dead_code)] // stdout is captured for tests that want to assert on it - stdout: SharedBytes, - stderr: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { - output, - stdout, - stderr, - } - } - /// Clear every CI-provider env var the resolver inspects, then /// apply the test-specific overrides on top. Without this, a test /// running on a real CI host (Buildkite, Actions, …) inherits @@ -286,7 +261,7 @@ mod tests { #[tokio::test] async fn run_skips_when_no_pull_request_detected() { - let mut cap = make_output(); + let mut cap = Captured::human(); with_ci_env_async(&[("GITHUB_REPOSITORY", Some("owner/repo"))], async { run( ScopesSendOptions { @@ -305,7 +280,7 @@ mod tests { .unwrap(); }) .await; - let stderr_str = String::from_utf8(cap.stderr.lock().unwrap().clone()).unwrap(); + let stderr_str = cap.stderr(); assert!( stderr_str.contains("skipping"), "expected skip message, got {stderr_str:?}" @@ -323,7 +298,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let direct = vec!["a".to_string()]; @@ -374,7 +349,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let direct = vec!["direct".to_string()]; @@ -409,7 +384,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( @@ -442,7 +417,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( @@ -461,7 +436,7 @@ mod tests { .await .unwrap(); - let err = String::from_utf8(cap.stderr.lock().unwrap().clone()).unwrap(); + let err = cap.stderr(); assert!(err.contains("--file is deprecated"), "got: {err:?}"); } @@ -486,7 +461,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( @@ -505,18 +480,7 @@ mod tests { .await .unwrap(); - let err = String::from_utf8(cap.stderr.lock().unwrap().clone()).unwrap(); + let err = cap.stderr(); assert!(err.contains("--file is deprecated"), "got: {err:?}"); } - - struct SharedWriter(std::sync::Arc>>); - impl std::io::Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-config/Cargo.toml b/crates/mergify-config/Cargo.toml index 5f9f6c6c..e543dd5a 100644 --- a/crates/mergify-config/Cargo.toml +++ b/crates/mergify-config/Cargo.toml @@ -18,6 +18,7 @@ serde_norway = "0.9" url = "2" [dev-dependencies] +mergify-test-support = { path = "../mergify-test-support" } tempfile = "3.14" temp-env = "0.3" tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } diff --git a/crates/mergify-config/src/simulate.rs b/crates/mergify-config/src/simulate.rs index ed5bef15..c9d1bcf9 100644 --- a/crates/mergify-config/src/simulate.rs +++ b/crates/mergify-config/src/simulate.rs @@ -134,8 +134,7 @@ fn emit_result(output: &mut dyn Output, response: &SimulatorResponse) -> std::io mod tests { use std::fs; - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -205,16 +204,7 @@ mod tests { }; let api_url = server.uri(); - let stdout: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let mut output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - + let mut cap = Captured::human(); run( SimulateOptions { pull_request: &pull_request, @@ -222,13 +212,12 @@ mod tests { token: Some("test-token"), api_url: Some(&api_url), }, - &mut output, + &mut cap.output, ) .await .unwrap(); - let stdout_bytes = stdout.lock().unwrap().clone(); - let stdout_str = String::from_utf8(stdout_bytes).unwrap(); + let stdout_str = cap.stdout(); assert!( stdout_str.contains("Would merge immediately"), "expected title in output: {stdout_str:?}", @@ -238,16 +227,4 @@ mod tests { "expected summary in output: {stdout_str:?}", ); } - - struct SharedWriter(std::sync::Arc>>); - - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-config/src/validate.rs b/crates/mergify-config/src/validate.rs index 4ba516f0..e669ac32 100644 --- a/crates/mergify-config/src/validate.rs +++ b/crates/mergify-config/src/validate.rs @@ -156,8 +156,7 @@ fn emit_result( mod tests { use std::fs; - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -255,31 +254,17 @@ mod tests { #[test] fn emit_result_success_writes_to_output() { - let buf: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr_buf: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stdout_writer = SharedWriter(std::sync::Arc::clone(&buf)); - let stderr_writer = SharedWriter(std::sync::Arc::clone(&stderr_buf)); - let mut output = StdioOutput::with_sinks(OutputMode::Human, stdout_writer, stderr_writer); - - emit_result(&mut output, Path::new("/tmp/x.yml"), &[]).unwrap(); - let stdout = String::from_utf8(buf.lock().unwrap().clone()).unwrap(); + let mut cap = Captured::human(); + emit_result(&mut cap.output, Path::new("/tmp/x.yml"), &[]).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("is valid")); } #[test] fn emit_result_errors_lists_each() { - let buf: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr_buf: std::sync::Arc>> = - std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stdout_writer = SharedWriter(std::sync::Arc::clone(&buf)); - let stderr_writer = SharedWriter(std::sync::Arc::clone(&stderr_buf)); - let mut output = StdioOutput::with_sinks(OutputMode::Human, stdout_writer, stderr_writer); - + let mut cap = Captured::human(); emit_result( - &mut output, + &mut cap.output, Path::new("/tmp/x.yml"), &[ValidationError { path: "pull_request_rules.0".into(), @@ -288,21 +273,9 @@ mod tests { ) .unwrap(); - let stdout = String::from_utf8(buf.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("has 1 error(s)")); assert!(stdout.contains("pull_request_rules.0")); assert!(stdout.contains("is not of type string")); } - - struct SharedWriter(std::sync::Arc>>); - - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-freeze/Cargo.toml b/crates/mergify-freeze/Cargo.toml index 53dd6ed7..06d62c98 100644 --- a/crates/mergify-freeze/Cargo.toml +++ b/crates/mergify-freeze/Cargo.toml @@ -19,6 +19,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" [dev-dependencies] +mergify-test-support = { path = "../mergify-test-support" } tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } wiremock = "0.6" diff --git a/crates/mergify-freeze/src/create.rs b/crates/mergify-freeze/src/create.rs index f32652d5..24fb9f9c 100644 --- a/crates/mergify-freeze/src/create.rs +++ b/crates/mergify-freeze/src/create.rs @@ -100,10 +100,7 @@ pub async fn run(opts: CreateOptions<'_>, output: &mut dyn Output) -> Result<(), #[cfg(test)] mod tests { - use std::io::Write; - - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use serde_json::json; use wiremock::Mock; use wiremock::MockServer; @@ -116,35 +113,6 @@ mod tests { use super::*; use crate::common::parse_naive_datetime; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - #[tokio::test] async fn run_posts_payload_with_optional_conditions_when_provided() { let server = MockServer::start().await; @@ -179,7 +147,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let matching = ["base=main".to_string()]; let exclude = ["label=hotfix".to_string()]; @@ -200,7 +168,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!( stdout.contains("Freeze created successfully"), "got: {stdout}" @@ -238,7 +206,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let empty: [String; 0] = []; run( diff --git a/crates/mergify-freeze/src/delete.rs b/crates/mergify-freeze/src/delete.rs index 08519b1d..c8d621ea 100644 --- a/crates/mergify-freeze/src/delete.rs +++ b/crates/mergify-freeze/src/delete.rs @@ -64,8 +64,7 @@ pub async fn run(opts: DeleteOptions<'_>, output: &mut dyn Output) -> Result<(), #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -75,35 +74,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - #[tokio::test] async fn run_posts_empty_body_when_no_reason_provided() { let server = MockServer::start().await; @@ -118,7 +88,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( DeleteOptions { @@ -141,7 +111,7 @@ mod tests { // require the key. assert!(map.is_empty(), "expected `{{}}` body, got {body}"); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!( stdout.contains("Freeze deleted successfully"), "got: {stdout}" @@ -161,7 +131,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( DeleteOptions { diff --git a/crates/mergify-freeze/src/list.rs b/crates/mergify-freeze/src/list.rs index e96efafc..80f619a0 100644 --- a/crates/mergify-freeze/src/list.rs +++ b/crates/mergify-freeze/src/list.rs @@ -227,7 +227,7 @@ fn write_separator(w: &mut dyn Write, widths: &[usize; 6]) -> std::io::Result<() #[cfg(test)] mod tests { use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use serde_json::json; use wiremock::Mock; use wiremock::MockServer; @@ -238,39 +238,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output(mode: OutputMode) -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - mode, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - - fn stdout_string(cap: &Captured) -> String { - String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap() - } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - fn freeze_sample() -> serde_json::Value { json!({ "id": "11111111-2222-3333-4444-555555555555", @@ -306,7 +273,7 @@ mod tests { let body = json!({"scheduled_freezes": [freeze.clone()]}); arrange(&server, body).await; - let mut cap = make_output(OutputMode::Json); + let mut cap = Captured::new(OutputMode::Json); let api_url = server.uri(); run( ListOptions { @@ -320,7 +287,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); assert_eq!(parsed, json!([freeze])); } @@ -332,7 +299,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, json!({"scheduled_freezes": []})).await; - let mut cap = make_output(OutputMode::Json); + let mut cap = Captured::new(OutputMode::Json); let api_url = server.uri(); run( ListOptions { @@ -346,7 +313,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); assert_eq!(parsed, json!([])); } @@ -356,7 +323,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, json!({"scheduled_freezes": []})).await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ListOptions { @@ -370,7 +337,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); assert!( stdout.contains("No scheduled freezes found"), "got: {stdout:?}" @@ -382,7 +349,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, json!({"scheduled_freezes": [freeze_sample()]})).await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ListOptions { @@ -396,7 +363,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); assert!(stdout.contains("Scheduled Freezes"), "got: {stdout}"); assert!(stdout.contains("emergency-fix"), "got: {stdout}"); assert!( @@ -435,7 +402,7 @@ mod tests { ) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ListOptions { @@ -449,7 +416,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); // The end column should render as a bare `-` (we don't pin // the surrounding whitespace because the table's column // widths depend on the row content). diff --git a/crates/mergify-freeze/src/update.rs b/crates/mergify-freeze/src/update.rs index c6111feb..1115bb90 100644 --- a/crates/mergify-freeze/src/update.rs +++ b/crates/mergify-freeze/src/update.rs @@ -94,8 +94,7 @@ pub async fn run(opts: UpdateOptions<'_>, output: &mut dyn Output) -> Result<(), #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use serde_json::json; use wiremock::Mock; use wiremock::MockServer; @@ -106,35 +105,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - #[tokio::test] async fn run_patches_only_user_supplied_fields() { // The whole point of PATCH semantics: only the fields the @@ -161,7 +131,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( UpdateOptions { @@ -204,7 +174,7 @@ mod tests { ); } - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!( stdout.contains("Freeze updated successfully"), "got: {stdout}" @@ -236,7 +206,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let empty: [String; 0] = []; run( diff --git a/crates/mergify-queue/Cargo.toml b/crates/mergify-queue/Cargo.toml index a1eae21e..c0e4310f 100644 --- a/crates/mergify-queue/Cargo.toml +++ b/crates/mergify-queue/Cargo.toml @@ -20,6 +20,7 @@ serde_json = "1.0" url = "2" [dev-dependencies] +mergify-test-support = { path = "../mergify-test-support" } tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } wiremock = "0.6" diff --git a/crates/mergify-queue/src/pause.rs b/crates/mergify-queue/src/pause.rs index 201d4140..ca81e36c 100644 --- a/crates/mergify-queue/src/pause.rs +++ b/crates/mergify-queue/src/pause.rs @@ -155,8 +155,7 @@ fn emit_confirmation(output: &mut dyn Output, response: &PauseResponse) -> std:: #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -167,24 +166,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - #[test] fn parse_reason_accepts_short() { assert_eq!( @@ -251,7 +232,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( PauseOptions { @@ -266,20 +247,9 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("Queue paused"), "got: {stdout:?}"); assert!(stdout.contains("deploy freeze"), "got: {stdout:?}"); assert!(stdout.contains("2026-04-23"), "got: {stdout:?}"); } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-queue/src/show.rs b/crates/mergify-queue/src/show.rs index add4e700..ec9f6e4f 100644 --- a/crates/mergify-queue/src/show.rs +++ b/crates/mergify-queue/src/show.rs @@ -470,7 +470,7 @@ fn write_condition_tree( #[cfg(test)] mod tests { use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use serde_json::json; use wiremock::Mock; use wiremock::MockServer; @@ -481,40 +481,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - stderr: SharedBytes, - } - - fn make_output(mode: OutputMode) -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - mode, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { - output, - stdout, - stderr, - } - } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - fn pull_response() -> serde_json::Value { json!({ "number": 123, @@ -570,7 +536,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, pull_response(), 200).await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ShowOptions { @@ -586,7 +552,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("PR #123"), "got: {stdout:?}"); assert!(stdout.contains("Position:"), "got: {stdout:?}"); assert!(stdout.contains("CI State:"), "got: {stdout:?}"); @@ -607,7 +573,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, pull_response(), 200).await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ShowOptions { @@ -623,7 +589,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); // Verbose table: every check name appears as its own row. assert!(stdout.contains("tests"), "got: {stdout:?}"); assert!(stdout.contains("linters"), "got: {stdout:?}"); @@ -645,7 +611,7 @@ mod tests { body["future_field"] = json!("preserved"); arrange(&server, body, 200).await; - let mut cap = make_output(OutputMode::Json); + let mut cap = Captured::new(OutputMode::Json); let api_url = server.uri(); run( ShowOptions { @@ -661,7 +627,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); assert_eq!(parsed["number"], json!(123)); assert_eq!(parsed["future_field"], json!("preserved")); @@ -677,7 +643,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); let err = run( ShowOptions { @@ -718,7 +684,7 @@ mod tests { }); arrange(&server, body, 200).await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( ShowOptions { @@ -734,7 +700,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!( stdout.contains("Waiting for mergeability check"), "got: {stdout:?}", @@ -811,12 +777,4 @@ mod tests { }; assert_eq!(child_label(&nested), "leaf"); } - - // Suppress dead-code warnings for the captured-stderr accessor: - // the existing tests use stdout assertions, but the field is - // wired up the same way as in pause/unpause/status for parity. - #[allow(dead_code)] - fn _stderr_accessor_lives(c: &Captured) -> SharedBytes { - std::sync::Arc::clone(&c.stderr) - } } diff --git a/crates/mergify-queue/src/status.rs b/crates/mergify-queue/src/status.rs index b88d0303..e1d8dc8a 100644 --- a/crates/mergify-queue/src/status.rs +++ b/crates/mergify-queue/src/status.rs @@ -454,8 +454,7 @@ fn group_by_scope<'a>(batches: &[&'a Batch]) -> IndexMap> #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -466,28 +465,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output(mode: OutputMode) -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - mode, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - - fn stdout_string(cap: &Captured) -> String { - String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap() - } - #[test] fn build_path_no_branch() { assert_eq!( @@ -624,7 +601,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -639,7 +616,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); assert_eq!(parsed, response); } @@ -659,7 +636,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -674,7 +651,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); assert!(stdout.contains("Merge Queue: owner/repo"), "got {stdout}"); assert!(stdout.contains("Queue is paused"), "got {stdout}"); assert!(stdout.contains("deploy freeze"), "got {stdout}"); @@ -695,7 +672,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -710,7 +687,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); assert!(stdout.contains("Queue is empty"), "got {stdout}"); } @@ -761,7 +738,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -776,7 +753,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); assert!(stdout.contains("Batches"), "got {stdout}"); assert!(stdout.contains("running"), "got {stdout}"); assert!(stdout.contains("checks 3/5"), "got {stdout}"); @@ -822,7 +799,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -837,7 +814,7 @@ mod tests { .await .unwrap(); - let stdout = stdout_string(&cap); + let stdout = cap.stdout(); // Two scopes → each labelled by its own name (no // generic "Batches" header). assert!(stdout.contains("frontend"), "got {stdout}"); @@ -861,7 +838,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -899,7 +876,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(OutputMode::Human); + let mut cap = Captured::human(); let api_url = server.uri(); run( StatusOptions { @@ -914,15 +891,4 @@ mod tests { .await .unwrap(); } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-queue/src/unpause.rs b/crates/mergify-queue/src/unpause.rs index c88f8858..a3f03151 100644 --- a/crates/mergify-queue/src/unpause.rs +++ b/crates/mergify-queue/src/unpause.rs @@ -48,8 +48,7 @@ fn emit_resumed(output: &mut dyn Output) -> std::io::Result<()> { #[cfg(test)] mod tests { - use mergify_core::OutputMode; - use mergify_core::StdioOutput; + use mergify_test_support::Captured; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -59,24 +58,6 @@ mod tests { use super::*; - type SharedBytes = std::sync::Arc>>; - - struct Captured { - output: StdioOutput, - stdout: SharedBytes, - } - - fn make_output() -> Captured { - let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let output = StdioOutput::with_sinks( - OutputMode::Human, - SharedWriter(std::sync::Arc::clone(&stdout)), - SharedWriter(std::sync::Arc::clone(&stderr)), - ); - Captured { output, stdout } - } - #[tokio::test] async fn run_unpauses_on_2xx() { let server = MockServer::start().await; @@ -88,7 +69,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); run( UnpauseOptions { @@ -101,7 +82,7 @@ mod tests { .await .unwrap(); - let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let stdout = cap.stdout(); assert!(stdout.contains("Queue resumed"), "got: {stdout:?}"); } @@ -115,7 +96,7 @@ mod tests { .mount(&server) .await; - let mut cap = make_output(); + let mut cap = Captured::human(); let api_url = server.uri(); let err = run( UnpauseOptions { @@ -131,15 +112,4 @@ mod tests { assert!(err.to_string().contains("not currently paused")); assert_eq!(err.exit_code(), mergify_core::ExitCode::MergifyApiError); } - - struct SharedWriter(SharedBytes); - impl Write for SharedWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0.lock().unwrap().extend_from_slice(bytes); - Ok(bytes.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } } diff --git a/crates/mergify-test-support/Cargo.toml b/crates/mergify-test-support/Cargo.toml new file mode 100644 index 00000000..a276da61 --- /dev/null +++ b/crates/mergify-test-support/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "mergify-test-support" +version = "0.0.0" +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +authors.workspace = true +description = "Shared test scaffolding for the mergify CLI Rust port. Not published." +publish = false + +[dependencies] +mergify-core = { path = "../mergify-core" } + +[lints] +workspace = true diff --git a/crates/mergify-test-support/src/lib.rs b/crates/mergify-test-support/src/lib.rs new file mode 100644 index 00000000..97c7def4 --- /dev/null +++ b/crates/mergify-test-support/src/lib.rs @@ -0,0 +1,114 @@ +//! Shared test scaffolding for the mergify CLI Rust port. +//! +//! Every ported command writes through a `&mut dyn Output`, so tests +//! need a way to feed it an in-memory sink and read back what the +//! command produced. Every test module used to re-roll the same +//! `SharedBytes` / `SharedWriter` / `Captured` / `make_output` trio +//! by hand — about 30 LOC × ~15 files of pure boilerplate, drifting +//! over time (some had `stderr`, some didn't; some named fields, some +//! used helper functions). This crate is the one canonical version. +//! +//! Typical usage: +//! +//! ```ignore +//! use mergify_test_support::Captured; +//! +//! let mut cap = Captured::human(); +//! run(options, &mut cap.output).await.unwrap(); +//! assert!(cap.stdout().contains("ok")); +//! ``` +//! +//! Lives as a separate `dev-dependencies` crate (rather than a feature +//! on `mergify-core`) so the test-only types never leak into a +//! production build. + +use std::io::Write; +use std::sync::Arc; +use std::sync::Mutex; + +use mergify_core::OutputMode; +use mergify_core::StdioOutput; + +/// Mutex-protected, `Arc`-shared byte buffer the captured output is +/// streamed into. Exposed so tests that need to take a snapshot +/// mid-run (or share the buffer across threads) can grab a clone +/// of the `Arc`. +pub type SharedBytes = Arc>>; + +/// `Write` adapter over a [`SharedBytes`] handle. Used as the +/// stdout/stderr sink for [`StdioOutput::with_sinks`]. +pub struct SharedWriter(pub SharedBytes); + +impl Write for SharedWriter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +/// In-memory capture of an [`StdioOutput`] for tests. +/// +/// Construct with [`Captured::human`] or [`Captured::new`], pass +/// `&mut cap.output` to the command under test, then assert on the +/// recorded bytes via [`Captured::stdout`] / [`Captured::stderr`]. +pub struct Captured { + pub output: StdioOutput, + stdout: SharedBytes, + stderr: SharedBytes, +} + +impl Captured { + /// Capture an [`OutputMode::Human`] output. The most common + /// case — every command renders human output by default and + /// only flips to JSON when the user passes `--json`. + #[must_use] + pub fn human() -> Self { + Self::new(OutputMode::Human) + } + + /// Capture with an explicit [`OutputMode`]. + #[must_use] + pub fn new(mode: OutputMode) -> Self { + let stdout: SharedBytes = Arc::new(Mutex::new(Vec::new())); + let stderr: SharedBytes = Arc::new(Mutex::new(Vec::new())); + let output = StdioOutput::with_sinks( + mode, + SharedWriter(Arc::clone(&stdout)), + SharedWriter(Arc::clone(&stderr)), + ); + Self { + output, + stdout, + stderr, + } + } + + /// Captured stdout as a UTF-8 string. Panics if the captured + /// bytes aren't valid UTF-8 — tests shouldn't be producing + /// invalid UTF-8 in the first place, and panicking with a clear + /// site beats silently lossy `String::from_utf8_lossy`. + #[must_use] + pub fn stdout(&self) -> String { + String::from_utf8(self.stdout.lock().unwrap().clone()) + .expect("captured stdout must be UTF-8") + } + + /// Captured stderr as a UTF-8 string. See [`Self::stdout`] for + /// the UTF-8 caveat. + #[must_use] + pub fn stderr(&self) -> String { + String::from_utf8(self.stderr.lock().unwrap().clone()) + .expect("captured stderr must be UTF-8") + } + + /// Snapshot the captured stdout bytes (without UTF-8 validation). + /// Useful when the command writes a binary or pre-encoded payload + /// the test wants to compare byte-for-byte. + #[must_use] + pub fn stdout_bytes(&self) -> Vec { + self.stdout.lock().unwrap().clone() + } +} From 5143191a01dcaa582f5d1d96c5ec7443a85ea1f2 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 19 May 2026 14:53:08 +0200 Subject: [PATCH 3/3] refactor(core): introduce CommandContext for the queue+freeze prelude Every `queue` and `freeze` command opens with the same four-line ritual: resolve repository slug, resolve token, resolve API URL, build a Mergify-flavored HTTP client. Wrap those into a `CommandContext::resolve(...)` constructor + `ctx.mergify_client()` builder living in a new `mergify_core::command_context` module. The eight queue/freeze command preludes shrink from let repository = auth::resolve_repository(opts.repository)?; let token = auth::resolve_token(opts.token)?; let api_url = auth::resolve_api_url(opts.api_url)?; let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; to let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; let client = ctx.mergify_client()?; `auth::resolve_*` stay public for the specialized callers that don't fit the shape: `config validate` needs no repository, `config simulate` derives the repo from a PR URL, `ci scopes-send` resolves the repo from CI-env vars (different fallback chain). Those keep wiring up the lower-level helpers by hand. Net `-73 / +63` across 9 files; conceptually the bigger win is that "the prelude of a Mergify command" is now a named concept with one fix-it-once point. Co-Authored-By: Claude Opus 4.7 Change-Id: I1b272570f6802dd13438c3d9baf26709b196574d --- crates/mergify-core/src/command_context.rs | 67 ++++++++++++++++++++++ crates/mergify-core/src/lib.rs | 2 + crates/mergify-freeze/src/create.rs | 17 +++--- crates/mergify-freeze/src/delete.rs | 16 +++--- crates/mergify-freeze/src/list.rs | 17 +++--- crates/mergify-freeze/src/update.rs | 16 +++--- crates/mergify-queue/src/pause.rs | 19 +++--- crates/mergify-queue/src/show.rs | 13 ++--- crates/mergify-queue/src/status.rs | 19 +++--- crates/mergify-queue/src/unpause.rs | 17 +++--- 10 files changed, 130 insertions(+), 73 deletions(-) create mode 100644 crates/mergify-core/src/command_context.rs diff --git a/crates/mergify-core/src/command_context.rs b/crates/mergify-core/src/command_context.rs new file mode 100644 index 00000000..6bcc7a4d --- /dev/null +++ b/crates/mergify-core/src/command_context.rs @@ -0,0 +1,67 @@ +//! Per-command "resolved context" + Mergify HTTP client builder. +//! +//! Every Mergify-API command starts the same way: resolve the +//! repository slug, the bearer token, and the API URL via the +//! standard fallback chain (flag → env → `gh auth token` / git +//! remote / default), then build a typed [`HttpClient`] from them. +//! [`CommandContext`] bundles those three pieces with a +//! `mergify_client()` builder so the prelude shrinks from a four-line +//! ritual to two: +//! +//! ```ignore +//! let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; +//! let client = ctx.mergify_client()?; +//! ``` +//! +//! Specialized commands that don't fit the shape (`config validate` +//! needs no repository; `ci scopes-send` resolves the repo from CI +//! env; `config simulate` derives it from a PR URL) keep wiring up +//! the lower-level [`auth::resolve_*`] / [`HttpClient::new`] calls +//! by hand. +//! +//! [`auth::resolve_*`]: crate::auth + +use url::Url; + +use crate::auth; +use crate::error::CliError; +use crate::http::ApiFlavor; +use crate::http::Client as HttpClient; + +/// Resolved repository / token / API URL for a Mergify-API command. +pub struct CommandContext { + pub repository: String, + pub token: String, + pub api_url: Url, +} + +impl CommandContext { + /// Resolve all three pieces of context using the standard + /// fallback chain. Used by the `queue` and `freeze` command + /// families — every member of those groups needs the same + /// shape. + /// + /// # Errors + /// + /// Surfaces the first resolution failure as + /// [`CliError::Configuration`]. + pub fn resolve( + repository: Option<&str>, + token: Option<&str>, + api_url: Option<&str>, + ) -> Result { + Ok(Self { + repository: auth::resolve_repository(repository)?, + token: auth::resolve_token(token)?, + api_url: auth::resolve_api_url(api_url)?, + }) + } + + /// Build a Mergify-flavored [`HttpClient`] from this context. + /// Clones the API URL so the [`CommandContext`] stays usable + /// afterwards — callers typically still need `self.repository` + /// to format URL paths. + pub fn mergify_client(&self) -> Result { + HttpClient::new(self.api_url.clone(), &self.token, ApiFlavor::Mergify) + } +} diff --git a/crates/mergify-core/src/lib.rs b/crates/mergify-core/src/lib.rs index 8b731465..d362f1d9 100644 --- a/crates/mergify-core/src/lib.rs +++ b/crates/mergify-core/src/lib.rs @@ -16,11 +16,13 @@ //! in subsequent sub-phases. pub mod auth; +pub mod command_context; pub mod error; pub mod exit_code; pub mod http; pub mod output; +pub use command_context::CommandContext; pub use error::CliError; pub use exit_code::ExitCode; pub use http::{ApiFlavor, Client as HttpClient, DeleteOutcome, RetryPolicy}; diff --git a/crates/mergify-freeze/src/create.rs b/crates/mergify-freeze/src/create.rs index 24fb9f9c..083682ee 100644 --- a/crates/mergify-freeze/src/create.rs +++ b/crates/mergify-freeze/src/create.rs @@ -11,11 +11,9 @@ use std::io::Write; use chrono::NaiveDateTime; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use serde::Serialize; use crate::common::NaiveDateTimeWire; @@ -56,9 +54,7 @@ struct CreatePayload<'a> { /// Run the `freeze create` command. pub async fn run(opts: CreateOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; let timezone = match opts.timezone { Some(tz) => tz.to_string(), None => detect_local_timezone()?, @@ -85,10 +81,13 @@ pub async fn run(opts: CreateOptions<'_>, output: &mut dyn Output) -> Result<(), }, }; - output.status(&format!("Creating scheduled freeze for {repository}…"))?; + output.status(&format!( + "Creating scheduled freeze for {repo}…", + repo = ctx.repository, + ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; - let path = format!("/v1/repos/{repository}/scheduled_freeze"); + let client = ctx.mergify_client()?; + let path = format!("/v1/repos/{}/scheduled_freeze", ctx.repository); let freeze: ScheduledFreeze = client.post(&path, &payload).await?; output.emit(&(), &mut |w: &mut dyn Write| { diff --git a/crates/mergify-freeze/src/delete.rs b/crates/mergify-freeze/src/delete.rs index c8d621ea..fa2339ec 100644 --- a/crates/mergify-freeze/src/delete.rs +++ b/crates/mergify-freeze/src/delete.rs @@ -9,11 +9,9 @@ use std::io::Write; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use serde::Serialize; pub struct DeleteOptions<'a> { @@ -36,22 +34,22 @@ struct DeletePayload<'a> { /// Run the `freeze delete` command. pub async fn run(opts: DeleteOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; output.status(&format!( - "Deleting scheduled freeze {id} on {repository}…", + "Deleting scheduled freeze {id} on {repo}…", id = opts.freeze_id, + repo = ctx.repository, ))?; let payload = DeletePayload { delete_reason: opts.delete_reason, }; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let client = ctx.mergify_client()?; let path = format!( - "/v1/repos/{repository}/scheduled_freeze/{id}/delete", + "/v1/repos/{repo}/scheduled_freeze/{id}/delete", + repo = ctx.repository, id = opts.freeze_id, ); client.post_no_response(&path, &payload).await?; diff --git a/crates/mergify-freeze/src/list.rs b/crates/mergify-freeze/src/list.rs index 80f619a0..c06d32d0 100644 --- a/crates/mergify-freeze/src/list.rs +++ b/crates/mergify-freeze/src/list.rs @@ -19,11 +19,9 @@ use std::io::Write; use anstyle::AnsiColor; use chrono::DateTime; use chrono::Utc; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use mergify_tui::Theme; use crate::common::ScheduledFreeze; @@ -39,14 +37,15 @@ pub struct ListOptions<'a> { /// Run the `freeze list` command. pub async fn run(opts: ListOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; - output.status(&format!("Fetching scheduled freezes for {repository}…"))?; + output.status(&format!( + "Fetching scheduled freezes for {repo}…", + repo = ctx.repository, + ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; - let path = format!("/v1/repos/{repository}/scheduled_freeze"); + let client = ctx.mergify_client()?; + let path = format!("/v1/repos/{}/scheduled_freeze", ctx.repository); let raw: serde_json::Value = client.get(&path).await?; // Python's `list_freezes` returns `data["scheduled_freezes"]` diff --git a/crates/mergify-freeze/src/update.rs b/crates/mergify-freeze/src/update.rs index 1115bb90..578d1834 100644 --- a/crates/mergify-freeze/src/update.rs +++ b/crates/mergify-freeze/src/update.rs @@ -10,11 +10,9 @@ use std::io::Write; use chrono::NaiveDateTime; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use serde::Serialize; use crate::common::NaiveDateTimeWire; @@ -60,9 +58,7 @@ struct UpdatePayload<'a> { /// Run the `freeze update` command. pub async fn run(opts: UpdateOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; let payload = UpdatePayload { reason: opts.reason, @@ -74,13 +70,15 @@ pub async fn run(opts: UpdateOptions<'_>, output: &mut dyn Output) -> Result<(), }; output.status(&format!( - "Updating scheduled freeze {id} on {repository}…", + "Updating scheduled freeze {id} on {repo}…", id = opts.freeze_id, + repo = ctx.repository, ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let client = ctx.mergify_client()?; let path = format!( - "/v1/repos/{repository}/scheduled_freeze/{id}", + "/v1/repos/{repo}/scheduled_freeze/{id}", + repo = ctx.repository, id = opts.freeze_id, ); let freeze: ScheduledFreeze = client.patch(&path, &payload).await?; diff --git a/crates/mergify-queue/src/pause.rs b/crates/mergify-queue/src/pause.rs index ca81e36c..09156215 100644 --- a/crates/mergify-queue/src/pause.rs +++ b/crates/mergify-queue/src/pause.rs @@ -16,11 +16,9 @@ use std::io::IsTerminal; use std::io::Write; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use serde::Deserialize; use serde::Serialize; @@ -74,20 +72,21 @@ pub async fn run(opts: PauseOptions<'_>, output: &mut dyn Output) -> Result<(), // Resolve auth/repo first so the prompt names the *actual* repo // (including the `GITHUB_REPOSITORY` fallback) and so a missing // repo or token fails loudly *before* we ask for confirmation. - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; confirm( opts.yes_i_am_sure, std::io::stdin().is_terminal(), - &repository, + &ctx.repository, )?; - output.status(&format!("Pausing merge queue for {repository}…"))?; + output.status(&format!( + "Pausing merge queue for {repo}…", + repo = ctx.repository, + ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; - let path = format!("/v1/repos/{repository}/merge-queue/pause"); + let client = ctx.mergify_client()?; + let path = format!("/v1/repos/{}/merge-queue/pause", ctx.repository); let resp: PauseResponse = client .put( &path, diff --git a/crates/mergify-queue/src/show.rs b/crates/mergify-queue/src/show.rs index ec9f6e4f..50cdf262 100644 --- a/crates/mergify-queue/src/show.rs +++ b/crates/mergify-queue/src/show.rs @@ -30,11 +30,9 @@ use anstyle::AnsiColor; use anstyle::Style; use chrono::DateTime; use chrono::Utc; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use mergify_tui::Theme; use mergify_tui::relative_time; use mergify_tui::tree; @@ -105,13 +103,12 @@ const fn default_match_true() -> bool { /// Run the `queue show` command. pub async fn run(opts: ShowOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let client = ctx.mergify_client()?; let path = format!( - "/v1/repos/{repository}/merge-queue/pull/{pr_number}", + "/v1/repos/{repo}/merge-queue/pull/{pr_number}", + repo = ctx.repository, pr_number = opts.pr_number, ); diff --git a/crates/mergify-queue/src/status.rs b/crates/mergify-queue/src/status.rs index e1d8dc8a..8807b62b 100644 --- a/crates/mergify-queue/src/status.rs +++ b/crates/mergify-queue/src/status.rs @@ -33,11 +33,9 @@ use anstyle::Style; use chrono::DateTime; use chrono::Utc; use indexmap::IndexMap; -use mergify_core::ApiFlavor; use mergify_core::CliError; -use mergify_core::HttpClient; +use mergify_core::CommandContext; use mergify_core::Output; -use mergify_core::auth; use mergify_tui::Theme; use mergify_tui::relative_time; use mergify_tui::tree; @@ -125,14 +123,15 @@ struct Author { /// Run the `queue status` command. pub async fn run(opts: StatusOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; - output.status(&format!("Fetching merge queue status for {repository}…"))?; + output.status(&format!( + "Fetching merge queue status for {repo}…", + repo = ctx.repository, + ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; - let path = build_path(&repository, opts.branch); + let client = ctx.mergify_client()?; + let path = build_path(&ctx.repository, opts.branch); let raw: serde_json::Value = client.get(&path).await?; @@ -141,7 +140,7 @@ pub async fn run(opts: StatusOptions<'_>, output: &mut dyn Output) -> Result<(), } else { let view: StatusView = serde_json::from_value(raw) .map_err(|e| CliError::Generic(format!("decode merge queue status response: {e}")))?; - emit_human(output, &repository, &view)?; + emit_human(output, &ctx.repository, &view)?; } Ok(()) } diff --git a/crates/mergify-queue/src/unpause.rs b/crates/mergify-queue/src/unpause.rs index a3f03151..14c990e6 100644 --- a/crates/mergify-queue/src/unpause.rs +++ b/crates/mergify-queue/src/unpause.rs @@ -7,12 +7,10 @@ use std::io::Write; -use mergify_core::ApiFlavor; use mergify_core::CliError; +use mergify_core::CommandContext; use mergify_core::DeleteOutcome; -use mergify_core::HttpClient; use mergify_core::Output; -use mergify_core::auth; pub struct UnpauseOptions<'a> { pub repository: Option<&'a str>, @@ -22,14 +20,15 @@ pub struct UnpauseOptions<'a> { /// Run the `queue unpause` command. pub async fn run(opts: UnpauseOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { - let repository = auth::resolve_repository(opts.repository)?; - let token = auth::resolve_token(opts.token)?; - let api_url = auth::resolve_api_url(opts.api_url)?; + let ctx = CommandContext::resolve(opts.repository, opts.token, opts.api_url)?; - output.status(&format!("Unpausing merge queue for {repository}…"))?; + output.status(&format!( + "Unpausing merge queue for {repo}…", + repo = ctx.repository, + ))?; - let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; - let path = format!("/v1/repos/{repository}/merge-queue/pause"); + let client = ctx.mergify_client()?; + let path = format!("/v1/repos/{}/merge-queue/pause", ctx.repository); match client.delete_if_exists(&path).await? { DeleteOutcome::Deleted => {