docs(live-tests): drop the mirror claims for deleted Python files - #1799
Conversation
Member
Author
|
This pull request is part of a Mergify stack:
|
Contributor
Merge Protections🔴 3 of 7 protections blocking · waiting on 👀 reviews and ⛓️ dependency
🔴 ⛓️ Depends-On RequirementsWaiting for
This rule is failing.Requirement based on the presence of
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is documentation-only and the remaining feedback is limited to minor clarity/wording nits.
Pull request overview
Removes stale “mirrors the Python version” claims from the live smoke test module header and helper docs now that the Rust port is complete, while preserving useful provenance notes about wire-contract decisions.
Changes:
- Updates the module-level docs in
live_smoke.rsto remove references to deleted Python files/fixtures. - Rewords several helper doc comments to avoid implying a 1:1 Python mirror and instead document the current invariants (credential grouping).
File summaries
| File | Description |
|---|---|
| crates/mergify-cli/tests/live_smoke.rs | Refreshes documentation to remove stale Python-mirroring assertions and clarify the current test grouping invariants. |
Review details
Suppressed comments (2)
crates/mergify-cli/tests/live_smoke.rs:89
- The doc comment uses a trailing semicolon and a lowercase continuation line, which reads like a sentence fragment. Consider making these two standalone sentences for clarity.
/// Closes stdin so an accidental interactive prompt fails fast
/// instead of blocking;
/// caps wall-clock at [`CLI_TIMEOUT`] so a pathological hang
/// doesn't drag the CI matrix down with it.
crates/mergify-cli/tests/live_smoke.rs:192
- "the key scoped to what a CI job does" is a bit unclear; this comment can be tighter and consistent with the later "CI-scoped" wording.
/// Look up `LIVE_TEST_MERGIFY_TOKEN_CI`, the key scoped to what a
/// CI job does. Empty / unset = skip the test (early return with
/// `SKIP:` printed to stderr so the cargo test log shows what was
/// skipped).
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The module header promised this file "deliberately mirrors the Python version 1:1 ... so the port can't drift the contract by accident", and named `func-tests/test_live_smoke.py` and `func-tests/conftest.py` as what it mirrors. Both were deleted when the port completed. Four helpers made the same promise individually (`Mirrors conftest.py::cli`, `Mirrors Python subprocess.run`, `Matches Python result.stdout + result.stderr`, and `live_token`'s "Mirrors Python `live_token` fixture"). AGENTS.md: "There is no Python: the port is complete ... If you find a doc, comment, or rule mentioning [it], it is stale — fix it." The harm is concrete rather than cosmetic: a maintainer auditing which secret each test needs goes looking for the fixture the header says pins that mapping, and there isn't one. The header now states the invariant that actually holds — tests are grouped by credential under a banner, and a test belongs under the banner matching its helper — which is the thing a reader needs and the thing the previous commit had to correct. It also spells `LIVE_TEST_MERGIFY_TOKEN_ADMIN` out rather than abbreviating it to `_ADMIN`, so grepping for either secret finds this file. Left alone: the in-body notes recording which wire contracts were preserved across the Python → Rust port. Those are provenance for why a contract is shaped the way it is, not claims about a file that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn Change-Id: Idd9e1e28bfc50b8b71d07c5938b4fa89af2745ff
jd
force-pushed
the
devs/jd/fix/live-tests-tests-show-admin-token/drop-mirror-claims-deleted-python-files--dd9e1e28
branch
from
September 4, 2026 15:38
5524182 to
9b118fc
Compare
Member
Author
Revision history
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The module header promised this file "deliberately mirrors the
Python version 1:1 ... so the port can't drift the contract by
accident", and named
func-tests/test_live_smoke.pyandfunc-tests/conftest.pyas what it mirrors. Both were deleted whenthe port completed. Four helpers made the same promise individually
(
Mirrors conftest.py::cli,Mirrors Python subprocess.run,Matches Python result.stdout + result.stderr, andlive_token's"Mirrors Python
live_tokenfixture").AGENTS.md: "There is no Python: the port is complete ... If you
find a doc, comment, or rule mentioning [it], it is stale — fix
it." The harm is concrete rather than cosmetic: a maintainer
auditing which secret each test needs goes looking for the fixture
the header says pins that mapping, and there isn't one.
The header now states the invariant that actually holds — tests are
grouped by credential under a banner, and a test belongs under the
banner matching its helper — which is the thing a reader needs and
the thing the previous commit had to correct. It also spells
LIVE_TEST_MERGIFY_TOKEN_ADMINout rather than abbreviating it to_ADMIN, so grepping for either secret finds this file.Left alone: the in-body notes recording which wire contracts were
preserved across the Python → Rust port. Those are provenance for
why a contract is shaped the way it is, not claims about a file
that no longer exists.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn