Skip to content

[PM-39279] chore: Remove unnecessary ErrorReportBuilderTests setup#2807

Open
matt-livefront wants to merge 1 commit into
mainfrom
matt/PM-39279-error-report-builder-tests
Open

[PM-39279] chore: Remove unnecessary ErrorReportBuilderTests setup#2807
matt-livefront wants to merge 1 commit into
mainfrom
matt/PM-39279-error-report-builder-tests

Conversation

@matt-livefront

@matt-livefront matt-livefront commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-39279

📔 Objective

Removes some unnecessary setup in ErrorReportBuilderTests.

Swift Testing makes it hard to see how long these tests take in the larger suite of tests, but sometimes I see these tests popping up with long test times (>1 second). I'm not sure how much of an effect this will have, but the tests seemed faster in the runs that I did without this setup. And there's no UI here, so there shouldn't be any downsides.

@matt-livefront matt-livefront requested a review from a team as a code owner June 18, 2026 21:44
@matt-livefront matt-livefront added ai-review Request a Claude code review t:tech-debt Change Type - Tech debt labels Jun 18, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context labels Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the removal of UI appearance setup (UI.applyDefaultAppearances(), UI.animated, UI.sizeCategory) and the @MainActor annotation from ErrorReportBuilderTests. The change is test-only and well-scoped. All tests in the file assert on text output via assertInlineSnapshot(..., as: .lines) rather than rendering views, so the removed UI configuration was genuinely unnecessary. ErrorReportBuilder is a pure data builder with no UI or main-actor dependencies, so dropping @MainActor is safe.

Code Review Details

No findings. The removed setup served no purpose for these text-based inline snapshot tests, and no test body or the subject under test relies on main-actor isolation or the UI appearance state.

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.65%. Comparing base (68caf09) to head (c151167).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2807       +/-   ##
===========================================
- Coverage   80.82%   40.65%   -40.18%     
===========================================
  Files        1018      356      -662     
  Lines       64941    16583    -48358     
===========================================
- Hits        52490     6742    -45748     
+ Misses      12451     9841     -2610     

☔ View full report in Codecov by Harness.
📢 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.

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:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant