From c86649e5a4d04a7704d95e9e489ffd168f264ec5 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Mon, 11 May 2026 14:05:10 +0200 Subject: [PATCH 1/2] test(queue): add live smoke test for queue show MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the contract for ``mergify queue show`` 404 handling before the Rust port lands on top. Same test exercises Python at this PR's CI and Rust on the port commit's rebase. Uses a PR number far above the test repo's actual PR count to force the 404 path. That's robust against the test repo's queue state (PR #1 may or may not be queued at any given moment) and exercises the parts that would silently break on URL or schema drift: endpoint reachability, auth, and 404 → ``MERGIFY_API_ERROR`` exit code (6) mapping with the ``not in the merge queue`` message. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I944234ba2d22d99b410f4b8c91e56f0d8a49a9f7 --- func-tests/test_live_smoke.py | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/func-tests/test_live_smoke.py b/func-tests/test_live_smoke.py index 7e2948fc..081810d8 100644 --- a/func-tests/test_live_smoke.py +++ b/func-tests/test_live_smoke.py @@ -149,6 +149,53 @@ def test_queue_status( ) +def test_queue_show_not_in_queue( + live_admin_token: str, + cli: typing.Callable[..., typing.Any], +) -> None: + """`GET /v1/repos/{owner}/{repo}/merge-queue/pull/{n}` 404 path. + + Uses the admin-scoped token because all queue endpoints (read + or write) require queue-management scope on the test repo; + the CI-scoped token is rejected with 403. + + Calls with a PR number that is almost certainly not in the + queue (the test repo has far fewer than this many PRs). + Both Python and Rust special-case 404 with the same + user-facing message and ``MERGIFY_API_ERROR`` exit code (6) + — that contract is what this test pins. + + Testing the 404 path (instead of a real queued PR) makes the + test independent of whether PR #1 happens to be queued at run + time. The endpoint reachability, auth, and 404 mapping are + the parts that would silently break on a URL or schema drift. + """ + # Group-level options come BEFORE the subcommand — same + # invocation shape as the queue status smoke test (Click + # requires it for Python, Rust accepts both via clap's + # ``global = true``). + result = cli( + "queue", + "--api-url", + API_URL, + "--token", + live_admin_token, + "--repository", + REPOSITORY, + "show", + "99999999", + ) + assert result.returncode == 6, ( + f"expected MERGIFY_API_ERROR (6), got {result.returncode}\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + combined = (result.stdout + result.stderr).lower() + assert "not in the merge queue" in combined, ( + f"expected 'not in the merge queue' message\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + + def test_ci_git_refs_fallback( cli: typing.Callable[..., typing.Any], ) -> None: From fe67fc573fb37165a4ed41f96d3893ab8830afd2 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Mon, 11 May 2026 09:13:03 +0200 Subject: [PATCH 2/2] feat(rust): port queue show to native Rust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rust binary now serves ``mergify queue show `` natively. The Python implementation (``mergify_cli/queue/cli.py:show`` plus the eight rendering helpers it depended on, plus ``mergify_cli/queue/api.py``) is removed in the same PR — the port-and-delete rule keeps a single live copy of every command. This is the last command in the ``queue`` group, so the whole ``mergify_cli/queue/`` Python package goes away. Closes the gap noted in #1380's commit message: ``mergify queue --help`` now lists ``show`` alongside ``pause`` / ``unpause`` / ``status``. ``mergify queue show [-v] [--json] [-r REPO] [-t TOKEN] [-u URL]``: 1. Resolves repository / token / API URL via the shared ``mergify_core::auth`` resolver. 2. Fetches ``GET /v1/repos//merge-queue/pull/`` through the new ``HttpClient::get_if_exists`` helper. On 404 the command exits with ``MERGIFY_API_ERROR`` and the message ``PR # is not in the merge queue``, matching the Python implementation. Other 4xx/5xx surface as normal API errors. 3. With ``--json``: pretty-prints the raw response. Schema is the Mergify API contract, so unknown fields survive verbatim. 4. Without ``--json``: renders the metadata block (position / priority / queue rule / queued / ETA), then a CI-state line and a checks section, then a conditions section. ``--verbose`` switches the checks summary to a full table and the conditions summary to a tree (``├── └── │ ``) instead of the compact ``N/M met`` summary with bullet-listed failures. New plumbing in ``mergify-core::http``: - ``Client::get_if_exists(&path) -> Result, _>`` — GET that returns ``None`` on 404. Mirrors ``delete_if_exists`` for read-only endpoints where "not found" is a meaningful caller branch rather than a server failure. Reuses the same retry policy, bearer-auth injection, and flavor-aware error mapping as ``get`` / ``post`` / ``put``. Tests: - 7 new unit tests in ``crates/mergify-queue/src/show.rs``: compact metadata + checks summary + failing-conditions block; verbose checks table + conditions tree; JSON passthrough with a synthetic ``future_field`` to verify unknown fields survive; 404 → ``MergifyApi`` error with the right message; missing ``mergeability_check`` falls through to "Waiting for mergeability check..."; condition-group summarization (two labels joined with ``or``; truncation at 3+ labels); aggregator recursion (``any of`` / ``all of`` / ``not`` falls through to the first leaf). Wiring: - ``crates/mergify-cli/src/main.rs``: adds ``Show(ShowCliArgs)`` to ``QueueSubcommand``, dispatch to ``mergify_queue::show::run``. Adds ``("queue", "show")`` to ``NATIVE_COMMANDS``. ``ShowCliArgs`` carries the positional ``pr_number: u64`` plus ``--verbose`` and ``--json`` flags. Python deletions: - ``mergify_cli/queue/__init__.py`` / ``cli.py`` / ``api.py``: removed entirely. The whole package goes away — all four ``queue`` subcommands are now Rust-native. - ``mergify_cli/cli.py``: drops the ``from mergify_cli.queue import cli as queue_cli_mod`` import and the ``cli.add_command(queue_cli_mod.queue)`` call. - ``mergify_cli/tests/queue/test_cli.py``: deleted entirely (it only covered ``_relative_time``, which lives in ``mergify-tui::time`` now). - ``mergify_cli/tests/queue/test_show.py``: deleted entirely. - ``mergify_cli/tests/queue/test_skill.py``: drops the click import; the skill-reference check now consults the binary alone (no parallel click-command list to merge). Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I6c265303a37642529dbbcef6f255eb429407a1d2 --- crates/mergify-cli/src/main.rs | 64 ++ crates/mergify-core/src/http.rs | 199 ++++-- crates/mergify-queue/src/lib.rs | 1 + crates/mergify-queue/src/show.rs | 838 ++++++++++++++++++++++++++ mergify_cli/cli.py | 2 - mergify_cli/queue/__init__.py | 0 mergify_cli/queue/api.py | 62 -- mergify_cli/queue/cli.py | 313 ---------- mergify_cli/tests/queue/test_cli.py | 54 -- mergify_cli/tests/queue/test_show.py | 363 ----------- mergify_cli/tests/queue/test_skill.py | 16 +- 11 files changed, 1064 insertions(+), 848 deletions(-) create mode 100644 crates/mergify-queue/src/show.rs delete mode 100644 mergify_cli/queue/__init__.py delete mode 100644 mergify_cli/queue/api.py delete mode 100644 mergify_cli/queue/cli.py delete mode 100644 mergify_cli/tests/queue/test_cli.py delete mode 100644 mergify_cli/tests/queue/test_show.py diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index 12dca9ec..a1776392 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -27,6 +27,7 @@ use mergify_config::simulate::SimulateOptions; use mergify_core::OutputMode; use mergify_core::StdioOutput; use mergify_queue::pause::PauseOptions; +use mergify_queue::show::ShowOptions; use mergify_queue::status::StatusOptions; use mergify_queue::unpause::UnpauseOptions; @@ -89,6 +90,7 @@ const NATIVE_COMMANDS: &[(&str, &str)] = &[ ("queue", "pause"), ("queue", "unpause"), ("queue", "status"), + ("queue", "show"), ]; /// Native commands the Rust binary handles without delegating to @@ -102,6 +104,7 @@ enum NativeCommand { QueuePause(QueuePauseOpts), QueueUnpause(QueueUnpauseOpts), QueueStatus(QueueStatusOpts), + QueueShow(QueueShowOpts), } struct ConfigSimulateOpts { @@ -144,6 +147,15 @@ struct QueueStatusOpts { output_json: bool, } +struct QueueShowOpts { + repository: Option, + token: Option, + api_url: Option, + pr_number: u64, + verbose: bool, + output_json: bool, +} + /// Heuristic: does argv look like the user intended a native /// subcommand? /// @@ -193,6 +205,7 @@ fn is_help_or_version(err: &clap::Error) -> bool { /// — the corresponding exit code is the one chosen by the command /// implementation (typically [`mergify_core::ExitCode::Configuration`] /// = 8), not 2. +#[allow(clippy::too_many_lines)] // mostly mechanical match arms fn detect_native(argv: &[String]) -> Option { let looks_native = looks_native(argv); @@ -303,9 +316,28 @@ fn detect_native(argv: &[String]) -> Option { branch, output_json: json, })), + Subcommands::Queue(QueueArgs { + repository, + token, + api_url, + command: + QueueSubcommand::Show(ShowCliArgs { + pr_number, + verbose, + json, + }), + }) => Some(NativeCommand::QueueShow(QueueShowOpts { + repository, + token, + api_url, + pr_number, + verbose, + output_json: json, + })), } } +#[allow(clippy::too_many_lines)] // mostly mechanical match arms fn run_native(cmd: NativeCommand) -> ExitCode { let rt = match tokio::runtime::Builder::new_current_thread() .enable_all() @@ -394,6 +426,20 @@ fn run_native(cmd: NativeCommand) -> ExitCode { ) .await } + NativeCommand::QueueShow(opts) => { + mergify_queue::show::run( + ShowOptions { + repository: opts.repository.as_deref(), + token: opts.token.as_deref(), + api_url: opts.api_url.as_deref(), + pr_number: opts.pr_number, + verbose: opts.verbose, + output_json: opts.output_json, + }, + &mut output, + ) + .await + } } }); @@ -567,6 +613,8 @@ enum QueueSubcommand { Unpause, /// Show merge queue status for the repository. Status(StatusCliArgs), + /// Show detailed state of a pull request in the merge queue. + Show(ShowCliArgs), } #[derive(clap::Args)] @@ -591,3 +639,19 @@ struct StatusCliArgs { #[arg(long, default_value_t = false)] json: bool, } + +#[derive(clap::Args)] +struct ShowCliArgs { + /// Pull request number to inspect. + #[arg(value_name = "PR_NUMBER")] + pr_number: u64, + + /// Show the full checks table and the conditions tree instead + /// of compact summaries. + #[arg(long, short = 'v', default_value_t = false)] + verbose: bool, + + /// Emit the raw API response as a single JSON document. + #[arg(long, default_value_t = false)] + json: bool, +} diff --git a/crates/mergify-core/src/http.rs b/crates/mergify-core/src/http.rs index 42dcb057..b17e3526 100644 --- a/crates/mergify-core/src/http.rs +++ b/crates/mergify-core/src/http.rs @@ -129,6 +129,23 @@ impl Client { self.decode_json(resp).await } + /// GET `path`, returning `None` on 404. Other 4xx/5xx responses + /// surface as the normal `CliError` API failure. Mirrors + /// [`Self::delete_if_exists`] but for read-only endpoints where + /// "not found" is a meaningful caller branch (e.g. `queue show` + /// must distinguish "PR not in queue" from a genuine API + /// failure). + pub async fn get_if_exists( + &self, + path: &str, + ) -> Result, CliError> { + let url = self.join(path)?; + match self.execute_request_optional(self.inner.get(url)).await? { + Some(resp) => self.decode_json(resp).await.map(Some), + None => Ok(None), + } + } + /// POST `body` as JSON to `path` and deserialize the JSON /// response as `T`. pub async fn post( @@ -197,14 +214,29 @@ impl Client { .map_err(|e| self.api_error(format!("invalid path {path:?}: {e}"))) } - /// Execute a request that cares only about the HTTP status. + /// Single retry/auth/error driver behind every public verb. /// - /// Used by [`Self::delete_if_exists`] — the response body (if - /// any) is discarded. - async fn execute_status( + /// `tolerate_not_found` lets callers opt into "404 is a + /// caller-branch, not an error" semantics: + /// + /// - `false` (default for `get` / `post` / `put`): 404 surfaces + /// as a [`CliError`] like any other 4xx. + /// - `true` (for `get_if_exists` / `delete_if_exists`): 404 + /// short-circuits to `Ok(None)`. The HTTP body is dropped. + /// + /// Success (2xx) always returns `Ok(Some(response))` — the + /// caller decides whether to decode the body, drop it, or + /// map it to a domain type. + /// + /// 5xx is retried with exponential backoff (`self.retry`); + /// transient send errors (timeout / connect) are retried with + /// the same backoff. Other terminal errors and non-5xx 4xx + /// fail immediately. + async fn execute_with_retry( &self, builder: reqwest::RequestBuilder, - ) -> Result { + tolerate_not_found: bool, + ) -> Result, CliError> { let mut backoff = self.retry.initial_backoff; let mut last_message = String::from("HTTP request failed without response"); @@ -223,10 +255,10 @@ impl Client { Ok(resp) => { let status = resp.status(); if status.is_success() { - return Ok(DeleteOutcome::Deleted); + return Ok(Some(resp)); } - if status == StatusCode::NOT_FOUND { - return Ok(DeleteOutcome::NotFound); + if tolerate_not_found && status == StatusCode::NOT_FOUND { + return Ok(None); } last_message = error_message(status, resp).await; if status.is_server_error() && attempt + 1 < self.retry.max_attempts { @@ -249,49 +281,40 @@ impl Client { Err(self.api_error(last_message)) } + /// Send a request that must return a response. 404 is treated + /// like any other 4xx (caller error → [`CliError`]). async fn execute_request( &self, builder: reqwest::RequestBuilder, ) -> Result { - let mut backoff = self.retry.initial_backoff; - let mut last_message = String::from("HTTP request failed without response"); + // `tolerate_not_found = false` means the driver never + // returns `None`; `Option::expect` documents that invariant. + Ok(self + .execute_with_retry(builder, false) + .await? + .expect("execute_with_retry returned None despite tolerate_not_found=false")) + } - for attempt in 0..self.retry.max_attempts { - let Some(cloned) = builder.try_clone() else { - return Err(self.api_error( - "request body is not cloneable (streaming?) — cannot retry".into(), - )); - }; - let req = match &self.token { - Some(token) => cloned.bearer_auth(token), - None => cloned, - }; + /// Send a request where 404 is a routine caller branch + /// rather than a server failure. Used by [`Self::get_if_exists`]. + async fn execute_request_optional( + &self, + builder: reqwest::RequestBuilder, + ) -> Result, CliError> { + self.execute_with_retry(builder, true).await + } - match req.send().await { - Ok(resp) => { - let status = resp.status(); - if status.is_success() { - return Ok(resp); - } - last_message = error_message(status, resp).await; - if status.is_server_error() && attempt + 1 < self.retry.max_attempts { - tokio::time::sleep(backoff).await; - backoff *= 2; - continue; - } - return Err(self.api_error(last_message)); - } - Err(e) if is_transient(&e) && attempt + 1 < self.retry.max_attempts => { - last_message = format!("network error: {e}"); - tokio::time::sleep(backoff).await; - backoff *= 2; - } - Err(e) => { - return Err(self.api_error(self.terminal_send_error_message(&e))); - } - } + /// Send a request that cares only about the HTTP status. + /// Used by [`Self::delete_if_exists`] — the response body + /// (if any) is discarded. + async fn execute_status( + &self, + builder: reqwest::RequestBuilder, + ) -> Result { + match self.execute_with_retry(builder, true).await? { + Some(_) => Ok(DeleteOutcome::Deleted), + None => Ok(DeleteOutcome::NotFound), } - Err(self.api_error(last_message)) } async fn decode_json( @@ -708,4 +731,92 @@ mod tests { msg.len() ); } + + #[tokio::test] + async fn get_if_exists_returns_some_on_2xx() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/foo")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_json(Foo { bar: 7 })) + .expect(1) + .mount(&server) + .await; + + let client = fast_client(&server, ApiFlavor::Mergify); + let got: Option = client.get_if_exists("/foo").await.unwrap(); + assert_eq!(got, Some(Foo { bar: 7 })); + } + + #[tokio::test] + async fn get_if_exists_returns_none_on_404() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/missing")) + .respond_with(ResponseTemplate::new(404).set_body_string("not found")) + .expect(1) + .mount(&server) + .await; + + let client = fast_client(&server, ApiFlavor::Mergify); + let got: Option = client.get_if_exists("/missing").await.unwrap(); + assert!(got.is_none(), "expected None on 404, got {got:?}"); + } + + #[tokio::test] + async fn get_if_exists_surfaces_other_4xx_as_error() { + // 403 / 401 / 422 etc. are real failures, not "doesn't + // exist" — they must surface as `CliError`. Only 404 is + // mapped to `None`. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/forbidden")) + .respond_with(ResponseTemplate::new(403).set_body_string(r#"{"detail":"nope"}"#)) + .expect(1) + .mount(&server) + .await; + + let client = fast_client(&server, ApiFlavor::Mergify); + let err = client.get_if_exists::("/forbidden").await.unwrap_err(); + assert!( + matches!(err, CliError::MergifyApi(_)), + "expected MergifyApi, got {err:?}", + ); + assert!(err.to_string().contains("403")); + } + + #[tokio::test] + async fn get_if_exists_retries_5xx_then_succeeds() { + // Same retry semantics as `get`: a 500 on the first + // attempt should not short-circuit; the second attempt's + // 200 must be returned as `Some`. + struct FlakyRespond { + calls: Arc, + } + impl Respond for FlakyRespond { + fn respond(&self, _req: &Request) -> ResponseTemplate { + let n = self.calls.fetch_add(1, Ordering::SeqCst); + if n == 0 { + ResponseTemplate::new(500) + } else { + ResponseTemplate::new(200).set_body_json(Foo { bar: 9 }) + } + } + } + let server = MockServer::start().await; + let calls = Arc::new(AtomicU32::new(0)); + Mock::given(method("GET")) + .and(path("/flaky")) + .respond_with(FlakyRespond { + calls: Arc::clone(&calls), + }) + .expect(2) + .mount(&server) + .await; + + let client = fast_client(&server, ApiFlavor::Mergify); + let got: Option = client.get_if_exists("/flaky").await.unwrap(); + assert_eq!(got, Some(Foo { bar: 9 })); + assert_eq!(calls.load(Ordering::SeqCst), 2, "expected two attempts"); + } } diff --git a/crates/mergify-queue/src/lib.rs b/crates/mergify-queue/src/lib.rs index 96455cb4..fa9f8b56 100644 --- a/crates/mergify-queue/src/lib.rs +++ b/crates/mergify-queue/src/lib.rs @@ -10,5 +10,6 @@ //! ports next. pub mod pause; +pub mod show; pub mod status; pub mod unpause; diff --git a/crates/mergify-queue/src/show.rs b/crates/mergify-queue/src/show.rs new file mode 100644 index 00000000..81c6a618 --- /dev/null +++ b/crates/mergify-queue/src/show.rs @@ -0,0 +1,838 @@ +//! `mergify queue show` — detailed state of a single PR in the +//! merge queue. +//! +//! `GET /v1/repos//merge-queue/pull/`. Two output +//! modes: +//! +//! - `--json`: pretty-prints the raw API response as a single JSON +//! document. The schema is Mergify's API contract, not this CLI's, +//! so unknown fields are preserved. +//! - Human (default): metadata block (position / priority / queue +//! rule / queued / ETA), then a CI-state line and a checks +//! section, then a conditions section. `--verbose` switches the +//! checks summary to a full table and the conditions summary to +//! a tree. +//! +//! 404 responses are special-cased: the API returns 404 for "PR is +//! not currently in the merge queue", which is a routine caller +//! branch rather than a server failure. The command returns a +//! [`CliError::MergifyApi`] whose message is `PR #N is not in the +//! merge queue`; the binary's top-level handler prints it to +//! stderr as `mergify: PR #N is not in the merge queue` (the +//! `mergify: ` prefix is the binary's standard error envelope) +//! and exits with the Mergify-API error code. Live smoke tests +//! assert against the substring, which is stable across the +//! Python and Rust implementations. + +use std::io::Write; + +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::Output; +use mergify_core::auth; +use mergify_tui::Theme; +use mergify_tui::relative_time; +use mergify_tui::tree; +use serde::Deserialize; + +pub struct ShowOptions<'a> { + pub repository: Option<&'a str>, + pub token: Option<&'a str>, + pub api_url: Option<&'a str>, + pub pr_number: u64, + pub verbose: bool, + pub output_json: bool, +} + +#[derive(Deserialize)] +struct PullView { + number: u64, + #[serde(default)] + queued_at: Option, + #[serde(default)] + estimated_time_of_merge: Option, + #[serde(default)] + position: Option, + #[serde(default)] + priority_rule_name: Option, + #[serde(default)] + queue_rule_name: Option, + #[serde(default)] + mergeability_check: Option, +} + +#[derive(Deserialize)] +struct MergeabilityCheck { + #[serde(default)] + check_type: Option, + #[serde(default)] + started_at: Option, + ci_state: String, + #[serde(default)] + checks: Vec, + #[serde(default)] + conditions_evaluation: Option, +} + +#[derive(Deserialize)] +struct Check { + name: String, + state: String, +} + +#[derive(Deserialize)] +struct ConditionEvaluation { + #[serde(default)] + label: String, + #[serde(default = "default_match_true")] + r#match: bool, + #[serde(default)] + subconditions: Vec, +} + +// The top-level `conditions_evaluation` payload may legitimately +// omit `match` (it's the aggregator node, not a leaf). Treat a +// missing flag as "matched" so we don't render a spurious failure +// for the root. +const fn default_match_true() -> bool { + true +} + +/// 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 client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let path = format!( + "/v1/repos/{repository}/merge-queue/pull/{pr_number}", + pr_number = opts.pr_number, + ); + + output.status(&format!( + "Fetching merge queue state for PR #{n}…", + n = opts.pr_number, + ))?; + + let raw: Option = client.get_if_exists(&path).await?; + let Some(raw) = raw else { + return Err(CliError::MergifyApi(format!( + "PR #{n} is not in the merge queue", + n = opts.pr_number, + ))); + }; + + if opts.output_json { + emit_json(output, &raw)?; + return Ok(()); + } + + let view: PullView = serde_json::from_value(raw) + .map_err(|e| CliError::Generic(format!("decode merge queue pull response: {e}")))?; + emit_human(output, &view, opts.verbose)?; + Ok(()) +} + +fn emit_json(output: &mut dyn Output, value: &serde_json::Value) -> std::io::Result<()> { + output.emit(value, &mut |w: &mut dyn Write| { + let rendered = serde_json::to_string_pretty(value) + .map_err(|e| std::io::Error::other(e.to_string()))?; + writeln!(w, "{rendered}") + }) +} + +fn emit_human(output: &mut dyn Output, view: &PullView, verbose: bool) -> std::io::Result<()> { + let now = Utc::now(); + let theme = Theme::detect(); + output.emit(&(), &mut |w: &mut dyn Write| { + print_metadata(w, &theme, view, now)?; + + match &view.mergeability_check { + None => { + writeln!(w)?; + writeln!( + w, + " {D}Waiting for mergeability check...{R}", + D = theme.dim, + R = theme.reset, + )?; + } + Some(mc) => { + print_checks_section(w, &theme, mc, verbose, now)?; + if let Some(conditions) = &mc.conditions_evaluation { + print_conditions_section(w, &theme, conditions, verbose)?; + } + } + } + Ok(()) + }) +} + +fn print_metadata( + w: &mut dyn Write, + theme: &Theme, + view: &PullView, + now: DateTime, +) -> std::io::Result<()> { + writeln!( + w, + "{B}PR #{n}{R}", + B = theme.bold, + n = view.number, + R = theme.reset, + )?; + writeln!(w)?; + writeln!( + w, + " Position: {}", + display_or_dash(view.position.map(|n| n.to_string()).as_deref()), + )?; + writeln!( + w, + " Priority: {}", + display_or_dash(view.priority_rule_name.as_deref()), + )?; + writeln!( + w, + " Queue rule: {}", + display_or_dash(view.queue_rule_name.as_deref()), + )?; + writeln!( + w, + " Queued at: {}", + relative_or_raw_or_dash(view.queued_at.as_deref(), now, false), + )?; + writeln!( + w, + " ETA: {}", + relative_or_raw_or_dash(view.estimated_time_of_merge.as_deref(), now, true), + ) +} + +fn display_or_dash(value: Option<&str>) -> &str { + value.filter(|s| !s.is_empty()).unwrap_or("-") +} + +fn relative_or_raw_or_dash(value: Option<&str>, now: DateTime, future: bool) -> String { + let Some(raw) = value else { + return "-".to_string(); + }; + let rel = relative_time(raw, now, future); + if rel.is_empty() { + // Unparseable timestamp — show the raw string so the user + // sees *something* rather than a silent dash. + raw.to_string() + } else { + rel + } +} + +fn print_checks_section( + w: &mut dyn Write, + theme: &Theme, + mc: &MergeabilityCheck, + verbose: bool, + now: DateTime, +) -> std::io::Result<()> { + writeln!(w)?; + let (icon, style) = check_state_glyph(theme, &mc.ci_state); + write!( + w, + " CI State: {S}{icon} {state}{R}", + S = style, + state = mc.ci_state, + R = theme.reset, + )?; + if let Some(check_type) = mc.check_type.as_deref().filter(|s| !s.is_empty()) { + write!(w, " {D}{check_type}{R}", D = theme.dim, R = theme.reset)?; + } + if let Some(started) = &mc.started_at { + let rel = relative_time(started, now, false); + if !rel.is_empty() { + write!(w, " {D}started {rel}{R}", D = theme.dim, R = theme.reset)?; + } + } + writeln!(w)?; + + if mc.checks.is_empty() { + return Ok(()); + } + + if verbose { + print_checks_table(w, theme, &mc.checks) + } else { + print_checks_summary(w, theme, &mc.checks) + } +} + +fn print_checks_table(w: &mut dyn Write, theme: &Theme, checks: &[Check]) -> std::io::Result<()> { + let name_width = checks + .iter() + .map(|c| c.name.chars().count()) + .max() + .unwrap_or(0); + for check in checks { + let (icon, style) = check_state_glyph(theme, &check.state); + let pad = name_width.saturating_sub(check.name.chars().count()); + writeln!( + w, + " {D}{name}{spaces}{R} {S}{icon} {state}{R}", + D = theme.dim, + name = check.name, + spaces = " ".repeat(pad), + R = theme.reset, + S = style, + state = check.state, + )?; + } + Ok(()) +} + +fn print_checks_summary(w: &mut dyn Write, theme: &Theme, checks: &[Check]) -> std::io::Result<()> { + let mut passed: u32 = 0; + let mut pending: u32 = 0; + let mut failed: u32 = 0; + for check in checks { + match check.state.as_str() { + "success" | "neutral" | "skipped" => passed += 1, + "pending" => pending += 1, + _ => failed += 1, + } + } + + write!(w, " Checks: ")?; + write!( + w, + "{S}{passed} passed{R}", + S = enabled_fg(theme, AnsiColor::Green), + R = theme.reset, + )?; + if pending > 0 { + write!( + w, + ", {S}{pending} pending{R}", + S = enabled_fg(theme, AnsiColor::Blue), + R = theme.reset, + )?; + } + if failed > 0 { + write!( + w, + ", {S}{failed} failed{R}", + S = enabled_fg(theme, AnsiColor::Red), + R = theme.reset, + )?; + } + writeln!(w)?; + + for check in checks { + if matches!( + check.state.as_str(), + "failure" | "error" | "timed_out" | "action_required" + ) { + let (icon, style) = check_state_glyph(theme, &check.state); + writeln!( + w, + " {S}{icon} {state}{R} {D}{name}{R}", + S = style, + state = check.state, + R = theme.reset, + D = theme.dim, + name = check.name, + )?; + } + } + Ok(()) +} + +/// Map a check state string to (icon, ANSI style). Mirrors Python's +/// `CHECK_STATE_STYLES`; unknown states fall back to a dim `?` so +/// the renderer never crashes on a new API code. +fn check_state_glyph(theme: &Theme, state: &str) -> (&'static str, Style) { + match state { + "success" => ("✓", enabled_fg(theme, AnsiColor::Green)), + "pending" => ("◌", enabled_fg(theme, AnsiColor::Yellow)), + "failure" | "error" | "action_required" => ("✗", enabled_fg(theme, AnsiColor::Red)), + "timed_out" => ("⏰", enabled_fg(theme, AnsiColor::Red)), + "cancelled" | "neutral" | "skipped" | "stale" => ("○", theme.dim), + _ => ("?", theme.dim), + } +} + +fn enabled_fg(theme: &Theme, color: AnsiColor) -> Style { + if theme.enabled { + theme.fg(color) + } else { + Style::new() + } +} + +fn print_conditions_section( + w: &mut dyn Write, + theme: &Theme, + evaluation: &ConditionEvaluation, + verbose: bool, +) -> std::io::Result<()> { + writeln!(w)?; + if verbose { + writeln!(w, "{B}Conditions{R}", B = theme.bold, R = theme.reset)?; + write_condition_tree(w, theme, &evaluation.subconditions, "")?; + return Ok(()); + } + + let top = &evaluation.subconditions; + if top.is_empty() { + return Ok(()); + } + + let met = top.iter().filter(|s| s.r#match).count(); + let total = top.len(); + let style = if met == total { + enabled_fg(theme, AnsiColor::Green) + } else { + enabled_fg(theme, AnsiColor::Yellow) + }; + writeln!( + w, + " Conditions: {S}{met}/{total} met{R}", + S = style, + R = theme.reset, + )?; + + for sub in top { + if sub.r#match { + continue; + } + let summary = if sub.subconditions.is_empty() { + sub.label.clone() + } else { + summarize_failing_group(sub) + }; + writeln!( + w, + " {S}✗{R} {summary}", + S = enabled_fg(theme, AnsiColor::Red), + R = theme.reset, + )?; + } + Ok(()) +} + +fn summarize_failing_group(evaluation: &ConditionEvaluation) -> String { + let labels: Vec = evaluation.subconditions.iter().map(child_label).collect(); + if labels.len() <= 3 { + labels.join(" or ") + } else { + let head: Vec<&str> = labels.iter().take(2).map(String::as_str).collect(); + format!("{} or ({} more)", head.join(" or "), labels.len() - 2) + } +} + +fn child_label(evaluation: &ConditionEvaluation) -> String { + let label = &evaluation.label; + if !is_aggregator(label) { + return label.clone(); + } + let Some(first) = evaluation.subconditions.first() else { + return label.clone(); + }; + if is_aggregator(&first.label) { + child_label(first) + } else { + first.label.clone() + } +} + +fn is_aggregator(label: &str) -> bool { + matches!(label, "all of" | "any of" | "not") +} + +fn write_condition_tree( + w: &mut dyn Write, + theme: &Theme, + nodes: &[ConditionEvaluation], + prefix: &str, +) -> std::io::Result<()> { + if nodes.is_empty() { + return Ok(()); + } + let last = nodes.len() - 1; + for (i, node) in nodes.iter().enumerate() { + let (branch, continuation) = tree::branch_chars(i == last); + let (icon, style) = if node.r#match { + ("✓", enabled_fg(theme, AnsiColor::Green)) + } else { + ("✗", enabled_fg(theme, AnsiColor::Red)) + }; + writeln!( + w, + "{prefix}{branch}{S}{icon}{R} {label}", + S = style, + R = theme.reset, + label = node.label, + )?; + let child_prefix = format!("{prefix}{continuation}"); + write_condition_tree(w, theme, &node.subconditions, &child_prefix)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use mergify_core::OutputMode; + use mergify_core::StdioOutput; + use serde_json::json; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::header; + use wiremock::matchers::method; + use wiremock::matchers::path; + + 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, + "queued_at": "2026-05-09T10:00:00Z", + "estimated_time_of_merge": "2026-05-09T11:00:00Z", + "position": 3, + "priority_rule_name": "default", + "queue_rule_name": "default", + "queue_rule": {"name": "default", "config": {}}, + "mergeability_check": { + "check_type": "in_place", + "queue_pull_request_number": 123, + "started_at": "2026-05-09T10:05:00Z", + "ci_state": "pending", + "state": "running", + "checks": [ + {"name": "tests", "description": "", "state": "success"}, + {"name": "linters", "description": "", "state": "pending"}, + {"name": "security", "description": "", "state": "failure"}, + ], + "conditions_evaluation": { + "match": false, + "label": "all of", + "subconditions": [ + { + "match": true, + "label": "#check-success=tests", + "subconditions": [], + }, + { + "match": false, + "label": "#check-success=linters", + "subconditions": [], + }, + ], + }, + }, + }) + } + + async fn arrange(server: &MockServer, body: serde_json::Value, status: u16) { + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/merge-queue/pull/123")) + .and(header("Authorization", "Bearer t")) + .respond_with(ResponseTemplate::new(status).set_body_json(body)) + .expect(1) + .mount(server) + .await; + } + + #[tokio::test] + async fn run_renders_metadata_and_compact_sections() { + let server = MockServer::start().await; + arrange(&server, pull_response(), 200).await; + + let mut cap = make_output(OutputMode::Human); + let api_url = server.uri(); + run( + ShowOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + pr_number: 123, + verbose: false, + output_json: false, + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + assert!(stdout.contains("PR #123"), "got: {stdout:?}"); + assert!(stdout.contains("Position:"), "got: {stdout:?}"); + assert!(stdout.contains("CI State:"), "got: {stdout:?}"); + // Compact summary: 1 passed (tests), 1 pending (linters), 1 + // failed (security). The failing check name is listed below + // the summary line. + assert!(stdout.contains("1 passed"), "got: {stdout:?}"); + assert!(stdout.contains("1 pending"), "got: {stdout:?}"); + assert!(stdout.contains("1 failed"), "got: {stdout:?}"); + assert!(stdout.contains("security"), "got: {stdout:?}"); + // Compact conditions: "1/2 met" + the failing label. + assert!(stdout.contains("1/2 met"), "got: {stdout:?}"); + assert!(stdout.contains("#check-success=linters"), "got: {stdout:?}"); + } + + #[tokio::test] + async fn run_renders_verbose_table_and_tree() { + let server = MockServer::start().await; + arrange(&server, pull_response(), 200).await; + + let mut cap = make_output(OutputMode::Human); + let api_url = server.uri(); + run( + ShowOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + pr_number: 123, + verbose: true, + output_json: false, + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + // Verbose table: every check name appears as its own row. + assert!(stdout.contains("tests"), "got: {stdout:?}"); + assert!(stdout.contains("linters"), "got: {stdout:?}"); + assert!(stdout.contains("security"), "got: {stdout:?}"); + // Verbose conditions: tree header + box-drawing characters. + assert!(stdout.contains("Conditions"), "got: {stdout:?}"); + assert!( + stdout.contains("├──") || stdout.contains("└──"), + "got: {stdout:?}" + ); + } + + #[tokio::test] + async fn run_emits_json_passthrough() { + let server = MockServer::start().await; + // Add a synthetic field to verify unknown fields survive + // the round-trip. + let mut body = pull_response(); + body["future_field"] = json!("preserved"); + arrange(&server, body, 200).await; + + let mut cap = make_output(OutputMode::Json); + let api_url = server.uri(); + run( + ShowOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + pr_number: 123, + verbose: false, + output_json: true, + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(parsed["number"], json!(123)); + assert_eq!(parsed["future_field"], json!("preserved")); + } + + #[tokio::test] + async fn run_404_is_not_in_queue() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/merge-queue/pull/999")) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + + let mut cap = make_output(OutputMode::Human); + let api_url = server.uri(); + let err = run( + ShowOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + pr_number: 999, + verbose: false, + output_json: false, + }, + &mut cap.output, + ) + .await + .unwrap_err(); + + assert!( + matches!(err, CliError::MergifyApi(_)), + "expected MergifyApi, got {err:?}", + ); + assert_eq!(err.exit_code(), mergify_core::ExitCode::MergifyApiError); + assert!( + err.to_string().contains("not in the merge queue"), + "got: {err}" + ); + } + + #[tokio::test] + async fn run_no_mergeability_check() { + let server = MockServer::start().await; + let body = json!({ + "number": 123, + "queued_at": "2026-05-09T10:00:00Z", + "position": 1, + "priority_rule_name": "default", + "queue_rule_name": "default", + "queue_rule": {"name": "default", "config": {}}, + "mergeability_check": null, + }); + arrange(&server, body, 200).await; + + let mut cap = make_output(OutputMode::Human); + let api_url = server.uri(); + run( + ShowOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + pr_number: 123, + verbose: false, + output_json: false, + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + assert!( + stdout.contains("Waiting for mergeability check"), + "got: {stdout:?}", + ); + } + + #[test] + fn summarize_failing_group_two_labels() { + let group = ConditionEvaluation { + label: "any of".to_string(), + r#match: false, + subconditions: vec![ + ConditionEvaluation { + label: "a".to_string(), + r#match: false, + subconditions: vec![], + }, + ConditionEvaluation { + label: "b".to_string(), + r#match: false, + subconditions: vec![], + }, + ], + }; + assert_eq!(summarize_failing_group(&group), "a or b"); + } + + #[test] + fn summarize_failing_group_truncates_at_three_plus() { + let group = ConditionEvaluation { + label: "any of".to_string(), + r#match: false, + subconditions: vec![ + ConditionEvaluation { + label: "a".to_string(), + r#match: false, + subconditions: vec![], + }, + ConditionEvaluation { + label: "b".to_string(), + r#match: false, + subconditions: vec![], + }, + ConditionEvaluation { + label: "c".to_string(), + r#match: false, + subconditions: vec![], + }, + ConditionEvaluation { + label: "d".to_string(), + r#match: false, + subconditions: vec![], + }, + ], + }; + // 4 items: keep first 2, summarize the rest. + assert_eq!(summarize_failing_group(&group), "a or b or (2 more)"); + } + + #[test] + fn child_label_recurses_through_aggregators() { + let nested = ConditionEvaluation { + label: "any of".to_string(), + r#match: false, + subconditions: vec![ConditionEvaluation { + label: "all of".to_string(), + r#match: false, + subconditions: vec![ConditionEvaluation { + label: "leaf".to_string(), + r#match: false, + subconditions: vec![], + }], + }], + }; + 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/mergify_cli/cli.py b/mergify_cli/cli.py index 1b9aacea..9a4e07c6 100644 --- a/mergify_cli/cli.py +++ b/mergify_cli/cli.py @@ -30,7 +30,6 @@ from mergify_cli.dym import DYMGroup from mergify_cli.exit_codes import ExitCode from mergify_cli.freeze import cli as freeze_cli_mod -from mergify_cli.queue import cli as queue_cli_mod from mergify_cli.stack import cli as stack_cli_mod @@ -54,7 +53,6 @@ def cli( cli.add_command(stack_cli_mod.stack) cli.add_command(ci_cli_mod.ci) cli.add_command(freeze_cli_mod.freeze) -cli.add_command(queue_cli_mod.queue) def main() -> None: diff --git a/mergify_cli/queue/__init__.py b/mergify_cli/queue/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/mergify_cli/queue/api.py b/mergify_cli/queue/api.py deleted file mode 100644 index 1b6b977f..00000000 --- a/mergify_cli/queue/api.py +++ /dev/null @@ -1,62 +0,0 @@ -from __future__ import annotations - -import typing - - -if typing.TYPE_CHECKING: - import httpx - - -class QueueRule(typing.TypedDict): - name: str - config: dict[str, typing.Any] - - -class QueueCheck(typing.TypedDict, total=False): - name: typing.Required[str] - description: typing.Required[str] - url: str | None - state: typing.Required[str] - avatar_url: str | None - - -class QueueConditionEvaluation(typing.TypedDict, total=False): - match: typing.Required[bool] - label: typing.Required[str] - description: str | None - subconditions: list[QueueConditionEvaluation] - evaluations: list[dict[str, typing.Any]] - - -class QueueMergeabilityCheck(typing.TypedDict, total=False): - check_type: typing.Required[str] - queue_pull_request_number: typing.Required[int] - started_at: str | None - ci_ended_at: str | None - ci_state: typing.Required[str] - state: typing.Required[str] - checks: typing.Required[list[QueueCheck]] - conditions_evaluation: QueueConditionEvaluation | None - - -class QueuePullResponse(typing.TypedDict, total=False): - number: typing.Required[int] - queued_at: typing.Required[str] - estimated_time_of_merge: str | None - position: typing.Required[int] - priority_rule_name: typing.Required[str] - queue_rule_name: typing.Required[str] - checks_timeout_at: str | None - queue_rule: typing.Required[QueueRule] - mergeability_check: QueueMergeabilityCheck | None - - -async def get_queue_pull( - client: httpx.AsyncClient, - repository: str, - pr_number: int, -) -> QueuePullResponse: - response = await client.get( - f"/v1/repos/{repository}/merge-queue/pull/{pr_number}", - ) - return response.json() # type: ignore[no-any-return] diff --git a/mergify_cli/queue/cli.py b/mergify_cli/queue/cli.py deleted file mode 100644 index f52eb6d6..00000000 --- a/mergify_cli/queue/cli.py +++ /dev/null @@ -1,313 +0,0 @@ -from __future__ import annotations - -import asyncio -import datetime - -import click -from rich.text import Text -from rich.tree import Tree - -from mergify_cli import console -from mergify_cli import utils -from mergify_cli.dym import DYMGroup -from mergify_cli.exit_codes import ExitCode -from mergify_cli.queue import api as queue_api - - -CHECK_STATE_STYLES: dict[str, tuple[str, str]] = { - "success": ("✓", "green"), - "pending": ("◌", "yellow"), - "failure": ("✗", "red"), - "error": ("✗", "red"), - "cancelled": ("○", "dim"), - "action_required": ("!", "red"), - "timed_out": ("⏰", "red"), - "neutral": ("○", "dim"), - "skipped": ("○", "dim"), - "stale": ("○", "dim"), -} - - -def _relative_time(iso_str: str | None, *, future: bool = False) -> str: - if not iso_str: - return "" - try: - dt = datetime.datetime.fromisoformat(iso_str) - except ValueError: - return iso_str - now = datetime.datetime.now(tz=datetime.UTC) - if dt.tzinfo is None: - dt = dt.replace(tzinfo=datetime.UTC) - delta = abs(now - dt) - total_seconds = int(delta.total_seconds()) - if total_seconds < 60: - value = f"{total_seconds}s" - elif total_seconds < 3600: - value = f"{total_seconds // 60}m" - elif total_seconds < 86400: - value = f"{total_seconds // 3600}h" - else: - value = f"{total_seconds // 86400}d" - if future: - return f"~{value}" - return f"{value} ago" - - -def _print_pull_metadata(data: queue_api.QueuePullResponse) -> None: - console.print(Text(f"PR #{data['number']}", style="bold")) - console.print() - console.print(f" Position: {data['position']}") - console.print(f" Priority: {data['priority_rule_name']}") - console.print(f" Queue rule: {data['queue_rule_name']}") - queued_rel = _relative_time(data["queued_at"]) - console.print( - f" Queued at: {queued_rel}" - if queued_rel - else f" Queued at: {data['queued_at']}", - ) - eta = data.get("estimated_time_of_merge") - eta_rel = _relative_time(eta, future=True) if eta else "" - console.print(f" ETA: {eta_rel}" if eta_rel else " ETA: -") - - -def _check_state_text(state: str) -> Text: - icon, style = CHECK_STATE_STYLES.get(state, ("?", "dim")) - text = Text() - text.append(f"{icon} ", style=style) - text.append(state, style=style) - return text - - -def _print_checks_section( - mc: queue_api.QueueMergeabilityCheck, - *, - verbose: bool = False, -) -> None: - from rich.table import Table - - console.print() - ci_label = Text(" CI State: ") - ci_label.append_text(_check_state_text(mc["ci_state"])) - ci_label.append(f" {mc['check_type']}", style="dim") - started = mc.get("started_at") - if started: - rel = _relative_time(started) - if rel: - ci_label.append(f" started {rel}", style="dim") - console.print(ci_label) - - checks = mc["checks"] - if not checks: - return - - if verbose: - table = Table(show_header=True, padding=(0, 1), box=None) - table.add_column(" Check", style="dim") - table.add_column("Status") - for check in checks: - table.add_row(f" {check['name']}", _check_state_text(check["state"])) - console.print(table) - else: - passed = sum( - 1 for c in checks if c["state"] in {"success", "neutral", "skipped"} - ) - pending = sum(1 for c in checks if c["state"] == "pending") - failed = len(checks) - passed - pending - summary = Text(" Checks: ") - summary.append(f"{passed} passed", style="green") - if pending: - summary.append(f", {pending} pending", style="blue") - if failed: - summary.append(f", {failed} failed", style="red") - console.print(summary) - failing = [c for c in checks if c["state"] in {"failure", "error", "timed_out"}] - for check in failing: - line = Text(" ") - line.append_text(_check_state_text(check["state"])) - line.append(f" {check['name']}", style="dim") - console.print(line) - - -def _child_label(evaluation: queue_api.QueueConditionEvaluation) -> str: - label = evaluation["label"] - if label not in {"all of", "any of", "not"}: - return label - subconditions = evaluation.get("subconditions") or [] - if not subconditions: - return label - first = subconditions[0]["label"] - if first in {"all of", "any of", "not"}: - return _child_label(subconditions[0]) - return first - - -def _summarize_failing_group( - evaluation: queue_api.QueueConditionEvaluation, -) -> str: - subconditions = evaluation.get("subconditions") or [] - labels = [_child_label(sub) for sub in subconditions] - if len(labels) <= 3: - return " or ".join(labels) - return " or ".join([*labels[:2], f"({len(labels) - 2} more)"]) - - -def _print_conditions_section( - evaluation: queue_api.QueueConditionEvaluation, - *, - verbose: bool = False, -) -> None: - console.print() - if verbose: - tree = Tree(Text("Conditions", style="bold")) - _add_condition_nodes(tree, evaluation) - console.print(tree) - return - - top_level = evaluation.get("subconditions") or [] - if not top_level: - return - - met = sum(1 for s in top_level if s["match"]) - total = len(top_level) - header = Text(" Conditions: ") - if met == total: - header.append(f"{met}/{total} met", style="green") - else: - header.append(f"{met}/{total} met", style="yellow") - console.print(header) - - for sub in top_level: - if sub["match"]: - continue - child_subs = sub.get("subconditions") or [] - summary = _summarize_failing_group(sub) if child_subs else sub["label"] - line = Text(" ") - line.append("✗ ", style="red") - line.append(summary) - console.print(line) - - -def _add_condition_nodes( - parent: Tree, - evaluation: queue_api.QueueConditionEvaluation, -) -> None: - subconditions = evaluation.get("subconditions") or [] - if subconditions: - for sub in subconditions: - icon = "✓" if sub["match"] else "✗" - style = "green" if sub["match"] else "red" - label = Text() - label.append(f"{icon} ", style=style) - label.append(sub["label"]) - node = parent.add(label) - _add_condition_nodes(node, sub) - - -@click.group( - cls=DYMGroup, - invoke_without_command=True, - help="Manage merge queue", -) -@click.option( - "--token", - "-t", - help="Mergify or GitHub token", - envvar=["MERGIFY_TOKEN", "GITHUB_TOKEN"], - required=True, - default=lambda: asyncio.run(utils.get_default_token()), -) -@click.option( - "--api-url", - "-u", - help="URL of the Mergify API", - envvar="MERGIFY_API_URL", - default=utils.MERGIFY_API_DEFAULT_URL, - show_default=True, -) -@click.option( - "--repository", - "-r", - help="Repository full name (owner/repo)", - required=True, - default=lambda: asyncio.run(utils.get_default_repository()), -) -@click.pass_context -def queue( - ctx: click.Context, - *, - token: str, - api_url: str, - repository: str, -) -> None: - ctx.ensure_object(dict) - ctx.obj["token"] = token - ctx.obj["api_url"] = api_url - ctx.obj["repository"] = repository - if ctx.invoked_subcommand is None: - click.echo(ctx.get_help()) - - -@queue.command(help="Show detailed state of a pull request in the merge queue") -@click.argument("pr_number", type=int) -@click.option( - "--verbose", - "-v", - is_flag=True, - help="Show full checks table and conditions tree", -) -@click.option( - "--json", - "output_json", - is_flag=True, - help="Output in JSON format", -) -@click.pass_context -@utils.run_with_asyncio -async def show( - ctx: click.Context, - *, - pr_number: int, - verbose: bool, - output_json: bool, -) -> None: - import httpx - - try: - async with utils.get_mergify_http_client( - ctx.obj["api_url"], - ctx.obj["token"], - ) as client: - data = await queue_api.get_queue_pull( - client, - ctx.obj["repository"], - pr_number, - ) - except httpx.HTTPStatusError as e: - if e.response.status_code == 404: - console.print( - f"PR #{pr_number} is not in the merge queue", - style="yellow", - ) - raise SystemExit(ExitCode.MERGIFY_API_ERROR) from None - raise - - if output_json: - import json - - # JSON output is a passthrough of the Mergify API response. - # The schema is Mergify's API contract, not this CLI's — the - # Rust port must preserve this passthrough behavior. - click.echo(json.dumps(data, indent=2)) - return - - _print_pull_metadata(data) - - mc = data.get("mergeability_check") - if mc is None: - console.print() - console.print(" Waiting for mergeability check...", style="dim") - else: - _print_checks_section(mc, verbose=verbose) - conditions = mc.get("conditions_evaluation") - if conditions is not None: - _print_conditions_section(conditions, verbose=verbose) diff --git a/mergify_cli/tests/queue/test_cli.py b/mergify_cli/tests/queue/test_cli.py deleted file mode 100644 index d8451f93..00000000 --- a/mergify_cli/tests/queue/test_cli.py +++ /dev/null @@ -1,54 +0,0 @@ -from __future__ import annotations - -import datetime -from unittest.mock import patch - -from mergify_cli.queue.cli import _relative_time - - -class TestRelativeTime: - def test_seconds(self) -> None: - now = datetime.datetime(2025, 1, 1, 12, 0, 30, tzinfo=datetime.UTC) - with patch("mergify_cli.queue.cli.datetime") as mock_dt: - mock_dt.datetime.now.return_value = now - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - assert _relative_time("2025-01-01T12:00:00Z") == "30s ago" - - def test_minutes(self) -> None: - now = datetime.datetime(2025, 1, 1, 12, 5, 0, tzinfo=datetime.UTC) - with patch("mergify_cli.queue.cli.datetime") as mock_dt: - mock_dt.datetime.now.return_value = now - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - assert _relative_time("2025-01-01T12:00:00Z") == "5m ago" - - def test_hours(self) -> None: - now = datetime.datetime(2025, 1, 1, 14, 0, 0, tzinfo=datetime.UTC) - with patch("mergify_cli.queue.cli.datetime") as mock_dt: - mock_dt.datetime.now.return_value = now - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - assert _relative_time("2025-01-01T12:00:00Z") == "2h ago" - - def test_days(self) -> None: - now = datetime.datetime(2025, 1, 4, 12, 0, 0, tzinfo=datetime.UTC) - with patch("mergify_cli.queue.cli.datetime") as mock_dt: - mock_dt.datetime.now.return_value = now - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - assert _relative_time("2025-01-01T12:00:00Z") == "3d ago" - - def test_future(self) -> None: - now = datetime.datetime(2025, 1, 1, 12, 0, 0, tzinfo=datetime.UTC) - with patch("mergify_cli.queue.cli.datetime") as mock_dt: - mock_dt.datetime.now.return_value = now - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - assert _relative_time("2025-01-01T12:30:00Z", future=True) == "~30m" - - def test_none(self) -> None: - assert not _relative_time(None) - - def test_empty(self) -> None: - assert not _relative_time("") diff --git a/mergify_cli/tests/queue/test_show.py b/mergify_cli/tests/queue/test_show.py deleted file mode 100644 index dd3e646c..00000000 --- a/mergify_cli/tests/queue/test_show.py +++ /dev/null @@ -1,363 +0,0 @@ -from __future__ import annotations - -import datetime -import typing -from unittest.mock import patch - -from click.testing import CliRunner -from httpx import Response -import respx - -from mergify_cli.exit_codes import ExitCode -from mergify_cli.queue.cli import queue -from mergify_cli.tests import utils as test_utils - - -FAKE_CHECK_SUCCESS = { - "name": "tests", - "description": "Running tests", - "url": "https://github.com/owner/repo/actions/runs/123", - "state": "success", - "avatar_url": None, -} - -FAKE_CHECK_PENDING = { - "name": "linters", - "description": "Running linters", - "url": None, - "state": "pending", - "avatar_url": None, -} - -FAKE_CHECK_FAILED = { - "name": "security-scan", - "description": "Security scan", - "url": None, - "state": "failure", - "avatar_url": None, -} - -FAKE_CONDITION_MATCH = { - "match": True, - "label": "#check-success=tests", - "description": None, - "subconditions": [], - "evaluations": [], -} - -FAKE_CONDITION_NO_MATCH = { - "match": False, - "label": "#check-success=linters", - "description": None, - "subconditions": [], - "evaluations": [], -} - -FAKE_CONDITIONS_EVALUATION = { - "match": False, - "label": "all of", - "description": None, - "subconditions": [FAKE_CONDITION_MATCH, FAKE_CONDITION_NO_MATCH], - "evaluations": [], -} - -FAKE_MERGEABILITY_CHECK = { - "check_type": "in_place", - "queue_pull_request_number": 123, - "started_at": "2025-11-05T10:05:00Z", - "ci_ended_at": None, - "ci_state": "pending", - "state": "running", - "checks": [FAKE_CHECK_SUCCESS, FAKE_CHECK_PENDING], - "conditions_evaluation": FAKE_CONDITIONS_EVALUATION, -} - -FAKE_PULL_RESPONSE = { - "number": 123, - "queued_at": "2025-11-05T10:00:00Z", - "estimated_time_of_merge": "2025-11-05T11:00:00Z", - "position": 3, - "priority_rule_name": "default", - "queue_rule_name": "default", - "checks_timeout_at": "2025-11-05T12:00:00Z", - "queue_rule": {"name": "default", "config": {}}, - "mergeability_check": FAKE_MERGEABILITY_CHECK, -} - -BASE_ARGS = [ - "--token", - "test-token", - "--api-url", - "https://api.mergify.com", - "--repository", - "owner/repo", -] - - -def _invoke_show( - mock: respx.MockRouter, - pr_number: int, - response_json: dict[str, typing.Any], - *, - status_code: int = 200, - extra_args: list[str] | None = None, -) -> typing.Any: - mock.get(f"/v1/repos/owner/repo/merge-queue/pull/{pr_number}").mock( - return_value=Response(status_code, json=response_json), - ) - runner = CliRunner() - args = [*BASE_ARGS, "show", str(pr_number), *(extra_args or [])] - return runner.invoke(queue, args) - - -class TestShowCommand: - def test_compact_output(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE) - assert result.exit_code == 0, result.output - assert "PR #123" in result.output - assert "Position: 3" in result.output - assert "Priority: default" in result.output - assert "Queue rule: default" in result.output - assert "pending" in result.output - assert "passed" in result.output - assert "met" in result.output - - def test_verbose_output(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE, extra_args=["-v"]) - assert result.exit_code == 0, result.output - assert "PR #123" in result.output - assert "tests" in result.output - assert "linters" in result.output - assert "Conditions" in result.output - - def test_metadata_fields(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE) - assert result.exit_code == 0, result.output - assert "Priority" in result.output - assert "Queue rule" in result.output - assert "Queued at" in result.output - assert "ETA" in result.output - - def test_compact_checks_summary(self) -> None: - response = { - **FAKE_PULL_RESPONSE, - "mergeability_check": { - **FAKE_MERGEABILITY_CHECK, - "checks": [FAKE_CHECK_SUCCESS, FAKE_CHECK_PENDING, FAKE_CHECK_FAILED], - }, - } - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, response) - assert result.exit_code == 0, result.output - assert "1 passed" in result.output - assert "1 pending" in result.output - assert "1 failed" in result.output - assert "security-scan" in result.output - - def test_verbose_checks_table(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE, extra_args=["-v"]) - assert result.exit_code == 0, result.output - assert "tests" in result.output - assert "linters" in result.output - - def test_compact_failing_conditions(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE) - assert result.exit_code == 0, result.output - assert "1/2 met" in result.output - assert "#check-success=linters" in result.output - - def test_verbose_conditions_tree(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, FAKE_PULL_RESPONSE, extra_args=["-v"]) - assert result.exit_code == 0, result.output - assert "#check-success=tests" in result.output - assert "#check-success=linters" in result.output - - def test_no_mergeability_check(self) -> None: - response = {**FAKE_PULL_RESPONSE, "mergeability_check": None} - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, response) - assert result.exit_code == 0, result.output - assert "Waiting for mergeability check" in result.output - - def test_not_in_queue_404(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show( - mock, - 999, - {"message": "Not Found"}, - status_code=404, - ) - assert result.exit_code == ExitCode.MERGIFY_API_ERROR - assert "not in the merge queue" in result.output - - def test_json_output(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show( - mock, - 123, - FAKE_PULL_RESPONSE, - extra_args=["--json"], - ) - assert result.exit_code == 0, result.output - data = test_utils.assert_stdout_is_single_json_document(result.output) - assert data["number"] == 123 - assert data["position"] == 3 - assert data["mergeability_check"]["ci_state"] == "pending" - - def test_no_eta(self) -> None: - response = {**FAKE_PULL_RESPONSE, "estimated_time_of_merge": None} - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, response) - assert result.exit_code == 0, result.output - assert "ETA" in result.output - - def test_nested_conditions_verbose(self) -> None: - nested = { - "match": False, - "label": "all of", - "description": None, - "subconditions": [ - { - "match": True, - "label": "any of", - "description": None, - "subconditions": [ - { - "match": True, - "label": "label=ready", - "description": None, - "subconditions": [], - "evaluations": [], - }, - ], - "evaluations": [], - }, - FAKE_CONDITION_NO_MATCH, - ], - "evaluations": [], - } - response = { - **FAKE_PULL_RESPONSE, - "mergeability_check": { - **FAKE_MERGEABILITY_CHECK, - "conditions_evaluation": nested, - }, - } - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, response, extra_args=["-v"]) - assert result.exit_code == 0, result.output - assert "label=ready" in result.output - assert "#check-success=linters" in result.output - - def test_api_error_non_404(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show( - mock, - 123, - {"message": "Forbidden"}, - status_code=403, - ) - assert result.exit_code != 0 - - def test_no_conditions_evaluation(self) -> None: - response = { - **FAKE_PULL_RESPONSE, - "mergeability_check": { - **FAKE_MERGEABILITY_CHECK, - "conditions_evaluation": None, - }, - } - with respx.mock(base_url="https://api.mergify.com") as mock: - result = _invoke_show(mock, 123, response) - assert result.exit_code == 0, result.output - assert "CI State" in result.output - assert "Conditions" not in result.output - - -FIXED_NOW = datetime.datetime(2025, 11, 5, 10, 10, 0, tzinfo=datetime.UTC) - - -class TestShowOutputSnapshot: - """Full output snapshot tests to visually assess UX from tests.""" - - def _invoke_with_fixed_time( - self, - response_json: dict[str, typing.Any], - extra_args: list[str] | None = None, - ) -> typing.Any: - with ( - patch("mergify_cli.queue.cli.datetime") as mock_dt, - respx.mock(base_url="https://api.mergify.com") as mock, - ): - mock_dt.datetime.now.return_value = FIXED_NOW - mock_dt.datetime.fromisoformat = datetime.datetime.fromisoformat - mock_dt.UTC = datetime.UTC - mock.get("/v1/repos/owner/repo/merge-queue/pull/123").mock( - return_value=Response(200, json=response_json), - ) - runner = CliRunner() - args = [*BASE_ARGS, "show", "123", *(extra_args or [])] - return runner.invoke(queue, args) - - def test_compact_snapshot(self) -> None: - result = self._invoke_with_fixed_time(FAKE_PULL_RESPONSE) - assert result.exit_code == 0, result.output - assert result.output == ( - "PR #123\n" - "\n" - " Position: 3\n" - " Priority: default\n" - " Queue rule: default\n" - " Queued at: 10m ago\n" - " ETA: ~50m\n" - "\n" - " CI State: ◌ pending in_place started 5m ago\n" - " Checks: 1 passed, 1 pending\n" - "\n" - " Conditions: 1/2 met\n" - " ✗ #check-success=linters\n" - ) - - def test_verbose_snapshot(self) -> None: - result = self._invoke_with_fixed_time(FAKE_PULL_RESPONSE, extra_args=["-v"]) - assert result.exit_code == 0, result.output - assert result.output == ( - "PR #123\n" - "\n" - " Position: 3\n" - " Priority: default\n" - " Queue rule: default\n" - " Queued at: 10m ago\n" - " ETA: ~50m\n" - "\n" - " CI State: ◌ pending in_place started 5m ago\n" - " Check Status \n" - " tests ✓ success \n" - " linters ◌ pending \n" - "\n" - "Conditions\n" - "├── ✓ #check-success=tests\n" - "└── ✗ #check-success=linters\n" - ) - - def test_no_mergeability_snapshot(self) -> None: - response = {**FAKE_PULL_RESPONSE, "mergeability_check": None} - result = self._invoke_with_fixed_time(response) - assert result.exit_code == 0, result.output - assert result.output == ( - "PR #123\n" - "\n" - " Position: 3\n" - " Priority: default\n" - " Queue rule: default\n" - " Queued at: 10m ago\n" - " ETA: ~50m\n" - "\n" - " Waiting for mergeability check...\n" - ) diff --git a/mergify_cli/tests/queue/test_skill.py b/mergify_cli/tests/queue/test_skill.py index bbc98c07..f233b0db 100644 --- a/mergify_cli/tests/queue/test_skill.py +++ b/mergify_cli/tests/queue/test_skill.py @@ -102,21 +102,17 @@ def test_skill_has_required_sections() -> None: def test_skill_references_valid_commands() -> None: """Every `mergify queue ` reference in the skill must resolve - to either a registered click command (still-shimmed) or a - Rust-native command reported by the binary. Catches typos and - skill drift after a port — and stays accurate without a parallel - hard-coded list because the native set is queried from - `mergify --list-native-commands` itself. + to a Rust-native command reported by the binary. The whole + `queue` group has been ported, so the binary is the only source + of truth — no parallel click-command list to consult. """ - from mergify_cli.queue.cli import queue - content = _get_skill_content() referenced = set(re.findall(r"mergify queue ([\w-]+)", content)) - available = set(queue.commands.keys()) | _native_commands_for_group("queue") + available = _native_commands_for_group("queue") for cmd in referenced: assert cmd in available, ( - f"Skill references 'mergify queue {cmd}' but it's neither a " - f"registered click command nor a Rust-native command. " + f"Skill references 'mergify queue {cmd}' but it's not a " + f"Rust-native command reported by `mergify --list-native-commands`. " f"Available: {sorted(available)}" )