Re-enable runtime async on iOS/Android tests - #125450
Conversation
|
Tagging subscribers to this area: @akoeplinger, @matouskozak, @simonrozsival |
There was a problem hiding this comment.
Pull request overview
This PR re-enables the runtime async test configuration for Android and Apple mobile targets by removing platform exclusions from the shared test MSBuild logic in eng/testing/tests.targets. This allows eligible .NET 11+ CoreCLR test projects on those targets to compile with the runtime-async=on feature like other supported platforms.
Changes:
- Removed the
TargetOS != androidexclusion from the runtime async enabling condition. - Removed the
TargetsAppleMobile != trueexclusion from the runtime async enabling condition.
You can also share your feedback on Copilot code review. Take the survey.
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "545d0e9603042d9af993d31a08ca1eff1ebfc1ec",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "3f56fbb71346a302f8a2174848bf2bf9d6c7a495",
"last_reviewed_commit": "545d0e9603042d9af993d31a08ca1eff1ebfc1ec",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "3f56fbb71346a302f8a2174848bf2bf9d6c7a495",
"last_recorded_worker_run_id": "29673729352",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "545d0e9603042d9af993d31a08ca1eff1ebfc1ec",
"review_id": 4730103989
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: Runtime async (v2) was recently enabled for wasm/coreclr test projects in #125430 (merged). This PR extends that rollout to iOS and Android by removing the two platform exclusions (TargetOS != android, TargetsAppleMobile != true) that previously kept the runtime-async=on feature off for those targets.
Approach: A minimal, two-line deletion in eng/testing/tests.targets that removes the Android and Apple-mobile conditions from the shared PropertyGroup that opts .NET 11+ test projects into runtime async. The remaining guards on that condition are unchanged: it still requires net11.0+ compatibility, excludes NativeAOT, ReadyToRun (unless async is explicitly on), RuntimeFlavor == Mono, wasi, and honors an explicit UseRuntimeAsync=false opt-out. Because the RuntimeFlavor != 'Mono' guard remains, runtime async is enabled only for CoreCLR-flavored mobile test builds, not the Mono-based mobile runtime, which is consistent with the feature's current scope.
Summary: The change is small, self-consistent, and mirrors the pattern established by the already-merged #125430. The condition's ordering and the retained guards are correct, and no other references in the file depend on the removed clauses. The real risk is behavioral rather than code-level: whether CoreCLR iOS/Android test legs actually pass with runtime-async=on. That is best validated by the PR's CI runs on the mobile/runtime-extra-platforms pipelines rather than by static review. Assuming those legs are green, this is a clean, low-risk enablement change. LGTM.
Detailed Findings
No actionable code-level findings. The only diff is the removal of the two platform exclusion clauses, and the surrounding condition remains valid MSBuild. Recommend confirming the relevant iOS/Android CoreCLR test pipelines are green before merge, since correctness here is determined by test execution results rather than the build logic itself.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 41.2 AIC · ⌖ 14.5 AIC · ⊞ 10K
This change removes the exclusions for Android and Apple mobile targets from the runtime async test configuration in eng/testing/tests.targets.
Runtime async was previously disabled for these platforms. This PR re-enables it by removing the
TargetOS != androidandTargetsAppleMobile != trueconditions, allowing runtime async tests to run on iOS and Android alongside other supported platforms.This follows the same pattern as #125430, which recently enabled runtime async (v2) for wasm/coreclr tests.