Skip to content

[PM-35680] chore: Allow Sourcery to generate BitwardenSdk mocks#2581

Merged
matt-livefront merged 5 commits into
mainfrom
matt/PM-35680-sourcery-sdk-mocks
Apr 30, 2026
Merged

[PM-35680] chore: Allow Sourcery to generate BitwardenSdk mocks#2581
matt-livefront merged 5 commits into
mainfrom
matt/PM-35680-sourcery-sdk-mocks

Conversation

@matt-livefront
Copy link
Copy Markdown
Collaborator

@matt-livefront matt-livefront commented Apr 24, 2026

🎟️ Tracking

PM-35680

📔 Objective

Sourcery cannot annotate types defined in external modules, so mocks for BitwardenSdk types had to be written and maintained by hand. This PR eliminates those manual mocks by:

  1. Adding a new BitwardenSdkMocks framework that declares empty extensions of SDK types annotated with // sourcery:file: AutoMockable, allowing Sourcery to generate the mocks automatically. This allows the mocks to be shared by Bitwarden and Authenticator.
  2. Extracting the inline Sourcery build phase script into Scripts/generate-mocks.sh, which derives BITWARDEN_SDK_PATH from Xcode's BUILD_DIR so Sourcery can resolve SDK source files when generating mocks. Sourcery needs access to the SPM checkout at BITWARDEN_SDK_PATH to be able to inspect the SDK source.
  3. Converting AccountTokenProvider, AccountTokenProviderDelegate, and ClientManagedTokens to AutoMockable and deleting their manual mock implementations.
  4. Updating BitwardenShared and AuthenticatorShared sourcery configs to include the SDK and Networking source paths so Sourcery can fully resolve inherited protocol APIs.

To mock a type from the SDK:

  1. Add an empty extension to BitwardenSdkMocks/BitwardenSdkMocks.swift. This tells Sourcery it should generate a mock for that type.
  2. Re-build, which will generate BitwardenSdkMocks/Sourcery/Generated/AutoMockable.generated.swift.
  3. Ensure BitwardenSdkMocks is imported in the test target.

Note

I ran into a Sourcery issue when generating the mocks initially, where MockAccountTokenProvider included unrelated types from the State struct. I was able to workaround this by removing the Sendable constraints on the associated types in SearchHandler. e270595

class MockAccountTokenProvider: AccountTokenProvider, @unchecked Sendable {
   // These all come from `State`, not `AccountTokenProvider`.
   var accounts: [String: Account] = [:]
   var activeUserId: String?
   var activeAccount: Account?
   ...
}

More details here: krzysztofzablocki/Sourcery#1458

@matt-livefront matt-livefront requested review from a team as code owners April 24, 2026 21:23
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:ci Change Type - Updates to automated workflows t:tech-debt Change Type - Tech debt t:llm Change Type - LLM related change (e.g. CLAUDE.md files) labels Apr 24, 2026
@matt-livefront matt-livefront added ai-review Request a Claude code review and removed t:ci Change Type - Updates to automated workflows t:llm Change Type - LLM related change (e.g. CLAUDE.md files) labels Apr 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.08%. Comparing base (aa51cd7) to head (f8e0b05).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2581      +/-   ##
==========================================
- Coverage   87.17%   86.08%   -1.09%     
==========================================
  Files        1886     2114     +228     
  Lines      166723   181654   +14931     
==========================================
+ Hits       145335   156380   +11045     
- Misses      21388    25274    +3886     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added t:ci Change Type - Updates to automated workflows t:llm Change Type - LLM related change (e.g. CLAUDE.md files) labels Apr 27, 2026
Comment on lines -10 to +12
associatedtype Action: Sendable
associatedtype Effect: Sendable
associatedtype State: Sendable
associatedtype Action
associatedtype Effect
associatedtype State
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Is it safe to remove Sendable here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types are ultimately constrained by Store (typealias HandlerStore = Store<State, Action, Effect>) which requires that they are Sendable, so I don't think there's any downside:

open class Store<State: Sendable, Action: Sendable, Effect: Sendable>: ObservableObject {

@fedemkr
Copy link
Copy Markdown
Member

fedemkr commented Apr 29, 2026

This looks awesome Matt, kudos on making this work with BitwardenSdk mocks!

Copy link
Copy Markdown
Contributor

@KatherineInCode KatherineInCode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This is a tooling/test-infrastructure PR that introduces a BitwardenSdkMocks framework so Sourcery can auto-generate mocks for BitwardenSdk types via empty extensions. It extracts the inline Sourcery build phase into Scripts/generate-mocks.sh, wires the new framework into BWPM and BWA test targets, and converts AccountTokenProvider, AccountTokenProviderDelegate, and ClientManagedTokens to AutoMockable (deleting the hand-written mocks). The Sendable constraint relaxation on SearchHandler's associated types is safe because conformers must still produce a concrete Store<State, Action, Effect>, whose generic constraints already require Sendable.

Code Review Details
  • ♻️: Sibling Sourcery docs (testing-ios-code/references/mock-generation.md, testing-ios-code/SKILL.md) still tell users to run mint run sourcery --config <framework>/Sourcery/sourcery.yml, which will no longer resolve the SDK sources after this PR because the configs now reference ${BITWARDEN_SDK_PATH}.
    • .claude/skills/build-test-verify/SKILL.md:80-85

Comment on lines 80 to +85
```bash
# Mock generation
mint run sourcery --config BitwardenShared/Sourcery/sourcery.yml
mint run sourcery --config AuthenticatorShared/Sourcery/sourcery.yml
mint run sourcery --config BitwardenKit/Sourcery/sourcery.yml
# Mock generation — run from an Xcode build phase (BUILD_DIR is set automatically by Xcode)
# To run standalone, supply BUILD_DIR manually — see the script header for the one-liner
./Scripts/generate-mocks.sh BitwardenShared # default if argument omitted
./Scripts/generate-mocks.sh AuthenticatorShared
./Scripts/generate-mocks.sh BitwardenKit
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ DEBT: Sibling Sourcery docs still reference the old mint run sourcery --config ... command, which will no longer work standalone after this PR.

Details

After this PR, BitwardenShared/Sourcery/sourcery.yml and AuthenticatorShared/Sourcery/sourcery.yml reference ${BITWARDEN_SDK_PATH}, which is only set by Scripts/generate-mocks.sh. Two skill docs still tell users to invoke Sourcery directly and would now produce broken or empty SDK source resolution:

  • .claude/skills/testing-ios-code/references/mock-generation.md lines 23, 26 (BitwardenShared, AuthenticatorShared configs)
  • .claude/skills/testing-ios-code/SKILL.md line 141

Consider updating these to point at ./Scripts/generate-mocks.sh <framework> for consistency with the build-test-verify SKILL.md change in this PR.

The BitwardenKit and AuthenticatorBridgeKit references in those docs continue to work since their configs don't reference ${BITWARDEN_SDK_PATH}.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matt-livefront : Follow-up PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will follow-up with a fix for this.

@github-actions
Copy link
Copy Markdown
Contributor

Logo
Checkmarx One – Scan Summary & Details477c3069-b6f6-4582-ac5e-2013e25ab663

Great job! No new security vulnerabilities introduced in this pull request

@matt-livefront matt-livefront merged commit e10d63c into main Apr 30, 2026
55 of 58 checks passed
@matt-livefront matt-livefront deleted the matt/PM-35680-sourcery-sdk-mocks branch April 30, 2026 15:20
@aj-bw aj-bw mentioned this pull request May 6, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:ci Change Type - Updates to automated workflows t:llm Change Type - LLM related change (e.g. CLAUDE.md files) t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants