List the wrong release first in release lookup specs - #764
Merged
AliSoftware merged 1 commit intoAug 3, 2026
Merged
Conversation
Collaborator
Generated by 🚫 Danger |
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens the regression coverage around GitHub Release lookup in #upload_release_assets by making the specs adversarial to API ordering, ensuring the helper deterministically selects the correct release rather than “whatever comes first”.
Changes:
- Reordered the stubbed
client.releasesreturn value in two#upload_release_assetsspecs so the wrong candidate is listed first. - Added clarifying spec comments explaining the intent (guarding against implementations that rely on API ordering).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5 tasks
AliSoftware
approved these changes
Aug 3, 2026
Both `#upload_release_assets` specs stubbed `client.releases` with the release they expect the helper to pick already in first position, so they passed against a lookup that just takes the first match and never exercised the deterministic selection they were added for. Putting the wrong candidate first makes them fail against such a lookup: verified by reverting `find_release` to `matches.first` over an unsorted list, which turns both red. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
AliSoftware
force-pushed
the
mokagio/pr-763-order-independent-specs
branch
from
August 3, 2026 06:22
93c7349 to
eb10938
Compare
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.
What does it do?
Addresses a false positive risk in the #763 tests highlighted by Copilot #763 (review)
To verify the false positive, I got Claude to open #765, which reverts the code to the pre-#763 lookup yet has green tests.
The two
#upload_release_assetsspecs added there stubclient.releaseswith the release they expect the helper to pick already in first position. That makes them pass against a lookup that simply takes the first match — so neither actually pins the deterministic selection they were added for.Reordering each stub so the wrong candidate comes first turns them into real regression tests. Verified locally: reverting
find_releasetomatches.firstover an unsorted list turns both red, and they go green again with the implementation from #763. The#publish_releasespecs already had adversarial ordering and need no change.#766 verifies the test fix in this PR. Same revert to pre-#763 lookup, but CI is red.
Checklist before requesting a review
bundle exec rubocopto test for code style violations and recommendations.specs/*_spec.rb) if applicable.bundle exec rspecto run the whole test suite and ensure all your tests pass.CHANGELOG.mdfile to describe your changes under the appropriate existing###subsection of the existing## Trunksection. — n/a, test-only change; Publish the most recent GitHub Release when several share a name #763 carries the entry.MIGRATION.mdfile to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider. — n/a.Posted by Claude Code (Opus 5) on behalf of @mokagio with approval.