Repository navigation
fix(test): accept a losing owner that retires the replaced token - #3323
Merged
Merged
Conversation
Two owners race one maintenance pass over a family whose rejected token is still current. The winner claims, issues and commits the replacement, and that commit moves the rejected token into retirement custody. A losing pass that reaches its retirement step after the commit revokes that token and reports `retired`; one that reaches it earlier reports `idle` and the next pass retires it. Both are correct, but the test only accepted `idle`. Add a shared `raceMaintenance` helper that asserts exactly one owner commits and returns the losers, use it in both prepared GitHub race tests, and in the token-service case accept `idle` or `retired` while counting the rejected token's revocations: one only when the loser retired it, exactly one retirement by the next pass, and none replayed by later passes.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
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.
Problem
postgres-prepared-token-service.test.ts"rejected fresh GitHub material recovers once across separate owners without replay" failed on main (PostgreSQL Platform, run 39972061000) atTwo owners run one maintenance pass at once over a family whose current token was just rejected. A pass is discover, then retire, then prepare, and it reports prepare's result, or the retire step's result when prepare claimed nothing. The winner claims, issues the replacement and commits it, and that commit moves the rejected token into retirement custody. The loser claims nothing, so it reports its own retire step:
idlewhen its retire step ran before the winner's commit (the next pass then retires the rejected token);retiredwhen its retire step ran after that commit and revoked the rejected token.Both are correct; retirement is shared by every owner and is never replayed. The test only accepted
idle. Delaying the loser by about 30 ms or more is enough to produceretired, so a slow runner hits it. The failing test took 510 ms; nothing hung. Since the test landed (#2487), this is 1 failure in about 230 completed PostgreSQL Platform jobs.This is not a product bug: the loser never claims the family, never issues, and the token it retires is the superseded, rejected one.
Change
Test only.
raceMaintenance(owners, signal)intests/helpers/github-metadata-exchange.mjs: runs one pass on each owner at once, asserts exactly one commits, and returns the sorted losing results. Both prepared GitHub race tests use it.postgres-prepared-github-lifecycle.test.mjs(cold family, nothing to retire) still requires the loser to beidle.postgres-prepared-token-service.test.tsacceptsidleorretiredfor the loser and checks retirement directly, by counting the provider revocations of the rejected token: one before the race (the test's own), a second right after the race only if the loser reportedretired, exactly two after the nextreadypass, and still two at the end. The new final count also catches a retirement replay that the old test missed. Before the test revokes the replacement itself, it checks that no maintenance pass revoked it.Verification
idleandretiredlosers.tsc --build --force, eslint and prettier are clean on the touched files.Risks
Low, test only. The race test is a bit more lenient about the losing result, but the invariants it protects are now asserted directly: exactly one commit, one new issue, and exactly one retirement of the rejected token with no replay.