[PM-27522] Test workflows use simulator ID for xcodebuild destinations and remove test retry flags

[vvolkgang]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-27522

📔 Objective

1. Ensures Build and Test steps use the exact simulator instance that was booted
2. The combination of -retry-tests-on-failure and -test-repetition-relaunch-enabled YES forces all tests of a Target to retry if a single test fails. Discussed it with the team and we agreed to remove it for now, we may revisit this decision later.

⏰ 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

[github-actions]

Checkmarx One – Scan Summary & Details – c992f5cd-5794-40e5-8100-50eca92dd105

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

[github-actions]

Warning

@vvolkgang Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details.

[github-actions]

Warning

@vvolkgang Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details.

[github-actions]

Warning

@vvolkgang Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details.

[claude]

Claude finished @vvolkgang's task —— View job

---

PR Review: Test workflows use simulator ID for xcodebuild destinations

Summary of Changes

This PR makes two key improvements to the iOS test workflows:

1. Simulator ID Output & Usage: Modified boot-simulator action to output the booted simulator's device ID, which is then used in xcodebuild commands instead of relying on name/OS matching
2. Test Retry Flags Removal: Removed -retry-tests-on-failure and -test-repetition-relaunch-enabled YES flags that were causing entire test targets to retry when a single test failed

Files Changed:

• .github/actions/boot-simulator/action.yml (+7 lines)
• .github/workflows/test-bwa.yml (+7/-4 lines)
• .github/workflows/test.yml (+9/-6 lines)

---

Critical Issues

None found. The implementation is correct and achieves the stated objectives.

---

Suggested Improvements

1. Quote Consistency Enhancement

Location: .github/workflows/test.yml:162

The Test step has one remaining inconsistency with quoting:

kill "$PYEETD_PID"

While this was fixed in test.yml:162, the same pattern exists in test-bwa.yml:167. Both files now correctly quote $PYEETD_PID, which is good for consistency.

Actually, reviewing the diff again - both files correctly quote "$PYEETD_PID" now. ✓

2. Variable Quoting in test.yml

Location: .github/workflows/test.yml:158

Minor quoting inconsistency (already fixed in this PR)

The PR already fixes this:

-              -resultBundlePath $_TESTS_RESULT_BUNDLE_PATH \
+              -resultBundlePath "$_TESTS_RESULT_BUNDLE_PATH" \

This is good practice for shell variable safety. ✓

3. Documentation Consideration

Location: .github/actions/boot-simulator/action.yml:14-16

Consider adding a usage note in the output description mentioning that this ID should be used for xcodebuild destinations:

outputs:
  simulator-id:
    description: 'The identifier of the booted simulator device (use with -destination "platform=iOS Simulator,id=<id>")'
    value: ${{ steps.boot.outputs.device-id }}

This would make it clearer for future developers how to use this output.

---

Good Practices Observed

• Proper use of GitHub Actions step outputs and references
• Consistent application of changes across both test workflows
• Shell variable quoting improvements ($PYEETD_PID and $_TESTS_RESULT_BUNDLE_PATH)
• Clear separation of concerns (boot action outputs ID, workflows consume it)
• Appropriate use of environment variables for simulator ID scoping

---

Architecture & Design Review

Simulator ID Approach ✓

The change from using name=$_SIMULATOR_NAME,OS=$_SIMULATOR_VERSION to id=$_SIMULATOR_ID is the correct approach:

Why this is better:

• Eliminates race conditions: Multiple simulators with the same name/OS combo could exist; ID is unique
• Guarantees correct target: Uses the exact simulator instance that was booted in the previous step
• Reduces flakiness: No ambiguity about which simulator xcodebuild should target

Implementation verification:

• .github/actions/boot-simulator/action.yml:32: Extracts DEVICE_ID from simctl output ✓
• .github/actions/boot-simulator/action.yml:38: Outputs device-id to $GITHUB_OUTPUT ✓
• Both workflows reference via ${{ steps.boot-simulator.outputs.simulator-id }} ✓

Test Retry Removal ✓

Removing -retry-tests-on-failure and -test-repetition-relaunch-enabled YES is appropriate:

Rationale: These flags together cause all tests in a target to retry if any single test fails, which is not the intended behavior. This can:

• Mask intermittent test failures
• Significantly increase CI runtime
• Make it harder to identify which specific tests are flaky

The remaining flags still provide test reliability:

• -test-timeouts-enabled yes
• -maximum-test-execution-time-allowance 1

These prevent hanging tests without forcing unnecessary retries.

---

Security Considerations

No security concerns identified. Changes are limited to:

• CI workflow configuration (test execution)
• Simulator device targeting (build/test commands)

No changes to:

• Authentication/authorization
• Data handling
• Encryption
• Access controls

---

Testing Verification

Manual verification needed:

• Confirm CI workflows execute successfully with the new simulator ID approach
• Verify tests run against the correct simulator instance
• Ensure test results are properly collected without retry flags

Observable CI signals:
The PR description mentions CI builds have passed, which indicates the changes work correctly in practice.

---

Action Items

Optional Improvements

1. Consider enhancing the output description in boot-simulator/action.yml as suggested above
2. Monitor CI runs to confirm the removal of retry flags doesn't negatively impact test reliability

No Blocking Issues

This PR is ready to merge. All changes are correct, well-structured, and achieve the stated objectives.

---

Conclusion

This is a well-executed PR that addresses real CI reliability issues:

Strengths:

• Correctly implements simulator ID passing and usage
• Appropriately removes problematic retry flags
• Improves shell script quoting consistency
• Changes are minimal and focused

Impact:

• Reduces test flakiness by targeting exact simulator instances
• Improves test result accuracy by removing overly aggressive retry behavior
• Should reduce overall CI execution time

Recommendation: ✅ Approve and merge

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.92%. Comparing base (6ff9048) to head (88dd8e7).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2075      +/-   ##
==========================================
- Coverage   85.28%   82.92%   -2.36%     
==========================================
  Files        1695     1978     +283     
  Lines      144592   160907   +16315     
==========================================
+ Hits       123319   133437   +10118     
- Misses      21273    27470    +6197     

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