fix gen5 pairing test pinning a stale present() signature - #302
Merged
abdulsaheel merged 2 commits intoAug 28, 2026
Merged
Conversation
The test read AccessorySetup.swift as source text and asserted the exact lines `present(items, allowGen4Retry: true)` and `self.present([items[0]], allowGen4Retry: false)`. Both calls gained a `known:` parameter when present() started forwarding the pre-picker accessory list, so the full-line match no longer held and the test failed deterministically on main. Only the first was ever reported, since it aborts the test body before the second runs. Match by shape instead: the invariant is that the widened list is presented WITH the Gen 4 retry armed and the single-item retry targets items[0] with the retry disarmed, whatever else rides along in between. Verified the regexes still fail on a disarmed widened list, a retry target that is not items[0], and a retry that re-arms itself. Every other pinned literal in the file was re-checked against its source and still holds.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the Gen 5 pairing source-text test to tolerate added named arguments to Swift File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="test/gen5_pairing_filter_test.dart" line_range="179" />
<code_context>
+ // the argument the retry hinges on, whatever else rides along.
+ expect(
+ swift,
+ matches(RegExp(r'present\(\s*items\s*,[^)]*allowGen4Retry:\s*true')),
+ reason: 'the widened list must be presented WITH the Gen 4 retry armed',
+ );
</code_context>
<issue_to_address>
**issue (testing):** The assertions match only the `true` or `false` prefix of the `allowGen4Retry` expression, so `allowGen4Retry: true && false` passes the first assertion while leaving retry disabled, and `allowGen4Retry: false || true` passes the second while re-arming the retry.
**Triggers:** When the Swift argument is changed from a boolean literal to a compound boolean expression.
**Suggested fix:** Require the boolean value to be a complete argument token, such as by matching `true` or `false` followed by optional whitespace and the closing parenthesis or a comma.
</issue_to_address>
### Comment 2
<location path="test/gen5_pairing_filter_test.dart" line_range="179" />
<code_context>
+ // the argument the retry hinges on, whatever else rides along.
+ expect(
+ swift,
+ matches(RegExp(r'present\(\s*items\s*,[^)]*allowGen4Retry:\s*true')),
+ reason: 'the widened list must be presented WITH the Gen 4 retry armed',
+ );
</code_context>
<issue_to_address>
**issue (testing):** The regexes fail when an intervening argument contains a parenthesized expression, because `[^)]*` stops at that inner closing parenthesis even though the outer `present` call has the required retry setting.
**Triggers:** When `present` gains an intervening argument such as `known: accessories.map((item) => item.id)` or another function call.
**Suggested fix:** Use a parser or a regex that explicitly supports the expected argument syntax instead of assuming intervening arguments contain no parentheses.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: test/gen5_pairing_filter_test.dart:179, test/gen5_pairing_filter_test.dart:179
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
main fixed the same failure at f268b5e by re-pinning the new exact lines (`present(items, known: known, allowGen4Retry: true)`). That is the same brittle construct that broke in the first place: the next parameter added to present() breaks it again. Resolved in favour of this branch's shape-matching assertion, which tolerates the `known:` parameter and any future one. Also addresses two findings from the PR bot, both of which were correct: - The value was matched as a prefix, so `allowGen4Retry: true && false` would have passed while leaving the retry disabled. It is now read as a complete token and a compound expression matches nothing. - The `[^)]*` gap could not cross a parenthesis, so an intervening argument containing a call (`known: ids.map((i) => i.uuid)`) would have failed the assertion spuriously — the exact drift this PR exists to stop. Arguments are now extracted with balanced parentheses. Verified against mutated copies of the Swift: a disarmed widened list, a retry target that is not items[0], a retry that re-arms itself, and both compound-boolean forms all fail; a nested-paren argument and an extra trailing parameter both still pass.
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
2 participants
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.
test/gen5_pairing_filter_test.dartfails deterministically onmain(verified at 979b20d, in isolation).What broke
The test reads
ios/Runner/AccessorySetup.swiftas source text and pinned two exact lines:Both calls gained a
known:parameter whenpresentstarted forwarding the pre-picker accessory list, so neither literal matches any more:Both had drifted, not just the one in the failure output — the first
expectaborts the test body, so the second was never reached.The fix
Match by shape rather than by full line. The invariant the test actually guards is that the widened list is presented with the Gen 4 retry armed, and that the single-item retry targets
items[0]with the retry disarmed.[^)]*absorbs any intervening arguments, and is deliberately paren-free so it cannot run past the end of the call.Checked that this is not merely permissive — against mutated copies of the Swift:
allowGen4Retry: falseitems[1]allowGen4Retry:Scope
Every other pinned literal in the file was re-checked against its source and still holds:
whoopMemberUUID16,CBUUID(string: AccessorySetup.whoopMemberUUID16),bluetoothNameSubstring, the fourretryInFlightpins, theitems-orderingindexOfpair, and theInfo.plist/ble_engine.dartliterals. The other 19 tests that read source files as text were swept for the same failure mode; none pin a call signature with named parameters, so there is no cleanup backlog behind this. No production code is touched.Verification
flutter test— 3165 passed, 0 failed, 423 skipped.Summary by Sourcery
Harden the Gen 5 pairing test’s source-level assertions against future
presentcall signature changes.Bug Fixes:
presentcall signature changes.Enhancements:
Tests: