test(ios): expect the construction focus event in composer diagnostics - #14003
Merged
Merged
Conversation
mutationsEmitPrivacySafeLifecycleDiagnostics() assumed MobileShellComposite init set the selection without running selectedTerminalID's didSet. Since #10539, init leaves the selection nil and lets syncSelectedTerminalForWorkspace() pick the first terminal, so every composite built with a diagnostic log records surfaceFocused (182) first. The test saw [182, 641, 642, 643] and failed. The product behavior is intended: a default-selected terminal records surfaceFocused like any other derived selection. Expect that event ahead of the three attachment events rather than filtering it out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe attachment diagnostics test now accounts for the construction-time ChangesAttachment lifecycle diagnostics
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to The test accounts for the construction-time focus event before checking attachment diagnostics. No concrete merge-blocking issue is indicated; the CI Xcode 26.6 run remains unverified. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 23, 2026
ccdbf30 fix(ios): let a legacy untagged Mac row take pushed presence routes (manaflow-ai#14012) 6332063 test(ios): give pool alias fixtures one build identity (manaflow-ai#14004) 82d49a3 fix(ios): keep an offline sibling build out of secondary aggregation (manaflow-ai#14005) 19a6ac1 test: read errno before the assertion that can overwrite it (manaflow-ai#13957) ef98b5e ci: name the macOS 26 runner variables after machines, not lanes (manaflow-ai#14009) 9697411 test(ios): expect the construction focus event in composer diagnostics (manaflow-ai#14003) ad7f9fe ci: run app-host unit tests for a cmuxTests/ diff without a label (manaflow-ai#14017) # Conflicts: # .github/workflows/ci-health-report.yml # .github/workflows/ci-macos.yml # .github/workflows/ci.yml # .github/workflows/nightly.yml
Collaborator
Author
|
Checked under CI's Xcode 26.6: all four fixes (#14003, #14004, #14005, #14012) merged onto main
With the fixes, the suite finished all 1303 tests in 386 s. Main hung and was killed at the 25-minute step timeout, so its job shows "cancelled". 21 tests still fail with the fixes. 4 of them also fail on main here: — Nyan g1 🗝️ |
This was referenced Sep 23, 2026
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.
ComposerPendingAttachmentTests.mutationsEmitPrivacySafeLifecycleDiagnostics()fails on main every time. It expects the diagnostic log to hold exactly the three attachment events,[641, 642, 643], but it gets[182, 641, 642, 643]. 182 issurfaceFocused.The test's helper assumes
MobileShellComposite.initsets the selection without runningselectedTerminalID'sdidSet. That stopped being true in #10539 (Aug 21). Since then, init leaves the selection nil and callssyncSelectedTerminalForWorkspace(), which selectsterm-aand recordssurfaceFocused. The test was added in #9977 nine days earlier, and the dispatch-only iOS lane never caught the change.I think the product is right and the test is stale. A terminal selected by default records
surfaceFocusedthe same way every other derived selection does. So the test now expects that event first, followed by the three attachment events. It does not filter the event out, and the attachment assertions (kinds, failure codes, non-nil surfaces, no leaked IDs or bytes) are unchanged.Testing
db5d212d86, upstream main, this test fails with the same[182, 641, 642, 643]as the CI log from fork run 35876818909.ComposerPendingAttachmentTests(run withswift test --no-paralleltogether with the pool and alt-screen suites).— Thimble g1 🔆
Run: run_ios_shell_mobile_tests_20260923_e464e5ba
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes
ComposerPendingAttachmentTests.mutationsEmitPrivacySafeLifecycleDiagnostics()so it expects thesurfaceFocusedevent recorded when the composite selects the default terminal on init.The test previously assumed
initset the selection without runningdidSet; since the selection is now left nil and resolved by the workspace synchronizer, the diagnostics log starts withsurfaceFocused. The test now expects that event first, followed by the three attachment events, and keeps all existing attachment assertions unchanged.Written for commit 97270ad. Summary will update on new commits.
Summary by CodeRabbit