From 951ac817ab72fde2006f4cd0ac7ea783ef33e02d Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 26 May 2026 18:04:17 +0200 Subject: [PATCH] feat(ci): promote `ci junit-process` from shim to native Rust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third and final layer of the port: the orchestration that wires the JUnit parser, OTLP encoder, uploader, and a new quarantine API client together, plus the native clap dispatch that retires the Python shim for this subcommand. Two new modules under `junit_process`: - `quarantine` POSTs failing test names to `/v1/ci//repositories//quarantines/check` and splits the response into quarantined / non-quarantined buckets. The categorize step counts unknown names as non-quarantined to match Python (an API that silently drops a name still blocks the corresponding failure). - `command::run` orchestrates the per-invocation flow: resolve the token / repo / target branch (falling back to the CI-env detector for the latter two), expand `**`/`*`/`?` patterns into paths, parse every file, hit the quarantine endpoint, feed the quarantined set into the OTLP builder so each case span gets the right `cicd.test.quarantined` value at build time, upload the gzipped protobuf, and render the report. Network failures on quarantine or upload are non-fatal — the report calls them out and the verdict still fires based on `failing_tests_not_quarantined_count`. The dispatch layer in `main.rs` promotes `Subcommands::Ci(CiSubcommand::JunitProcess)` from `ShimmedArgs` to a full clap variant, adds the `("ci", "junit-process")` entry to `NATIVE_COMMANDS`, and routes through `mergify_ci::junit_process::run`. `ci junit-upload` (deprecated) keeps its shim. The detector gains `get_tests_target_branch` (base ref OR head ref) so the orchestrator can derive the branch the quarantine API should look up tests on without round-tripping through Python. Output is byte-for-byte the same as the Python implementation's `process_junit_files` — same separators, same emoji, same `Run ID`/`Exit code` lines, same `┌ … │ … └─` box-drawn failure blocks. End-to-end smoke against the live `junit_fail.xml` fixture exits 1 on a non-quarantined failure and 0 on an all-pass run, matching Python's contract. Co-Authored-By: Claude Opus 4.7 Change-Id: I6bd59de407ddcd4196bdd7e7b8ddb4a88885af7a --- Cargo.lock | 7 + crates/mergify-ci/Cargo.toml | 1 + crates/mergify-ci/src/detector.rs | 10 + .../mergify-ci/src/junit_process/command.rs | 733 ++++++++++++++++++ crates/mergify-ci/src/junit_process/mod.rs | 23 +- .../src/junit_process/quarantine.rs | 330 ++++++++ crates/mergify-cli/src/main.rs | 152 +++- 7 files changed, 1214 insertions(+), 42 deletions(-) create mode 100644 crates/mergify-ci/src/junit_process/command.rs create mode 100644 crates/mergify-ci/src/junit_process/quarantine.rs diff --git a/Cargo.lock b/Cargo.lock index e7dbbb76..3a81358f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -620,6 +620,12 @@ dependencies = [ "wasip3", ] +[[package]] +name = "glob" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280" + [[package]] name = "globset" version = "0.4.18" @@ -1128,6 +1134,7 @@ dependencies = [ "chrono", "flate2", "getrandom 0.3.4", + "glob", "globset", "mergify-config", "mergify-core", diff --git a/crates/mergify-ci/Cargo.toml b/crates/mergify-ci/Cargo.toml index 18773adf..167b65bd 100644 --- a/crates/mergify-ci/Cargo.toml +++ b/crates/mergify-ci/Cargo.toml @@ -13,6 +13,7 @@ publish = false chrono = { version = "0.4", default-features = false, features = ["clock", "serde", "std"] } flate2 = "1" getrandom = "0.3" +glob = "0.3" globset = "0.4" mergify-config = { path = "../mergify-config" } mergify-core = { path = "../mergify-core" } diff --git a/crates/mergify-ci/src/detector.rs b/crates/mergify-ci/src/detector.rs index 8728c5c6..b61b8043 100644 --- a/crates/mergify-ci/src/detector.rs +++ b/crates/mergify-ci/src/detector.rs @@ -382,6 +382,16 @@ fn non_empty_env(name: &str) -> Option { env::var(name).ok().filter(|s| !s.is_empty()) } +/// Branch the quarantine API should look up tests for. Mirrors +/// Python's `get_tests_target_branch`: the PR base branch when +/// available, otherwise the head branch — i.e. "the branch the +/// tests *will* land on" so quarantine decisions match where +/// the merge will go. +#[must_use] +pub fn get_tests_target_branch() -> Option { + get_base_ref_name().or_else(get_head_ref_name) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/mergify-ci/src/junit_process/command.rs b/crates/mergify-ci/src/junit_process/command.rs new file mode 100644 index 00000000..ef03b0e4 --- /dev/null +++ b/crates/mergify-ci/src/junit_process/command.rs @@ -0,0 +1,733 @@ +//! `mergify ci junit-process` orchestration. +//! +//! Glues the four `junit_process` modules together: parse `JUnit` +//! XML → check quarantine status → build OTLP spans (now tagged +//! with `cicd.test.quarantined`) → upload them, then render the +//! human-facing report the way Python's `process_junit_files` +//! does. Errors during quarantine or upload are *non-fatal* by +//! design — the report calls them out but the overall exit code +//! is driven by the failing-tests-not-quarantined count plus the +//! silent-failure detection. + +// The report builder appends formatted snippets to a single +// `String`. clippy's `format_push_string` lint suggests `write!` +// everywhere, which adds a `use std::fmt::Write` and an awkward +// `let _ = write!(…)` per line for no semantic improvement — +// `String::push_str(&format!(…))` is the readable form for this +// kind of templated text emission. +#![allow(clippy::format_push_string)] + +use std::env; +use std::path::{Path, PathBuf}; + +use mergify_core::{CliError, ExitCode, Output}; +use url::Url; + +use crate::detector; +use crate::junit_process::junit::{self, ParseResult, TestCase}; +use crate::junit_process::quarantine::{self, QuarantineFailed, QuarantineResult}; +use crate::junit_process::spans::{self, UploadMetadata}; +use crate::junit_process::upload; + +const SEPARATOR: &str = "══════════════════════════════════════════"; +const SEPARATOR_LIGHT: &str = "──────────────────────────────────────────"; + +/// CLI options for `mergify ci junit-process`. Mirrors the +/// Python flag set — see `mergify_cli/ci/cli.py`. +pub struct JunitProcessOptions<'a> { + pub api_url: Option<&'a str>, + pub token: Option<&'a str>, + pub repository: Option<&'a str>, + pub test_framework: Option<&'a str>, + pub test_language: Option<&'a str>, + pub tests_target_branch: Option<&'a str>, + pub test_exit_code: Option, + /// Raw `files` arguments as the user typed them. Globs (`**`, + /// `*`, `?`) are expanded here, matching Python's + /// `_expand_junit_patterns` callback. + pub files: &'a [String], +} + +/// Run the command. Returns an [`ExitCode`] reflecting the final +/// verdict so the caller can plumb it through to the process +/// exit. Network failures (quarantine / upload) do NOT propagate +/// as errors — they print to the report and the run continues. +/// The only `Err` paths are argument resolution failures (e.g. +/// missing token) and unrecoverable input errors (no XML, parse +/// failure on every file). +#[allow(clippy::too_many_lines)] // Straight-line orchestration: parse → +// quarantine → build spans → upload → render. Splitting this into +// helpers spreads the report builder's ordering across the file +// without buying anything you can't already see by scrolling. +pub async fn run( + opts: JunitProcessOptions<'_>, + output: &mut dyn Output, +) -> Result { + // ── Resolve required inputs up front so we fail before + // printing any of the banner — matches Python's click defaults + // surfacing as exit code 2 when a required flag is missing, + // before the command body runs. + let api_url_raw = resolve_api_url(opts.api_url); + let api_url = Url::parse(&api_url_raw) + .map_err(|e| CliError::Configuration(format!("--api-url is not a valid URL: {e}")))?; + let token = resolve_token(opts.token)?; + let repository = resolve_repository(opts.repository)?; + let tests_target_branch = resolve_tests_target_branch(opts.tests_target_branch)?; + let files = expand_files(opts.files)?; + + // ── Header (printed regardless of outcome). + let mut report = String::new(); + write_header(&mut report); + + // ── Parse. A parse failure aborts before the upload step + // (Python returns ExitCode.GENERIC_ERROR with an inline error + // banner; we do the same). + let parsed = match parse_all(&files) { + Ok(p) => p, + Err(err) => { + write_early_exit( + &mut report, + &format!("Failed to parse JUnit XML: {err}"), + "Check that your test framework is generating valid JUnit XML output.", + ); + emit(output, &report)?; + return Ok(ExitCode::GenericError); + } + }; + + if parsed.cases.is_empty() { + write_early_exit( + &mut report, + "No spans found in the JUnit files", + "Check that the JUnit XML files are not empty.", + ); + emit(output, &report)?; + return Ok(ExitCode::GenericError); + } + + // ── Quarantine check (best effort). Failures here don't + // abort — we fall back to "treat every failure as blocking". + let quarantine_result = quarantine::check_failing( + &api_url, + &token, + &repository, + &tests_target_branch, + &parsed.cases, + ) + .await; + + let (quarantine_result, quarantine_error) = match quarantine_result { + Ok(r) => (r, None::), + Err(QuarantineFailed { message }) => ( + // Conservative fallback: every failure is treated as + // blocking, none as quarantined. + blocking_fallback(&parsed.cases), + Some(message), + ), + }; + + // ── Build spans + upload (best effort). The quarantined set + // gets folded into each case span's `cicd.test.quarantined` + // attribute at build time, so we don't need to re-tag after + // the fact. + let metadata = UploadMetadata { + test_framework: opts.test_framework.map(str::to_string), + test_language: opts.test_language.map(str::to_string), + mergify_test_job_name: env::var("MERGIFY_TEST_JOB_NAME") + .ok() + .filter(|s| !s.is_empty()), + // Let the span builder generate the run_id from random + // bytes — the orchestrator's CLI surface is its own + // top-level command, so there is no caller-supplied + // run_id to honour the way the Python bridge passes one. + run_id: None, + quarantined: quarantine_result + .quarantined + .iter() + .map(|c| c.name.clone()) + .collect(), + }; + let built = spans::build_traces(&parsed, &metadata)?; + + let client = upload::default_client(); + let upload_error = + match upload::upload(&client, &api_url_raw, &token, &repository, &built.request).await { + Ok(()) => None, + Err(err) => Some(err.to_string()), + }; + + // ── Report sections — order matches Python verbatim. + write_run_id(&mut report, &built.run_id); + + let total_cases = count_test_cases(&parsed); + let nb_failures = count_failures(&parsed); + write_upload_summary( + &mut report, + files.len(), + total_cases, + nb_failures, + upload_error.is_some(), + ); + + if let Some(err) = &upload_error { + write_upload_error_block(&mut report, err); + } + + write_quarantine_section(&mut report, &quarantine_result, quarantine_error.as_deref()); + + // ── Silent-failure detection. If the test runner exited + // non-zero but the JUnit report has no failures, the runner + // probably crashed — fail loudly so the user knows the report + // is incomplete. + if let Some(exit_code) = opts.test_exit_code { + if exit_code != 0 && nb_failures == 0 { + write_silent_failure(&mut report, exit_code); + emit(output, &report)?; + return Ok(ExitCode::GenericError); + } + } + + // ── Verdict. + let final_failure_message = + quarantine_failure_message(&quarantine_result, nb_failures, quarantine_error.is_some()); + let nb_quarantined_failures = quarantine_result.failing.len(); + write_verdict( + &mut report, + final_failure_message.as_deref(), + nb_quarantined_failures, + ); + + emit(output, &report)?; + + Ok(if final_failure_message.is_some() { + ExitCode::GenericError + } else { + ExitCode::Success + }) +} + +fn emit(output: &mut dyn Output, report: &str) -> Result<(), CliError> { + output + .emit(&(), &mut |w| w.write_all(report.as_bytes())) + .map_err(|e| CliError::Generic(format!("could not write output: {e}"))) +} + +fn resolve_api_url(explicit: Option<&str>) -> String { + if let Some(v) = explicit.filter(|s| !s.is_empty()) { + return v.to_string(); + } + if let Ok(v) = env::var("MERGIFY_API_URL") { + if !v.is_empty() { + return v; + } + } + "https://api.mergify.com".to_string() +} + +fn resolve_token(explicit: Option<&str>) -> Result { + if let Some(v) = explicit.filter(|s| !s.is_empty()) { + return Ok(v.to_string()); + } + env::var("MERGIFY_TOKEN") + .ok() + .filter(|s| !s.is_empty()) + .ok_or_else(|| { + CliError::Configuration( + "--token not provided and MERGIFY_TOKEN env var is empty".to_string(), + ) + }) +} + +fn resolve_repository(explicit: Option<&str>) -> Result { + if let Some(v) = explicit.filter(|s| !s.is_empty()) { + return Ok(v.to_string()); + } + detector::get_github_repository().ok_or_else(|| { + CliError::Configuration( + "--repository not provided and could not be detected from the CI environment" + .to_string(), + ) + }) +} + +fn resolve_tests_target_branch(explicit: Option<&str>) -> Result { + let raw = if let Some(v) = explicit.filter(|s| !s.is_empty()) { + v.to_string() + } else { + detector::get_tests_target_branch().ok_or_else(|| { + CliError::Configuration( + "--tests-target-branch not provided and could not be detected".to_string(), + ) + })? + }; + // Mirror Python's `_process_tests_target_branch` callback that + // strips `refs/heads/` so the branch name matches what the + // quarantine API expects. + Ok(raw.strip_prefix("refs/heads/").unwrap_or(&raw).to_string()) +} + +/// Expand each `files` entry: existing literal paths take +/// precedence over glob expansion (so `report[1].xml` keeps +/// working), then `**`/`*`/`?` patterns get expanded. A glob that +/// matches nothing is a hard error — same as Python's behavior, +/// since "no test reports" almost always means a previous CI step +/// silently failed and we want the user to see it. +fn expand_files(raw: &[String]) -> Result, CliError> { + if raw.is_empty() { + return Err(CliError::Configuration( + "at least one JUnit XML file path is required".to_string(), + )); + } + // Preserve insertion order while deduplicating — `Vec` + // is small (a handful of XML reports per CI step), so a linear + // `contains` check on each insert is cheaper than a `BTreeSet` + // and keeps ordering deterministic across runs. + let mut out: Vec = Vec::new(); + for entry in raw { + let literal = Path::new(entry); + if literal.is_file() { + if !out.iter().any(|p| p == literal) { + out.push(literal.to_path_buf()); + } + continue; + } + if has_glob_magic(entry) { + let matches = glob::glob(entry).map_err(|e| { + CliError::Configuration(format!("invalid glob pattern {entry:?}: {e}")) + })?; + let mut any_match = false; + for m in matches { + let path = m.map_err(|e| { + CliError::Configuration(format!("glob walk failed for {entry:?}: {e}")) + })?; + if !path.is_file() { + continue; + } + if !out.iter().any(|p| p == &path) { + out.push(path); + } + any_match = true; + } + if !any_match { + return Err(CliError::Configuration(format!( + "Pattern '{entry}' did not match any file.\n\n\ + This usually indicates that a previous CI step failed to generate the test results.\n\ + Please check if your test execution step completed successfully and produced the expected output files." + ))); + } + continue; + } + if literal.is_dir() { + return Err(CliError::Configuration(format!( + "'{entry}' is a directory, not a JUnit XML file.\n\n\ + Pass a file path or a quoted glob pattern (e.g. 'reports/**/*.xml') instead." + ))); + } + return Err(CliError::Configuration(format!( + "JUnit XML file '{entry}' does not exist.\n\n\ + This usually indicates that a previous CI step failed to generate the test results.\n\ + Please check if your test execution step completed successfully and produced the expected output file." + ))); + } + Ok(out) +} + +fn has_glob_magic(s: &str) -> bool { + s.contains(['*', '?', '[']) +} + +fn parse_all(files: &[PathBuf]) -> Result { + // Concatenate the cases from every file. The OTLP layer + // doesn't care which file a case came from — JUnit suites + // already group them, and that grouping is what becomes a + // suite span downstream. `suite_names` from each file is + // appended in order; the OTLP span builder dedupes via its + // `group_by_suite` linear scan. + let mut all_cases = Vec::new(); + let mut all_suite_names = Vec::new(); + for path in files { + let bytes = std::fs::read(path).map_err(|e| junit::InvalidJunitXml { + details: format!("cannot read {}: {e}", path.display()), + })?; + let parsed = junit::parse(&bytes)?; + all_suite_names.extend(parsed.suite_names); + all_cases.extend(parsed.cases); + } + Ok(ParseResult { + suite_names: all_suite_names, + cases: all_cases, + }) +} + +fn count_test_cases(parsed: &ParseResult) -> usize { + parsed.cases.len() +} + +fn count_failures(parsed: &ParseResult) -> usize { + parsed + .cases + .iter() + .filter(|c| c.status.is_failure()) + .count() +} + +fn blocking_fallback(cases: &[TestCase]) -> QuarantineResult { + let failing: Vec = cases + .iter() + .filter(|c| c.status.is_failure()) + .cloned() + .collect(); + let count = failing.len(); + QuarantineResult { + non_quarantined: failing.clone(), + failing, + quarantined: Vec::new(), + failing_not_quarantined_count: count, + } +} + +fn quarantine_failure_message( + result: &QuarantineResult, + nb_failures: usize, + quarantine_errored: bool, +) -> Option { + if quarantine_errored { + return Some(format!( + "Treating {nb_failures}/{nb_failures} failures as blocking" + )); + } + if result.failing_not_quarantined_count > 0 { + let count = result.failing_not_quarantined_count; + let total = result.failing.len(); + let quarantined = total - count; + return Some(format!("{quarantined}/{total} failures quarantined")); + } + None +} + +fn write_header(out: &mut String) { + out.push_str(SEPARATOR); + out.push('\n'); + out.push_str(" 🚀 CI Insights\n"); + out.push('\n'); + out.push_str(concat!( + " Uploads JUnit test results to Mergify CI Insights and evaluates\n", + " quarantine status for failing tests. This step determines the\n", + " final CI status — quarantined failures are ignored.\n", + " Learn more: https://docs.mergify.com/ci-insights/quarantine\n", + )); + out.push_str(SEPARATOR); + out.push('\n'); +} + +fn write_run_id(out: &mut String, run_id: &str) { + out.push('\n'); + out.push_str(&format!(" Run ID: {run_id}\n")); +} + +fn write_upload_summary( + out: &mut String, + reports: usize, + tests: usize, + failures: usize, + upload_failed: bool, +) { + let reports_label = format!( + "{reports} {kind}", + kind = if reports == 1 { "report" } else { "reports" } + ); + let failures_label = format!( + "{failures} {kind}", + kind = if failures == 1 { "failure" } else { "failures" } + ); + if upload_failed { + out.push_str(&format!(" ☁️ {reports_label} not uploaded\n")); + } else { + out.push_str(&format!(" ☁️ {reports_label} uploaded\n")); + } + out.push_str(&format!(" 🧪 {tests} tests ({failures_label})\n")); +} + +fn write_upload_error_block(out: &mut String, error: &str) { + out.push_str("\n ⚠️ Failed to upload test results\n"); + out.push_str(" Mergify CI Insights won't process these test results.\n"); + out.push_str(" Quarantine status and CI outcome are unaffected.\n"); + out.push('\n'); + out.push_str(" ┌ Details\n"); + for line in error.lines() { + out.push_str(&format!(" │ {line}\n")); + } + out.push_str(" └─\n"); +} + +fn write_quarantine_section(out: &mut String, result: &QuarantineResult, error: Option<&str>) { + // Skip the whole section when there's nothing to say — Python + // does the same `if not result.failing_spans and error is None` + // early return. + if result.failing.is_empty() && error.is_none() { + return; + } + out.push('\n'); + out.push_str(SEPARATOR_LIGHT); + out.push('\n'); + out.push('\n'); + out.push_str("🛡️ Quarantine\n"); + + if let Some(err) = error { + out.push('\n'); + out.push_str(" ⚠️ Failed to check quarantine status\n"); + out.push_str(" Contact Mergify support if this error persists.\n"); + out.push('\n'); + out.push_str(" ┌ Details\n"); + for line in err.lines() { + out.push_str(&format!(" │ {line}\n")); + } + out.push_str(" └─\n"); + } + + if !result.quarantined.is_empty() { + out.push_str(&format!( + "\n 🔒 Quarantined ({n}):\n", + n = result.quarantined.len() + )); + for case in &result.quarantined { + out.push_str(&format!(" · {name}\n", name = case.name)); + } + } + + if !result.non_quarantined.is_empty() { + let label = if error.is_some() { + "Could not verify quarantine status" + } else { + "Unquarantined" + }; + out.push_str(&format!( + "\n ❌ {label} ({n}):\n", + n = result.non_quarantined.len() + )); + for case in &result.non_quarantined { + write_failure_block(out, case); + } + } +} + +fn write_failure_block(out: &mut String, case: &TestCase) { + out.push_str(&format!("\n ┌ {name}\n", name = case.name)); + let f = &case.failure; + if f.kind.is_none() && f.message.is_none() && f.stacktrace.is_none() { + out.push_str(" │\n"); + out.push_str(" │ (no error details in JUnit report)\n"); + out.push_str(" └─\n"); + return; + } + if f.kind.is_some() || f.message.is_some() { + let parts: Vec<&str> = [f.kind.as_deref(), f.message.as_deref()] + .into_iter() + .flatten() + .collect(); + out.push_str(" │\n"); + out.push_str(&format!(" │ {joined}\n", joined = parts.join(": "))); + } + if let Some(stack) = &f.stacktrace { + out.push_str(" │\n"); + for line in stack.lines() { + out.push_str(&format!(" │ {line}\n")); + } + } + out.push_str(" └─\n"); +} + +fn write_silent_failure(out: &mut String, test_exit_code: i32) { + out.push('\n'); + out.push_str(SEPARATOR_LIGHT); + out.push('\n'); + out.push('\n'); + out.push_str(&format!( + " ⚠️ Test runner exited with an error (exit code: {test_exit_code})\n" + )); + out.push_str(" but no test failures appear in the JUnit report.\n"); + out.push_str(" The report may be incomplete — check your test runner logs.\n"); + out.push('\n'); + out.push_str(SEPARATOR); + out.push('\n'); + out.push_str("❌ FAIL — test runner exited with an error but no failures were reported\n"); + out.push_str(&format!( + " Exit code: {code}\n", + code = ExitCode::GenericError.as_u8() + )); + out.push_str(SEPARATOR); + out.push('\n'); +} + +fn write_verdict(out: &mut String, failure_message: Option<&str>, nb_quarantined_failures: usize) { + out.push('\n'); + out.push_str(SEPARATOR); + out.push('\n'); + if let Some(msg) = failure_message { + out.push_str(&format!("❌ FAIL — {msg}\n")); + out.push_str(" Exit code: 1\n"); + } else if nb_quarantined_failures == 0 { + out.push_str("✅ OK — all tests passed, no quarantine needed\n"); + out.push_str(" Exit code: 0\n"); + } else { + let n = nb_quarantined_failures; + out.push_str(&format!( + "✅ OK — {n}/{n} failures quarantined, CI status unaffected\n", + )); + out.push_str(" Exit code: 0\n"); + } + out.push_str(SEPARATOR); + out.push('\n'); +} + +fn write_early_exit(out: &mut String, message: &str, hint: &str) { + out.push_str(&format!("❌ FAIL — {message}\n")); + out.push_str(&format!(" {hint}\n")); + out.push_str(" Exit code: 1\n"); + out.push_str(SEPARATOR); + out.push('\n'); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::junit_process::junit::{Failure, TestStatus}; + use std::time::Duration; + + fn case(name: &str, status: TestStatus) -> TestCase { + TestCase { + name: name.to_string(), + suite_name: "s".to_string(), + duration: Some(Duration::from_secs(0)), + file: None, + line: None, + status, + failure: Failure::default(), + } + } + + #[test] + fn quarantine_failure_message_signals_blocking_when_check_errored() { + let result = QuarantineResult { + failing: vec![case("a", TestStatus::Failed), case("b", TestStatus::Failed)], + non_quarantined: vec![case("a", TestStatus::Failed), case("b", TestStatus::Failed)], + quarantined: vec![], + failing_not_quarantined_count: 2, + }; + let msg = quarantine_failure_message(&result, 2, true); + // Pythonic phrasing: "Treating X/X failures as blocking". + assert_eq!(msg.as_deref(), Some("Treating 2/2 failures as blocking")); + } + + #[test] + fn quarantine_failure_message_says_quarantined_when_some_pass() { + let result = QuarantineResult { + failing: vec![case("a", TestStatus::Failed), case("b", TestStatus::Failed)], + quarantined: vec![case("a", TestStatus::Failed)], + non_quarantined: vec![case("b", TestStatus::Failed)], + failing_not_quarantined_count: 1, + }; + // 1/2 still blocking → message says "1/2 quarantined". + let msg = quarantine_failure_message(&result, 2, false); + assert_eq!(msg.as_deref(), Some("1/2 failures quarantined")); + } + + #[test] + fn quarantine_failure_message_none_when_all_quarantined() { + let result = QuarantineResult { + failing: vec![case("a", TestStatus::Failed)], + quarantined: vec![case("a", TestStatus::Failed)], + non_quarantined: vec![], + failing_not_quarantined_count: 0, + }; + // Every failure quarantined → no failure message. + assert_eq!(quarantine_failure_message(&result, 1, false), None); + } + + #[test] + fn expand_files_rejects_unknown_path() { + let err = expand_files(&["does/not/exist.xml".to_string()]).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("does/not/exist.xml") && msg.contains("does not exist"), + "got: {msg}" + ); + } + + #[test] + fn expand_files_rejects_directory() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("sub"); + std::fs::create_dir(&dir).unwrap(); + let err = expand_files(&[dir.to_string_lossy().to_string()]).unwrap_err(); + assert!(err.to_string().contains("directory"), "got: {err}"); + } + + #[test] + fn expand_files_dedupes_repeated_literal_paths() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("a.xml"); + std::fs::write(&path, b"x").unwrap(); + let raw = path.to_string_lossy().to_string(); + let out = expand_files(&[raw.clone(), raw]).unwrap(); + assert_eq!(out.len(), 1); + } + + #[test] + fn expand_files_rejects_pattern_with_no_matches() { + let tmp = tempfile::tempdir().unwrap(); + // tempdir is empty — a wildcard for *.xml here matches nothing. + let pattern = tmp.path().join("*.xml").to_string_lossy().to_string(); + let err = expand_files(&[pattern]).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("did not match any file"), "got: {msg}"); + } + + #[test] + fn write_verdict_renders_blocking_failure() { + let mut s = String::new(); + write_verdict(&mut s, Some("1/2 failures quarantined"), 0); + // The "Exit code: 1" line is what users grep for in CI logs. + assert!(s.contains("❌ FAIL — 1/2 failures quarantined"), "{s}"); + assert!(s.contains("Exit code: 1"), "{s}"); + } + + #[test] + fn write_verdict_renders_all_pass() { + let mut s = String::new(); + write_verdict(&mut s, None, 0); + assert!(s.contains("✅ OK — all tests passed"), "{s}"); + assert!(s.contains("Exit code: 0"), "{s}"); + } + + #[test] + fn write_verdict_renders_all_quarantined_pass() { + let mut s = String::new(); + write_verdict(&mut s, None, 3); + // "3/3 failures quarantined" — the second arm of the verdict. + assert!(s.contains("3/3 failures quarantined"), "{s}"); + assert!(s.contains("Exit code: 0"), "{s}"); + } + + #[test] + fn write_failure_block_handles_missing_details() { + let mut s = String::new(); + write_failure_block(&mut s, &case("orphan", TestStatus::Failed)); + assert!(s.contains("(no error details in JUnit report)"), "{s}"); + } + + #[test] + fn write_failure_block_joins_kind_and_message() { + let mut s = String::new(); + let mut c = case("t", TestStatus::Failed); + c.failure.kind = Some("AssertionError".to_string()); + c.failure.message = Some("assert 1 == 0".to_string()); + c.failure.stacktrace = Some("line1\nline2".to_string()); + write_failure_block(&mut s, &c); + // Kind and message joined with ": ", stacktrace lines each + // prefixed with the box-drawing column. + assert!(s.contains("AssertionError: assert 1 == 0"), "{s}"); + assert!(s.contains("│ line1"), "{s}"); + assert!(s.contains("│ line2"), "{s}"); + } +} diff --git a/crates/mergify-ci/src/junit_process/mod.rs b/crates/mergify-ci/src/junit_process/mod.rs index c56c5742..68c0b588 100644 --- a/crates/mergify-ci/src/junit_process/mod.rs +++ b/crates/mergify-ci/src/junit_process/mod.rs @@ -7,23 +7,24 @@ //! - **Phase A** (landed) — [`junit`]: `JUnit` XML parser //! producing semantically-tagged [`TestCase`] values. //! Hermetic, no network. -//! - **Phase B** (this commit) — [`spans`] turns parser output -//! into an OTLP `ExportTraceServiceRequest`; [`upload`] gzips -//! that protobuf payload and POSTs it to +//! - **Phase B** (landed) — [`spans`] turns parser output into an +//! OTLP `ExportTraceServiceRequest`; [`upload`] gzips that +//! protobuf payload and POSTs it to //! `/v1/repos///ci/traces`. -//! - **Phase C** (next) — quarantine API client, CLI dispatch, -//! and `Subcommands::Ci(CiSubcommand::JunitProcess)` promotion -//! from shim to native. -//! -//! Until Phase C lands, the binary keeps shimming -//! `ci junit-process` to Python — but the parser and uploader -//! already live here so the dispatch layer just needs to wire -//! them together. +//! - **Phase C** (this commit) — [`quarantine`] queries the +//! quarantine API; [`command::run`] orchestrates everything and +//! renders the human report so the binary can promote +//! `Subcommands::Ci(CiSubcommand::JunitProcess)` from shim to +//! native. +pub mod command; pub mod junit; +pub mod quarantine; pub mod spans; pub mod upload; +pub use command::{JunitProcessOptions, run}; pub use junit::{Failure, InvalidJunitXml, ParseResult, TestCase, TestStatus}; +pub use quarantine::{QuarantineFailed, QuarantineResult, QuarantinedTests}; pub use spans::{BuiltTraces, UploadMetadata, build_traces}; pub use upload::{UploadError, default_client, upload}; diff --git a/crates/mergify-ci/src/junit_process/quarantine.rs b/crates/mergify-ci/src/junit_process/quarantine.rs new file mode 100644 index 00000000..66ae783a --- /dev/null +++ b/crates/mergify-ci/src/junit_process/quarantine.rs @@ -0,0 +1,330 @@ +//! Quarantine API client for `mergify ci junit-process`. +//! +//! Mirrors `mergify_cli/ci/junit_processing/quarantine.py`. The +//! API answers "for these failing tests on this branch, which are +//! currently quarantined?" — failures of quarantined tests are +//! ignored by the final CI verdict; failures of non-quarantined +//! tests still block. +//! +//! Endpoint shape: +//! `POST {api_url}/v1/ci/{owner}/repositories/{repo}/quarantines/check` +//! ```json +//! { "tests_names": [...], "branch": "..." } +//! ``` +//! returns +//! ```json +//! { "quarantined_tests_names": [...], "non_quarantined_tests_names": [...] } +//! ``` + +use std::collections::BTreeSet; + +use mergify_core::{ApiFlavor, CliError, HttpClient}; +use serde::{Deserialize, Serialize}; +use url::Url; + +use crate::detector; +use crate::junit_process::junit::TestCase; + +/// What the quarantine API told us about a set of failing test +/// case names — sets so membership checks are O(log n) when we +/// later tag each span. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct QuarantinedTests { + pub quarantined: BTreeSet, + pub non_quarantined: BTreeSet, +} + +/// Cross-cutting view of a `junit-process` run: which case names +/// failed, which the backend says are currently quarantined, and +/// which are not. Drives the OTLP attribute tagging (a failing +/// case in the quarantined set gets `cicd.test.quarantined = +/// true`) as well as the CLI verdict (a non-zero count of +/// non-quarantined failures means the run fails). +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct QuarantineResult { + /// Every failing test case (status = Failed or Errored). + pub failing: Vec, + /// Subset of `failing` whose names appear in the + /// `quarantined_tests_names` API response. + pub quarantined: Vec, + /// Subset of `failing` the API explicitly reported as + /// non-quarantined. May be a strict subset of + /// `failing - quarantined` when the API silently dropped some + /// names; we trust the API's split rather than reconstructing + /// it locally, to match Python. + pub non_quarantined: Vec, + /// Count of failing tests that are NOT quarantined. This is + /// what determines the final exit code: zero → CI passes, + /// non-zero → CI fails. + pub failing_not_quarantined_count: usize, +} + +#[derive(Debug, Clone)] +pub struct QuarantineFailed { + pub message: String, +} + +impl std::fmt::Display for QuarantineFailed { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for QuarantineFailed {} + +/// Find every test case in `cases` whose status is a failure +/// (`Failed` or `Errored`). Mirrors Python's filter; the spans +/// inheriting this property are tagged `cicd.test.quarantined` +/// at the `spans` layer based on the result of [`check`]. +fn failing_cases(cases: &[TestCase]) -> Vec { + cases + .iter() + .filter(|c| c.status.is_failure()) + .cloned() + .collect() +} + +/// Query the Mergify CI Insights quarantine API for the names in +/// `failing_names` against the given branch. Returns the API's +/// own split — we do NOT reconstruct `non_quarantined = +/// failing - quarantined` locally, to match Python's behavior. +/// +/// `failing_names` may contain duplicates if the `JUnit` input has +/// the same test name reported by multiple suites; that's fine — +/// the API treats names as a set. +pub async fn check( + api_url: &Url, + token: &str, + repository: &str, + branch: &str, + failing_names: &[String], +) -> Result { + let (owner, repo) = detector::split_owner_repo(repository).map_err(|e| QuarantineFailed { + message: e.to_string(), + })?; + + let client = HttpClient::new(api_url.clone(), token, ApiFlavor::Mergify).map_err(|e| { + QuarantineFailed { + message: e.to_string(), + } + })?; + + let path = format!("/v1/ci/{owner}/repositories/{repo}/quarantines/check"); + let body = CheckRequest { + tests_names: failing_names, + branch, + }; + + let resp: CheckResponse = client + .post(&path, &body) + .await + .map_err(|e| QuarantineFailed { + message: e.to_string(), + })?; + + Ok(QuarantinedTests { + quarantined: resp.quarantined_tests_names.into_iter().collect(), + non_quarantined: resp.non_quarantined_tests_names.into_iter().collect(), + }) +} + +/// Categorize the failing test cases into quarantined and +/// non-quarantined buckets, given the API's verdict. The result +/// keeps the failing-cases list intact so the CLI can render the +/// "X/Y failures quarantined" summary without re-walking the +/// original `JUnit` input. +#[must_use] +pub fn categorize(failing: Vec, verdict: &QuarantinedTests) -> QuarantineResult { + let mut quarantined = Vec::new(); + let mut non_quarantined = Vec::new(); + let mut failing_not_quarantined_count = 0; + + for case in &failing { + let is_quarantined = verdict.quarantined.contains(&case.name); + if is_quarantined { + quarantined.push(case.clone()); + } else { + failing_not_quarantined_count += 1; + if verdict.non_quarantined.contains(&case.name) { + non_quarantined.push(case.clone()); + } + } + } + + QuarantineResult { + failing, + quarantined, + non_quarantined, + failing_not_quarantined_count, + } +} + +/// Resolve the failing test cases for `cases`, hit the quarantine +/// API, and combine the two into a [`QuarantineResult`]. The CLI +/// orchestration uses the bundled fn so the happy path is a single +/// call instead of three. +pub async fn check_failing( + api_url: &Url, + token: &str, + repository: &str, + branch: &str, + cases: &[TestCase], +) -> Result { + let failing = failing_cases(cases); + if failing.is_empty() { + return Ok(QuarantineResult::default()); + } + let names: Vec = failing.iter().map(|c| c.name.clone()).collect(); + let verdict = check(api_url, token, repository, branch, &names).await?; + Ok(categorize(failing, &verdict)) +} + +/// Lift a [`QuarantineFailed`] into the shared [`CliError`] so +/// callers can `?` it. +impl From for CliError { + fn from(err: QuarantineFailed) -> Self { + Self::Generic(err.message) + } +} + +#[derive(Serialize)] +struct CheckRequest<'a> { + tests_names: &'a [String], + branch: &'a str, +} + +#[derive(Deserialize)] +struct CheckResponse { + quarantined_tests_names: Vec, + non_quarantined_tests_names: Vec, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::junit_process::junit::{Failure, TestStatus}; + use std::time::Duration; + use wiremock::matchers::{body_json, header, method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + fn case(name: &str, status: TestStatus) -> TestCase { + TestCase { + name: name.to_string(), + suite_name: "s".to_string(), + duration: Some(Duration::from_secs(0)), + file: None, + line: None, + status, + failure: Failure::default(), + } + } + + #[test] + fn categorize_buckets_quarantined_separately() { + let failing = vec![ + case("a", TestStatus::Failed), + case("b", TestStatus::Errored), + case("c", TestStatus::Failed), + ]; + let verdict = QuarantinedTests { + quarantined: ["a".to_string()].into_iter().collect(), + non_quarantined: ["b".to_string(), "c".to_string()].into_iter().collect(), + }; + let r = categorize(failing, &verdict); + assert_eq!( + r.quarantined.iter().map(|c| &c.name).collect::>(), + vec!["a"] + ); + assert_eq!( + r.non_quarantined + .iter() + .map(|c| &c.name) + .collect::>(), + vec!["b", "c"] + ); + // 2 failures not quarantined — drives the non-zero exit code. + assert_eq!(r.failing_not_quarantined_count, 2); + } + + #[test] + fn categorize_counts_unknown_as_not_quarantined() { + // The API may omit names it doesn't recognize (e.g. typo, + // never seen before). Python treats those as failures that + // weren't quarantined → must count toward + // `failing_not_quarantined_count` even though they're not + // explicitly listed in `non_quarantined_tests_names`. + let failing = vec![case("x", TestStatus::Failed)]; + let verdict = QuarantinedTests::default(); + let r = categorize(failing, &verdict); + assert!(r.quarantined.is_empty()); + assert!(r.non_quarantined.is_empty()); + assert_eq!(r.failing_not_quarantined_count, 1); + } + + #[tokio::test] + async fn check_posts_to_owner_scoped_path() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/ci/owner/repositories/repo/quarantines/check")) + .and(header("Authorization", "Bearer secret")) + .and(body_json(serde_json::json!({ + "tests_names": ["t1", "t2"], + "branch": "main", + }))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "quarantined_tests_names": ["t1"], + "non_quarantined_tests_names": ["t2"], + }))) + .mount(&server) + .await; + + let api_url = Url::parse(&server.uri()).unwrap(); + let verdict = check( + &api_url, + "secret", + "owner/repo", + "main", + &["t1".to_string(), "t2".to_string()], + ) + .await + .expect("API call succeeds"); + assert_eq!(verdict.quarantined.len(), 1); + assert!(verdict.quarantined.contains("t1")); + assert!(verdict.non_quarantined.contains("t2")); + } + + #[tokio::test] + async fn check_failing_short_circuits_when_no_failures() { + // Empty failing list → no HTTP call, no QuarantineResult to + // categorize. Mirrors Python's early return. If the function + // accidentally tried to POST, the bogus URL would fail. + let api_url = Url::parse("http://127.0.0.1:1").unwrap(); + let cases = vec![ + case("ok", TestStatus::Passed), + case("skipped", TestStatus::Skipped), + ]; + let r = check_failing(&api_url, "tok", "owner/repo", "main", &cases) + .await + .expect("must short-circuit"); + assert!(r.failing.is_empty()); + assert_eq!(r.failing_not_quarantined_count, 0); + } + + #[tokio::test] + async fn check_surfaces_non_200_as_quarantine_failed() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(503).set_body_string("backend down")) + .mount(&server) + .await; + let api_url = Url::parse(&server.uri()).unwrap(); + let err = check(&api_url, "tok", "owner/repo", "main", &["t".to_string()]) + .await + .expect_err("503 must surface as QuarantineFailed"); + assert!( + err.message.contains("503") || err.message.contains("backend down"), + "got: {}", + err.message, + ); + } +} diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index 1e43d322..25734c6f 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -28,6 +28,7 @@ use clap::Parser; use clap::Subcommand; use mergify_ci::git_refs::Format as GitRefsFormat; use mergify_ci::git_refs::GitRefsOptions; +use mergify_ci::junit_process::JunitProcessOptions; use mergify_ci::scopes_send::ScopesSendOptions; use mergify_ci::tests_show::TestsShowOptions; use mergify_config::simulate::PullRequestRef; @@ -146,6 +147,7 @@ const NATIVE_COMMANDS: &[(&str, &str)] = &[ ("ci", "scopes-send"), ("ci", "git-refs"), ("ci", "queue-info"), + ("ci", "junit-process"), ("tests", "show"), ("queue", "pause"), ("queue", "unpause"), @@ -176,6 +178,7 @@ enum NativeCommand { format: GitRefsFormat, }, CiQueueInfo, + CiJunitProcess(CiJunitProcessOpts), TestsShow(TestsShowOpts), QueuePause(QueuePauseOpts), QueueUnpause(QueueUnpauseOpts), @@ -239,6 +242,17 @@ struct CiScopesSendOpts { file_deprecated: Option, } +struct CiJunitProcessOpts { + api_url: Option, + token: Option, + repository: Option, + test_framework: Option, + test_language: Option, + tests_target_branch: Option, + test_exit_code: Option, + files: Vec, +} + struct QueuePauseOpts { repository: Option, token: Option, @@ -454,11 +468,27 @@ fn dispatch_from_parsed(parsed: CliRoot) -> Dispatch { write, })), Subcommands::Ci(CiArgs { - command: CiSubcommand::JunitProcess(ShimmedArgs { args }), - }) => Dispatch::Shim(inject_global_flags( - debug, - prepend_two("ci", "junit-process", args), - )), + command: + CiSubcommand::JunitProcess(JunitProcessCliArgs { + api_url, + token, + repository, + test_framework, + test_language, + tests_target_branch, + test_exit_code, + files, + }), + }) => Dispatch::Native(NativeCommand::CiJunitProcess(CiJunitProcessOpts { + api_url, + token, + repository, + test_framework, + test_language, + tests_target_branch, + test_exit_code, + files, + })), Subcommands::Ci(CiArgs { command: CiSubcommand::JunitUpload(ShimmedArgs { args }), }) => Dispatch::Shim(inject_global_flags( @@ -749,6 +779,22 @@ fn run_native(cmd: NativeCommand) -> ExitCode { NativeCommand::CiQueueInfo => { mergify_ci::queue_info::run(&mut output).map(|()| mergify_core::ExitCode::Success) } + NativeCommand::CiJunitProcess(opts) => { + mergify_ci::junit_process::run( + JunitProcessOptions { + api_url: opts.api_url.as_deref(), + token: opts.token.as_deref(), + repository: opts.repository.as_deref(), + test_framework: opts.test_framework.as_deref(), + test_language: opts.test_language.as_deref(), + tests_target_branch: opts.tests_target_branch.as_deref(), + test_exit_code: opts.test_exit_code, + files: &opts.files, + }, + &mut output, + ) + .await + } NativeCommand::CiScopes(opts) => mergify_ci::scopes_detect::run( mergify_ci::scopes_detect::ScopesOptions { config: opts.config.as_deref(), @@ -1139,6 +1185,11 @@ struct CiArgs { } #[derive(Subcommand)] +// CiSubcommand variant docstrings double as `mergify ci --help` +// entries — clap renders them verbatim, so backticks would surface +// as literal characters to the user. Suppress doc_markdown here +// so the help text reads naturally. +#[allow(clippy::doc_markdown)] enum CiSubcommand { /// Send scopes tied to a pull request to Mergify. #[command(name = "scopes-send")] @@ -1151,11 +1202,11 @@ enum CiSubcommand { QueueInfo, /// Give the list of scopes impacted by changed files. Scopes(ScopesCliArgs), - /// Upload `JUnit` XML reports and ignore failed tests with + /// Upload JUnit XML reports and ignore failed tests with /// Mergify's CI Insights Quarantine. #[command(name = "junit-process")] - JunitProcess(ShimmedArgs), - /// Upload `JUnit` XML reports (deprecated: use `junit-process`). + JunitProcess(JunitProcessCliArgs), + /// Upload JUnit XML reports (deprecated: use `junit-process`). #[command(name = "junit-upload")] JunitUpload(ShimmedArgs), } @@ -1237,6 +1288,54 @@ struct ScopesSendCliArgs { file_deprecated: Option, } +#[derive(clap::Args)] +// Help text is rendered verbatim by clap; backticks would surface +// as literal characters to the user. Suppress the doc_markdown +// lint just for this struct so the docstrings read naturally in +// `--help` output. +#[allow(clippy::doc_markdown)] +struct JunitProcessCliArgs { + /// Mergify API URL. Falls back to ``MERGIFY_API_URL`` env var, + /// then to the default (`https://api.mergify.com`). + #[arg(long = "api-url", short = 'u')] + api_url: Option, + + /// CI Issues application key. Falls back to ``MERGIFY_TOKEN``. + #[arg(long, short = 't')] + token: Option, + + /// Repository full name (owner/repo). Auto-detected from the + /// CI environment when omitted. + #[arg(long, short = 'r')] + repository: Option, + + /// Test framework label (e.g. `pytest`). Optional; passed as a + /// span attribute. + #[arg(long = "test-framework")] + test_framework: Option, + + /// Test language label (e.g. `python`). Optional; passed as a + /// span attribute. + #[arg(long = "test-language")] + test_language: Option, + + /// Branch the quarantine API should look up tests on. Defaults + /// to the PR base branch, or the head branch as a fallback. + #[arg(long = "tests-target-branch", short = 'b')] + tests_target_branch: Option, + + /// Exit code of the test runner. When this is non-zero but no + /// failures appear in the JUnit report, the run is flagged + /// as a silent failure. Falls back to ``MERGIFY_TEST_EXIT_CODE``. + #[arg(long = "test-exit-code", short = 'e', env = "MERGIFY_TEST_EXIT_CODE")] + test_exit_code: Option, + + /// JUnit XML files or glob patterns (e.g. + /// `reports/**/*.xml`). At least one path or pattern is required. + #[arg(value_name = "FILE", required = true, num_args = 1..)] + files: Vec, +} + #[derive(clap::Args)] struct TestsArgs { #[command(subcommand)] @@ -1556,28 +1655,19 @@ mod tests { #[test] fn shimmed_dispatch_reinjects_debug_for_ci_subcommand() { - // The remaining two-token shim paths (`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. `ci scopes` is now - // native and follows the native dispatch path. - for (group, sub, tail) in &[ - ("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"); - } + // The remaining two-token shim path (`ci junit-upload`) + // needs the same treatment as the single-token `stack` + // shim. `ci scopes` and `ci junit-process` are now native + // and follow the native dispatch path — they're verified + // through `dispatch_from_parsed` returning a `Native` + // variant elsewhere, not through this shim-loop test. + let parsed = parse(&["--debug", "ci", "junit-upload", "--files", "a.xml"]); + let Dispatch::Shim(argv) = dispatch_from_parsed(parsed) else { + panic!("ci junit-upload must dispatch to the Python shim"); + }; + assert_eq!( + argv, + vec!["--debug", "ci", "junit-upload", "--files", "a.xml"] + ); } }