diff --git a/crates/mergify-cli/tests/live_smoke.rs b/crates/mergify-cli/tests/live_smoke.rs index d35019d1..67f993f6 100644 --- a/crates/mergify-cli/tests/live_smoke.rs +++ b/crates/mergify-cli/tests/live_smoke.rs @@ -217,6 +217,24 @@ fn live_admin_token() -> Option { (!token.is_empty()).then_some(token) } +/// 8 random hex-ish chars, so concurrent or repeated runs never +/// fight over the same row on the shared canary repository. +/// `tempfile`'s name generation is already a dependency and draws +/// from `getrandom`, so it saves pulling in a uuid crate for this. +fn unique_suffix() -> String { + let dir = tempfile::tempdir().expect("tempdir for entropy"); + let name = dir + .path() + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("00000000") + .to_string(); + // tempdir names are like ".tmpXXXXXX" — take a tail slice so + // the literal prefix is not included. + let tail: String = name.chars().rev().take(8).collect(); + tail.chars().rev().collect::() +} + /// Helper for "early-return on missing token". Cargo's stock /// test harness doesn't have a "skip" outcome — early-returning /// counts as pass. Logging `SKIP:` to stderr makes the elided @@ -509,25 +527,8 @@ fn freeze_create_update_delete_roundtrip() { let token = skip_if_unset!(live_admin_token()); // Unique reason so concurrent or repeated runs don't fight - // over the same row. The Python suite uses - // `uuid.uuid4().hex[:8]`; reproduce that entropy with - // `tempfile`'s name-generation (32 hex chars from - // `getrandom`), truncated to 8. - let suffix = { - let dir = tempfile::tempdir().expect("tempdir for entropy"); - let name = dir - .path() - .file_name() - .and_then(|s| s.to_str()) - .unwrap_or("00000000") - .to_string(); - // tempdir names are like ".tmpXXXXXX" or "tmpXXXXXX" with - // ~6+ random chars after the prefix. Take a tail slice so - // we don't include the literal prefix. - let tail: String = name.chars().rev().take(8).collect(); - tail.chars().rev().collect::() - }; - let reason = format!("func-tests-live-smoke-{suffix}"); + // over the same row. + let reason = format!("func-tests-live-smoke-{}", unique_suffix()); let create = cli(&[ "freeze", @@ -608,7 +609,7 @@ fn freeze_create_update_delete_roundtrip() { } // --------------------------------------------------------------- -// CI-Insights test reads (admin token). +// CI-Insights tests family (admin token). // --------------------------------------------------------------- #[test] @@ -654,6 +655,137 @@ fn tests_show_no_match() { ); } +/// RAII cleanup for `tests_quarantine_add_remove_roundtrip` — +/// runs `quarantines remove` from `Drop` so a failed assertion +/// mid-test still leaves the canary repository clean. Same +/// warn-don't-panic posture as [`DeleteFreezeOnDrop`]: panicking +/// in `Drop` during an unwind aborts the process and buries the +/// real assertion message. +struct RemoveQuarantineOnDrop<'a> { + token: &'a str, + test_name: &'a str, +} +impl Drop for RemoveQuarantineOnDrop<'_> { + fn drop(&mut self) { + let remove = cli(&[ + "tests", + "quarantines", + "remove", + "--api-url", + API_URL, + "--token", + self.token, + "--repository", + REPOSITORY, + self.test_name, + ]); + // Two outcomes leave the repository clean: exit 0, when + // the body failed before its own remove, and the happy + // path's exit 6 — `MergifyApiError` — carrying + // `not_found`'s `'' is not quarantined`, because the + // body already removed the row. + // + // Both halves are load-bearing. Exit 6 alone is every + // Mergify API error, a 403 on a narrowed scope included; + // the message alone would swallow any failure whose text + // happens to contain that phrase. Anything else is a + // cleanup that did not happen, and the row is still there. + let already_gone = remove.returncode + == i32::from(mergify_core::ExitCode::MergifyApiError.as_u8()) + && remove.combined().contains("is not quarantined"); + if remove.returncode != 0 && !already_gone { + eprintln!("WARNING: quarantine cleanup failed{}", remove.context()); + } + } +} + +#[test] +fn tests_quarantine_add_remove_roundtrip() { + // `POST` + `DELETE` round-trip on + // `/v1/ci/{owner}/repositories/{repo}/quarantines`. + // + // MRGFY-9001 dropped `ci_application_key` from both of these + // alongside `search/tests`, and nothing here noticed, because + // the quarantine mutations had no live coverage at all — the + // wiremock tests in `tests_quarantine.rs` cannot see a scope + // change, since a mock server never authenticates. This is + // that canary: it fires on an auth, URL or wire-format change + // to either endpoint. + // + // Quarantines a name no real suite reports, so the entry can + // never suppress a genuine failure on the canary repository + // even if cleanup is skipped entirely. + let token = skip_if_unset!(live_admin_token()); + + let test_name = format!("__mergify_cli_smoke_quarantine_{}__", unique_suffix()); + let reason = "func-tests-live-smoke"; + + let add = cli(&[ + "tests", + "quarantines", + "add", + "--api-url", + API_URL, + "--token", + &token, + "--repository", + REPOSITORY, + "--reason", + reason, + "--json", + &test_name, + ]); + assert_eq!(add.returncode, 0, "quarantines add failed{}", add.context()); + + // Registered before the payload is parsed: an unparseable + // body still means the row was created server-side. + let _cleanup = RemoveQuarantineOnDrop { + token: &token, + test_name: &test_name, + }; + + let added: Value = serde_json::from_str(&add.stdout).unwrap_or_else(|e| { + panic!( + "quarantines add --json emitted non-JSON output\nerror: {e}\nstdout:\n{}", + add.stdout + ) + }); + assert_eq!( + added["test_name"], + serde_json::json!(test_name), + "add echoed a different test name\nstdout:\n{}", + add.stdout + ); + assert!( + added["id"].as_str().is_some_and(|id| !id.is_empty()), + "add emitted no quarantine id\nstdout:\n{}", + add.stdout + ); + + // Remove by name, which is the path that resolves the id via + // the list endpoint — so one run covers both the `ci`-allowed + // read and the admin-only delete. + let remove = cli(&[ + "tests", + "quarantines", + "remove", + "--api-url", + API_URL, + "--token", + &token, + "--repository", + REPOSITORY, + "--json", + &test_name, + ]); + assert_eq!( + remove.returncode, + 0, + "quarantines remove failed{}", + remove.context() + ); +} + // --------------------------------------------------------------- // CI commands — locally evaluated, no token needed. // ---------------------------------------------------------------