[PM-26063] Fix flaky test

[matt-livefront]

🎟️ Tracking

PM-26063

📔 Objective

Fixes a flaky Authenticator test. In this case, the waitFor was checking for a value set in the repository and the assertion was checking for a value in the state. Since the repository value is set before the state value, there was a race condition if the waitFor exited before the state value was set.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @matt-livefront's task —— View job

---

Code Review Complete

Summary of Changes

This PR fixes a flaky test in test_perform_search_totpExpired by correcting the race condition between repository value updates and state value updates. The fix changes both the waitFor condition and the assertion to check the same value (state), eliminating the timing issue.

Files Changed: 1 file

• AuthenticatorShared/UI/Vault/ItemList/ItemList/ItemListProcessorTests.swift (+4/-3)

---

Critical Issues

None found. The fix correctly addresses the race condition.

---

Suggested Improvements

1. Consider using waitForAsync for consistency

Location: ItemListProcessorTests.swift:449

The fix uses a closure-based waitFor condition, which is valid. However, the codebase shows a pattern of using waitForAsync for async conditions in several other tests in this file (lines 726, 777, 792, 793, 813, 828, etc.).

Current implementation

waitFor { subject.state.searchResults == [firstItemRefreshed] }

Alternative for consistency

try await waitForAsync { self.subject.state.searchResults == [firstItemRefreshed] }

Note: This would require changing the test function signature to async throws and using self. prefix, so the current fix is acceptable if you want to minimize changes.

2. Test name could be more descriptive

Location: ItemListProcessorTests.swift:418

The test name test_perform_search_totpExpired could be more descriptive of what behavior it's verifying. Following the naming pattern in Testing.md (line 969-972), consider:

// Current:
func test_perform_search_totpExpired() throws

// More descriptive:
func test_perform_search_totpExpired_updatesSearchResults() throws

This makes it clearer at a glance that the test verifies search results are updated when TOTP codes expire.

---

Good Practices Observed

• Correctly identified and fixed the root cause of the race condition
• Changed both the wait condition and assertion to check the same source of truth
• Maintained test isolation and proper setup/teardown
• Added comprehensive assertions to verify both repository and state updates

---

Action Items

Optional (for author's consideration):

1. Consider if waitForAsync would be more consistent with other async tests in this file (though current fix is acceptable)
2. Consider renaming test to test_perform_search_totpExpired_updatesSearchResults for better clarity

No blocking issues - This PR is ready to merge as-is. The suggested improvements are minor stylistic considerations.

---

[github-actions]

Checkmarx One – Scan Summary & Details – 290846ee-3c64-4108-8f41-bb259232de01

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.11%. Comparing base (68feef3) to head (107ee8c).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2108       +/-   ##
===========================================
- Coverage   85.23%   47.11%   -38.13%     
===========================================
  Files        1709      530     -1179     
  Lines      145493    27298   -118195     
===========================================
- Hits       124011    12861   -111150     
+ Misses      21482    14437     -7045     

☔ 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.