test(live): cover the quarantine add/remove round-trip - #1801
Conversation
MRGFY-9001 dropped `ci_application_key` from `POST` and `DELETE` on
`/ci/{owner}/repositories/{repo}/quarantines` at the same time it
dropped it from `search/tests`. Only `search/tests` had a live
test, so only `search/tests` turned red; `mergify tests quarantines
add` and `remove` broke for every `ci` key with no signal at all.
The first two commits of this stack documented that. This closes
it.
The 20-odd wiremock tests in `tests_quarantine.rs` cannot cover
this: a mock server never authenticates, so no scope change is
visible to them. It has to be the live suite or nothing.
Shape follows `freeze_create_update_delete_roundtrip`, the existing
create-and-clean-up test in this file: a `Drop` guard removes the
entry so a failed assertion mid-test still leaves the canary
repository clean, and it warns rather than panics, because
panicking in `Drop` during an unwind aborts the process and buries
the assertion message that explains the failure. The guard is
registered before the response is parsed — an unparseable body
still means the row exists server-side.
Two details specific to quarantines:
- The quarantined name is `__mergify_cli_smoke_quarantine_<rand>__`
and matches nothing any real suite reports, so even a completely
skipped cleanup cannot suppress a genuine failure on the canary
repository. A leaked row is inert.
- Removal goes by name rather than by the id `add` returned, so one
run covers the list read that resolves the name *and* the delete.
Cleanup then expects exit 6 ("not quarantined") on the happy
path, since the body already removed the row.
The 8-char entropy the freeze test generated inline is now
`unique_suffix()`, shared by both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn
Change-Id: I20ed0098fe86acd633ad6864eed035f6e5f47dd1
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. |
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
1 participant
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.
MRGFY-9001 dropped
ci_application_keyfromPOSTandDELETEon/ci/{owner}/repositories/{repo}/quarantinesat the same time itdropped it from
search/tests. Onlysearch/testshad a livetest, so only
search/teststurned red;mergify tests quarantines addandremovebroke for everycikey with no signal at all.The first two commits of this stack documented that. This closes
it.
The 20-odd wiremock tests in
tests_quarantine.rscannot coverthis: a mock server never authenticates, so no scope change is
visible to them. It has to be the live suite or nothing.
Shape follows
freeze_create_update_delete_roundtrip, the existingcreate-and-clean-up test in this file: a
Dropguard removes theentry so a failed assertion mid-test still leaves the canary
repository clean, and it warns rather than panics, because
panicking in
Dropduring an unwind aborts the process and buriesthe assertion message that explains the failure. The guard is
registered before the response is parsed — an unparseable body
still means the row exists server-side.
Two details specific to quarantines:
__mergify_cli_smoke_quarantine_<rand>__and matches nothing any real suite reports, so even a completely
skipped cleanup cannot suppress a genuine failure on the canary
repository. A leaked row is inert.
addreturned, so onerun covers the list read that resolves the name and the delete.
Cleanup then expects exit 6 ("not quarantined") on the happy
path, since the body already removed the row.
The 8-char entropy the freeze test generated inline is now
unique_suffix(), shared by both.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn