Split the monolithic Mac test suite - #144
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@macroscope-app review |
|
@codex review |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR splits the Mac XCTest suite into domain-focused test files, adds shared test-support helpers, updates the Xcode test target to compile the new files, and documents the new test layout in the changelog. ChangesMac XCTest suite reorganization
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
Review is in progress. Results will be posted as check runs when complete. |
ApprovabilityVerdict: Needs human review Unable to check for correctness in 541a9b8. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CodexBarMacTests/AppAndCredentialWorkflowTests.swift`:
- Around line 178-189: Add kSecUseDataProtectionKeychain set to true to the
SecItemAdd attributes in the test and to the keychain attributes used by
KeychainService.saveSecret. Preserve the existing ThisDeviceOnly accessibility
setting and ensure both insertion paths explicitly target the Data Protection
keychain.
In `@CodexBarMacTests/CredentialTestSupport.swift`:
- Line 99: Add an explicit empty deinit to TransactionalReplacementSecretStore
and the six other test-double classes at the referenced declarations, matching
the existing deinit declarations in CredentialMutationSecretStore and
FailingCredentialCompensationSecretStore to satisfy required_deinit.
- Around line 232-254: Make SelectiveDeletionSecretStore’s failingAccounts state
thread-safe to justify its `@unchecked` Sendable conformance, following the
NSLock-protected storage pattern used by the other test doubles. Guard both
reads in deleteSecret(account:) and mutations of failingAccounts, or remove
`@unchecked` Sendable if synchronization is not needed.
In `@CodexBarMacTests/OAuthTestSupport.swift`:
- Around line 87-110: Protect presentedAuthorizationURL with a lock-guarded box
shared by the main-actor authorization callback and MockURLProtocol.handler.
Replace direct assignment and read access with the box’s synchronized setter and
getter, preserving the existing XCTUnwrap and request-inspection flow.
In `@CodexBarMacTests/RefreshTestSupport.swift`:
- Around line 70-74: Update waitUntilStarted to use the same bounded polling
pattern as waitUntilCancellationObserved, returning a Boolean that indicates
whether started became true before the limit; preserve yielding between checks
so callers can assert the result with XCTAssertTrue instead of hanging
indefinitely.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 335e8d4a-8442-4b06-b645-1182cb283e59
📒 Files selected for processing (19)
CHANGELOG.mdCodexBarMac.xcodeproj/project.pbxprojCodexBarMacTests/APIKeyAndCursorProviderTests.swiftCodexBarMacTests/AppAndCredentialWorkflowTests.swiftCodexBarMacTests/ClaudeProviderTests.swiftCodexBarMacTests/CodexBarMacTests.swiftCodexBarMacTests/CodexProviderTests.swiftCodexBarMacTests/ConfigurationPersistenceTests.swiftCodexBarMacTests/CopilotProviderTests.swiftCodexBarMacTests/CredentialTestSupport.swiftCodexBarMacTests/DashboardTests.swiftCodexBarMacTests/GeminiProviderTests.swiftCodexBarMacTests/NetworkTestSupport.swiftCodexBarMacTests/OAuthTestSupport.swiftCodexBarMacTests/OpenCodeProviderTests.swiftCodexBarMacTests/RefreshTestSupport.swiftCodexBarMacTests/UsageAlertTests.swiftCodexBarMacTests/UsageHistoryTests.swiftCodexBarMacTests/UsageRefreshTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
|
@codex review |
|
@macroscope-app review |
|
@coderabbitai review |
|
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
@macroscope-app review |
|
@coderabbitai review |
|
|
Review triggered. Results will be posted as check runs when complete. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #142
Summary
Verification
./test.sh— passed; 344 tests, 0 failures (plus release/run smoke checks)xcodebuild ... build— passed with project-warnings-as-errorsenabledgit diff --check— passedxcodebuild analyze— passedResidual risk
Summary by cubic
Split the 12,850-line monolithic Mac XCTest file into 12 domain-focused test suites with small, shared test support. Test coverage (344 tests) and assertions are preserved.
Refactors
CodexBarMacTests.swiftwith 12 focused test classes (providers, dashboard, usage, config, workflows).MockURLProtocolrequest parsing, sequenced usage providers).Migration
Written for commit 541a9b8. Summary will update on new commits.
Note
Split the monolithic Mac XCTest suite into domain-focused test classes
Replaces the single CodexBarMacTests.swift file with multiple domain-scoped test files covering usage refresh, alerts, history, dashboard ordering, and provider-specific behavior (Claude, Codex, Copilot, Gemini, OpenCode, OpenRouter).
Macroscope summarized 541a9b8.