Skip to content

Make terminal dock duration test deterministic - #8940

Merged
1 commit merged into
mainfrom
task-terminal-dock-duration-determinism
Jul 26, 2026
Merged

1 commit merged into
mainfrom
task-terminal-dock-duration-determinism

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • compare the synthesized animation plan against its deterministic expected value
  • remove the one-sided duration tolerance flagged by the determinism gate

Testing

  • swift test --filter TerminalDockKeyboardTransitionPlannerTests
  • python3 scripts/check-test-determinism.py --strict
  • bash tests/test_ci_self_hosted_guard.sh

Context


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Make the terminal dock duration test deterministic by computing the expected duration from inputs and asserting the plan equals an exact animate duration, instead of using a tolerance. Fixes the determinism gate failure on main and stabilizes CI.

Written for commit 9bb1151. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated animation transition tests to calculate expected durations from input values.
    • Strengthened assertions for synthesized zero-duration transitions.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f4db2841-93cb-4543-8971-ddf22a00dda4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The zeroDurationSynthesizesDuration test now calculates the expected synthesized duration from its input occupancies and lastTransitionDuration, then directly asserts the planner returns the matching .animate(duration:) plan.

Changes

Keyboard transition planner validation

Layer / File(s) Summary
Duration synthesis assertion
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalDockKeyboardTransitionPlannerTests.swift
The test derives the expected duration from explicit occupancy inputs and directly compares it with the planner’s .animate(duration:) result.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change to the terminal dock duration test.
Description check ✅ Passed The description covers summary and testing well, though it omits the demo video, review trigger, and checklist sections from the template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed Only a test file changed; no production Swift actor-isolation declarations were introduced or worsened.
Cmux Swift Blocking Runtime ✅ Passed The changed Swift code is test-only and contains no semaphores, sleeps, sync waits, locks, or delayed dispatch; the rule allows deterministic test scaffolding.
Cmux Browser Automation Off-Main ✅ Passed The commit changes only a terminal dock test file; no browser.* routing, mainActor, or worker-router code was touched.
Cmux Expensive Synchronous Load ✅ Passed Diff only changes a test file; no production Swift path adds synchronous agent-history loading or interactive main-actor work.
Cmux Cache Substitution Correctness ✅ Passed Only a Swift test changed; it computes an expected duration and asserts it, with no cache, persistence, history, undo, or snapshot substitution.
Cmux No Hacky Sleeps ✅ Passed The only change is a Swift test under Tests, and it removes a tolerance check without adding any sleeps, timers, polling, or wall-clock waits.
Cmux Algorithmic Complexity ✅ Passed Only a test file changed; the algorithmic-complexity rule explicitly exempts test-only scaffolding, and no production hot-path code was introduced.
Cmux Swift Concurrency ✅ Passed HEAD only changes a test file, replacing a tolerance check with a deterministic equality assertion; no DispatchQueue, Task, Combine, or completion-handler APIs were added.
Cmux Swift @Concurrent ✅ Passed Touched Swift test file has no async/concurrent annotations, and the PR diff shows no Swift concurrency changes that would violate the rule.
Cmux Swift Package Boundaries ✅ Passed Only a test file under Tests/ changed; no production Swift target or package-boundary shift was introduced.
Cmux Swiftpm Lockfiles ✅ Passed Only a test file changed; no cmux-owned .gitignore, Package.swift, Package.resolved, or Xcode project reference files were touched.
Cmux Swift Logging ✅ Passed Only a test file changed, and it adds no print/debugPrint/dump/NSLog or ad hoc logging; tests are explicitly allowed.
Cmux User-Facing Error Privacy ✅ Passed Only a test file changed; the privacy rule allows tests, and no user-facing error text or sensitive details were added.
Cmux Full Internationalization ✅ Passed Only a test file changed; no user-facing Swift text, catalogs, web messages, or locale entries were touched.
Cmux Swiftui State Layout ✅ Passed Only a unit test changed; no SwiftUI view/state/layout code was introduced or modified, so the SwiftUI state-layout rule is not implicated.
Cmux Architecture Rethink ✅ Passed The diff only refactors a unit test to assert a deterministic planner result; no sleeps, observers, extra owners, or lifecycle wiring were introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only changes a Swift test file; no NSWindow/NSPanel/NSWindowController/WindowGroup or cmuxAuxiliaryWindowIdentifiers changes were introduced.
Cmux Source Artifacts ✅ Passed The only changed path is a hand-written Swift test file, which the rule explicitly allows; no artifact, cache, or build-output paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Only a Tests/ Swift file changed; no production Sources/ diff introduced a test/debug seam.
Cmux No Ambient Global State ✅ Passed Changed file is a test suite only; it adds no top-level funcs/vars/singletons, just a private helper inside the struct.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-terminal-dock-duration-determinism

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes the terminal dock duration test deterministic by deriving the expected duration from named inputs and comparing the complete synthesized animation plan with exact equality.

Confidence Score: 5/5

The PR appears safe to merge with no actionable issues identified.

The change is confined to test code and replaces a one-sided floating-point tolerance with a deterministic expected plan derived using the same input types and arithmetic order.

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalDockKeyboardTransitionPlannerTests.swift Refactors one test to use explicit input constants and exact plan equality without changing production behavior.

Reviews (1): Last reviewed commit: "test: compare dock duration structurally" | Re-trigger Greptile

@lawrencecchen lawrencecchen closed this pull request by merging all changes into main in 7c82622 Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant