feat(config): add mergify config simulate command#1018
Conversation
|
This pull request is part of a stack:
|
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 ⛓️ Depends-On RequirementsThis rule is failing.Requirement based on the presence of
🔴 👀 Review RequirementsThis rule is failing.
🔴 🔎 ReviewsThis rule is failing.
🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Pull request overview
Adds a new mergify config simulate subcommand that posts a local .mergify.yml to the Mergify PR simulator API for a given PR URL and prints the simulator’s title/summary.
Changes:
- Add
config simulate <PR_URL>command with--config,--token, and--api-urloptions. - Add async simulator API helper (
simulate_pr) plus a raw config reader. - Extend config CLI tests to cover simulate success + basic argument/file validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
mergify_cli/config/cli.py |
Introduces the simulate subcommand and PR URL parsing logic. |
mergify_cli/config/validate.py |
Adds raw config reading and the async simulator API call helper/result type. |
mergify_cli/tests/config/test_validate.py |
Adds tests for the new simulate command and refactors schema URL usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response = await client.post( | ||
| f"/v1/repos/{repository}/pulls/{pull_number}/simulator", | ||
| json={"mergify_yml": mergify_yml}, | ||
| ) |
There was a problem hiding this comment.
simulate_pr() never calls response.raise_for_status(). It currently relies on the caller to have an event hook that raises on HTTP >= 400, which makes the helper easy to misuse (and inconsistent with fetch_schema()). Call raise_for_status() before decoding JSON so errors are handled predictably regardless of client configuration.
| ) | |
| ) | |
| response.raise_for_status() |
|
|
||
| _SCHEMA_URL = "https://docs.mergify.com/mergify-configuration-schema.json" | ||
| _PR_URL = "https://github.com/owner/repo/pull/42" | ||
| _SIMULATOR_URL = "https://api.mergify.com/v1/repos/owner/repo/pulls/42/simulator" |
There was a problem hiding this comment.
_SIMULATOR_URL is introduced but never used in this test module. Either remove it or use it to build the respx route in test_simulate_pr() so the constant serves a purpose.
| _SIMULATOR_URL = "https://api.mergify.com/v1/repos/owner/repo/pulls/42/simulator" |
| assert "The configuration is valid" in result.output | ||
| assert "No actions will be triggered" in result.output | ||
|
|
||
|
|
There was a problem hiding this comment.
The new simulate command tests cover success and local validation errors, but there is no coverage for simulator API failures (e.g., HTTP 4xx/5xx or network errors). Adding a test that mocks a non-2xx simulator response (and asserts a clean, non-traceback CLI error with non-zero exit) would help lock down the intended UX.
| def test_simulate_api_failure(tmp_path: pathlib.Path) -> None: | |
| config_path = _write_config(tmp_path, "pull_request_rules: []\n") | |
| with respx.mock(base_url="https://api.mergify.com") as rsp: | |
| rsp.post("/v1/repos/owner/repo/pulls/42/simulator").mock( | |
| return_value=Response( | |
| 500, | |
| json={ | |
| "message": "internal error", | |
| }, | |
| ), | |
| ) | |
| result = CliRunner().invoke( | |
| config, | |
| [ | |
| "simulate", | |
| _PR_URL, | |
| "--config", | |
| config_path, | |
| "--token", | |
| "test-token", | |
| ], | |
| ) | |
| assert result.exit_code != 0 | |
| assert "Traceback" not in result.output |
| result = await config_validate.simulate_pr( | ||
| client, | ||
| repository, | ||
| pull_number, | ||
| mergify_yml, | ||
| ) |
There was a problem hiding this comment.
simulate() does not catch network-level httpx exceptions (e.g., DNS/connect/timeout). Those will bypass cli.main()’s HTTPStatusError handler and surface as an uncaught traceback. Mirror validate() by catching httpx.HTTPError around the simulator call and raising a click.ClickException with a user-friendly message (keeping HTTPStatusError behavior intact).
| result = await config_validate.simulate_pr( | |
| client, | |
| repository, | |
| pull_number, | |
| mergify_yml, | |
| ) | |
| try: | |
| result = await config_validate.simulate_pr( | |
| client, | |
| repository, | |
| pull_number, | |
| mergify_yml, | |
| ) | |
| except httpx.HTTPStatusError: | |
| # Let HTTPStatusError bubble up so it can be handled by the global handler. | |
| raise | |
| except httpx.HTTPError as exc: | |
| msg = f"Failed to contact Mergify API at {api_url}: {exc}" | |
| raise click.ClickException(msg) from exc |
| _PR_URL_RE = re.compile( | ||
| r"https?://[^/]+/(?P<owner>[^/]+)/(?P<repo>[^/]+)/pull/(?P<number>\d+)", | ||
| ) | ||
|
|
||
|
|
||
| def _parse_pr_url(url: str) -> tuple[str, int]: | ||
| m = _PR_URL_RE.match(url) | ||
| if not m: | ||
| msg = f"Invalid pull request URL: {url}" | ||
| raise click.ClickException(msg) | ||
| return f"{m.group('owner')}/{m.group('repo')}", int(m.group("number")) |
There was a problem hiding this comment.
_PR_URL_RE is not anchored to the end of the string. As written, inputs like https://github.com/org/repo/pull/42foo will be accepted and parsed as PR #42 instead of being rejected. Consider switching to fullmatch() or updating the regex to require an end delimiter (e.g., end-of-string or /, ?, #).
216d896 to
d119bfc
Compare
68e009f to
01b46c4
Compare
Add a new `config validate` subcommand that validates Mergify configuration files against the official JSON schema. Supports auto-detection of config file location and explicit path via --config. Closes #1010 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Change-Id: Ib7a8075cd92a62a2a7d761a4eab5e309c914dd21 Claude-Session-Id: 35f710ae-3790-4349-b55b-82247979f4a8
d119bfc to
d391491
Compare
Add a `simulate` subcommand that sends the local Mergify configuration to the Mergify PR simulator API, showing what actions would be triggered for a given pull request. Usage: mergify config simulate <PR_URL> [--config path] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Change-Id: I5246a6407b4c3a7aa38dfa8b963bae3bf016a8e1 Claude-Session-Id: 35f710ae-3790-4349-b55b-82247979f4a8
d391491 to
a6569f3
Compare
a0412dc to
be4ceb9
Compare
🧪 CI InsightsHere's what we observed from your CI run for a6569f3. 🟢 All jobs passed!But CI Insights is watching 👀 |
Add a
simulatesubcommand that sends the local Mergify configurationto the Mergify PR simulator API, showing what actions would be triggered
for a given pull request.
Usage: mergify config simulate <PR_URL> [--config path]
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Depends-On: #1016