From c228d7088582f479d5c2253490ca665c3998f71a Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 12 May 2026 15:29:02 +0200 Subject: [PATCH 1/2] test(skill): port the skill-references test to Rust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces ``mergify_cli/tests/queue/test_skill.py`` with a Rust integration test in ``crates/mergify-cli/tests/skill_references.rs``. The test validates two artifacts with no Python in the picture: - The ``skills/mergify-merge-queue/SKILL.md`` Markdown file (frontmatter shape, required sections). - The Rust binary's ``--list-native-commands`` output (every ``mergify queue `` reference in the skill must resolve to a native command). Keeping the test in pytest meant carrying Python plumbing for a language-agnostic concern. The Rust port: - spawns the binary via ``CARGO_BIN_EXE_mergify`` (the artifact ``cargo test`` just built), so the test always exercises the current code rather than whatever ``mergify`` happens to be on ``PATH``; - reads the skill file via ``CARGO_MANIFEST_DIR``-relative resolution, so it's robust to the cwd; - adds two dev-deps to ``mergify-cli`` (``regex`` for the frontmatter / reference patterns, ``serde_yaml_ng`` for frontmatter parsing — already used by ``mergify-ci``). The ``mergify_cli/tests/queue/`` directory had no other content, so it goes away entirely; the Python ``yaml`` dev-dep stays in place for now (still used elsewhere). 4 tests, same coverage as before: - ``skill_content_is_readable`` - ``skill_has_valid_frontmatter`` - ``skill_has_required_sections`` - ``skill_references_valid_commands`` Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I7eb5e3849dcb4219341be78173717ecd137f2d08 --- Cargo.lock | 2 + crates/mergify-cli/Cargo.toml | 4 + crates/mergify-cli/tests/skill_references.rs | 137 +++++++++++++++++++ mergify_cli/tests/queue/__init__.py | 0 mergify_cli/tests/queue/test_skill.py | 118 ---------------- 5 files changed, 143 insertions(+), 118 deletions(-) create mode 100644 crates/mergify-cli/tests/skill_references.rs delete mode 100644 mergify_cli/tests/queue/__init__.py delete mode 100644 mergify_cli/tests/queue/test_skill.py diff --git a/Cargo.lock b/Cargo.lock index 68fa00d7..f8eeff91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1083,6 +1083,8 @@ dependencies = [ "mergify-core", "mergify-py-shim", "mergify-queue", + "regex", + "serde_yaml_ng", "tokio", ] diff --git a/crates/mergify-cli/Cargo.toml b/crates/mergify-cli/Cargo.toml index 7d28db2c..a3b26400 100644 --- a/crates/mergify-cli/Cargo.toml +++ b/crates/mergify-cli/Cargo.toml @@ -22,5 +22,9 @@ mergify-py-shim = { path = "../mergify-py-shim" } mergify-queue = { path = "../mergify-queue" } tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } +[dev-dependencies] +regex = "1" +serde_yaml_ng = "0.10" + [lints] workspace = true diff --git a/crates/mergify-cli/tests/skill_references.rs b/crates/mergify-cli/tests/skill_references.rs new file mode 100644 index 00000000..e0a8a2f7 --- /dev/null +++ b/crates/mergify-cli/tests/skill_references.rs @@ -0,0 +1,137 @@ +//! Cross-checks the `mergify-merge-queue` skill against the +//! freshly-built test binary. +//! +//! Replaces the pre-port Python `tests/queue/test_skill.py`: the +//! artifacts being validated (a Markdown skill file and the Rust +//! binary's `--list-native-commands` output) have no Python in +//! the picture, so the test lives next to the binary that emits +//! the truth. +//! +//! Each test fires the freshly-built binary via +//! `CARGO_BIN_EXE_mergify` — that's the same artifact `cargo test` +//! built moments earlier, so the test always exercises the +//! current code rather than whatever happens to be on `PATH`. + +use std::collections::BTreeSet; +use std::path::PathBuf; +use std::process::Command; + +use regex::Regex; +use serde_yaml_ng::Value; + +const REQUIRED_SECTIONS: &[&str] = &[ + "## Commands", + "## Checking Queue Status", + "## Inspecting a PR", + "## Queue States", + "## Troubleshooting", +]; + +/// Resolve `skills/mergify-merge-queue/SKILL.md` from the +/// repository root. `CARGO_MANIFEST_DIR` points at this crate's +/// directory; two `..` hops up to the workspace root. +fn skill_path() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .join("skills") + .join("mergify-merge-queue") + .join("SKILL.md") +} + +fn skill_content() -> String { + let path = skill_path(); + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())) +} + +/// Ask the binary for its `(group, subcommand)` pairs and collect +/// the subcommands for `group`. Spawning the binary keeps the +/// test honest — a port that adds a native subcommand and its +/// `NATIVE_COMMANDS` entry shows up automatically, no parallel +/// list to drift. +fn native_commands_for_group(group: &str) -> BTreeSet { + let binary = env!("CARGO_BIN_EXE_mergify"); + let output = Command::new(binary) + .arg("--list-native-commands") + .output() + .unwrap_or_else(|e| panic!("spawn {binary} --list-native-commands: {e}")); + assert!( + output.status.success(), + "mergify --list-native-commands exited {:?}\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stderr), + ); + let stdout = String::from_utf8(output.stdout).expect("stdout is UTF-8"); + stdout + .lines() + .filter_map(|line| { + let (g, sub) = line.split_once(char::is_whitespace)?; + (g == group).then(|| sub.to_string()) + }) + .collect() +} + +#[test] +fn skill_content_is_readable() { + assert!(!skill_content().is_empty(), "SKILL.md must not be empty"); +} + +#[test] +fn skill_has_valid_frontmatter() { + let content = skill_content(); + // Extract YAML frontmatter between --- markers — the same + // shape Claude Code's skill loader expects. + let re = Regex::new(r"(?s)^---\n(.+?)\n---\n").expect("frontmatter regex compiles"); + let captures = re + .captures(&content) + .expect("Skill must have YAML frontmatter"); + let yaml = captures.get(1).unwrap().as_str(); + let parsed: Value = serde_yaml_ng::from_str(yaml).expect("frontmatter is valid YAML"); + let mapping = parsed + .as_mapping() + .expect("frontmatter must be a YAML mapping"); + let name = mapping + .get(Value::from("name")) + .and_then(Value::as_str) + .expect("frontmatter must have 'name'"); + assert_eq!(name, "mergify-merge-queue"); + assert!( + mapping.get(Value::from("description")).is_some(), + "frontmatter must have 'description'", + ); +} + +#[test] +fn skill_has_required_sections() { + let content = skill_content(); + for section in REQUIRED_SECTIONS { + assert!( + content.contains(section), + "Skill is missing required section: {section}", + ); + } +} + +#[test] +fn skill_references_valid_commands() { + let content = skill_content(); + let re = Regex::new(r"mergify queue ([\w-]+)").expect("reference regex compiles"); + // BTreeSet so iteration order — and therefore which assertion + // trips first — is deterministic. Same for `available` below: + // its `Debug` output ends up in the failure message and would + // otherwise reshuffle between runs. + let referenced: BTreeSet = re + .captures_iter(&content) + .map(|c| c[1].to_string()) + .collect(); + let available = native_commands_for_group("queue"); + + for cmd in &referenced { + assert!( + available.contains(cmd), + "Skill references 'mergify queue {cmd}' but it's not a Rust-native \ + command reported by `mergify --list-native-commands`. \ + Available: {available:?}", + ); + } +} diff --git a/mergify_cli/tests/queue/__init__.py b/mergify_cli/tests/queue/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/mergify_cli/tests/queue/test_skill.py b/mergify_cli/tests/queue/test_skill.py deleted file mode 100644 index f233b0db..00000000 --- a/mergify_cli/tests/queue/test_skill.py +++ /dev/null @@ -1,118 +0,0 @@ -# -# Copyright © 2021-2026 Mergify SAS -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -from __future__ import annotations - -import pathlib -import re -import shutil -import subprocess - -import yaml - - -SKILL_PATH = ( - pathlib.Path(__file__).parents[3] / "skills" / "mergify-merge-queue" / "SKILL.md" -) - - -def _get_skill_content() -> str: - return SKILL_PATH.read_text(encoding="utf-8") - - -def _native_commands_for_group(group: str) -> set[str]: - """Ask the installed `mergify` binary which ` ` pairs - it handles natively, then return the subcommands for `group`. - - Spawning the binary keeps this test honest: the source of truth - is the binary itself, so a port that adds a native subcommand - (and its `NATIVE_COMMANDS` entry) automatically becomes visible - here. No parallel hard-coded list to drift. - """ - # Fail (don't skip) when `mergify` isn't on PATH: the wheel must - # be installed for the test to be meaningful, and silently - # skipping would let a regression in the wheel's bin scripts - # slip through unnoticed. `uv run pytest` installs the wheel - # before invoking pytest, so on every supported entry point the - # binary is present. - binary = shutil.which("mergify") - assert binary is not None, ( - "`mergify` binary not on PATH. Install the wheel first " - "(`uv run pytest` does this automatically) — running this " - "test against an uninstalled checkout would give a false pass." - ) - out = subprocess.run( - [binary, "--list-native-commands"], - check=True, - capture_output=True, - text=True, - ).stdout - pairs = (line.split(maxsplit=1) for line in out.splitlines() if line.strip()) - return { - sub - for pair in pairs - if len(pair) == 2 and pair[0] == group - for sub in [pair[1]] - } - - -def test_skill_content_is_readable() -> None: - content = _get_skill_content() - assert len(content) > 0 - - -def test_skill_has_valid_frontmatter() -> None: - content = _get_skill_content() - # Extract YAML frontmatter between --- markers - match = re.match(r"^---\n(.+?)\n---\n", content, re.DOTALL) - assert match is not None, "Skill must have YAML frontmatter" - - frontmatter = yaml.safe_load(match.group(1)) - assert isinstance(frontmatter, dict), "Frontmatter must be a YAML mapping" - assert "name" in frontmatter, "Frontmatter must have 'name'" - assert "description" in frontmatter, "Frontmatter must have 'description'" - assert frontmatter["name"] == "mergify-merge-queue" - - -REQUIRED_SECTIONS = [ - "## Commands", - "## Checking Queue Status", - "## Inspecting a PR", - "## Queue States", - "## Troubleshooting", -] - - -def test_skill_has_required_sections() -> None: - content = _get_skill_content() - for section in REQUIRED_SECTIONS: - assert section in content, f"Skill is missing required section: {section}" - - -def test_skill_references_valid_commands() -> None: - """Every `mergify queue ` reference in the skill must resolve - 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. - """ - content = _get_skill_content() - referenced = set(re.findall(r"mergify queue ([\w-]+)", content)) - available = _native_commands_for_group("queue") - - for cmd in referenced: - assert cmd in available, ( - 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)}" - ) From 331118f7a7fdd4bb07e2b05e4006d7f6f4be0ffa Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 19 May 2026 09:59:38 +0200 Subject: [PATCH 2/2] test(freeze): add live smoke test for freeze list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the URL + auth + JSON-array shape of `freeze list --json` against the real Mergify API before the upcoming Python → Rust port. The Python `list_cmd` returns the inner `scheduled_freezes` array verbatim, so the test asserts that the `--json` output parses as a JSON array — same contract we want preserved across both ends of the port. Uses `live_admin_token` because scheduled-freeze endpoints sit under the queue-management family and the CI-scoped token is rejected with 403. Group-level options (`--token` / `--api-url` / `--repository`) come before the subcommand — Click requires it on the Python side, clap accepts both orders. Co-Authored-By: Claude Opus 4.7 Change-Id: I51b653702c10e184daff5f450f1edc4f3c581433 --- func-tests/test_live_smoke.py | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/func-tests/test_live_smoke.py b/func-tests/test_live_smoke.py index 081810d8..b7f8e5d3 100644 --- a/func-tests/test_live_smoke.py +++ b/func-tests/test_live_smoke.py @@ -196,6 +196,56 @@ def test_queue_show_not_in_queue( ) +def test_freeze_list( + live_admin_token: str, + cli: typing.Callable[..., typing.Any], +) -> None: + """`GET /v1/repos/{owner}/{repo}/scheduled_freeze`. + + Uses the admin-scoped token because scheduled-freeze endpoints + sit under the queue-management family; the CI-scoped token is + rejected with 403. + + ``--json`` mode is a passthrough of the inner + ``scheduled_freezes`` array (Python's ``list_freezes`` returns + ``data["scheduled_freezes"]``, the CLI prints that verbatim). + The smoke test only checks the call succeeds and parses as a + JSON array — the contract preserved across the Python → Rust + port is the URL, the auth, and the array shape of the + ``--json`` output. + """ + import json + + # Group-level options (``--token`` / ``--api-url`` / + # ``--repository``) come BEFORE the subcommand — Click requires + # it on the Python side (options live on ``@freeze``), Rust + # accepts both via clap's ``global = true``. + result = cli( + "freeze", + "--api-url", + API_URL, + "--token", + live_admin_token, + "--repository", + REPOSITORY, + "list", + "--json", + ) + assert result.returncode == 0, ( + f"freeze list failed\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + try: + payload = json.loads(result.stdout) + except json.JSONDecodeError as exc: + pytest.fail( + f"freeze list --json emitted non-JSON output\n" + f"error: {exc}\nstdout:\n{result.stdout}", + ) + assert isinstance(payload, list), ( + f"freeze list --json must emit a JSON array\nstdout:\n{result.stdout}" + ) + + def test_ci_git_refs_fallback( cli: typing.Callable[..., typing.Any], ) -> None: