[mobile] Skip AssemblyLoadContext tests using Assembly.Location on mobile#127082
[mobile] Skip AssemblyLoadContext tests using Assembly.Location on mobile#127082kotlarmilos merged 3 commits intomainfrom
Conversation
These tests fail on mobile platforms because Assembly.Location returns an empty string. On mobile, assemblies are loaded from the APK/app bundle, not from individual files. Fixes failures discovered in build #1383320: - InvalidCastException_DifferentALC_ShowsAssemblyInfo - InvalidCastException_GenericTypeArg_DifferentALC_ShowsAssemblyInfo Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to 'os-tvos': @vitek-karas, @kotlarmilos, @steveisok, @akoeplinger |
|
Note This review was generated by Copilot. 🤖 Copilot Code Review — PR #127082Holistic AssessmentMotivation: The PR is justified. Both tests call Approach: The fix works functionally, but uses Summary: Detailed Findings
|
| File | Line(s) |
|---|---|
AssemblyLoadContextTest.cs |
111 |
ContextualReflection.cs |
147 |
LoaderLinkTest.cs |
16 |
RefEmitLoadContextTest.cs |
67 |
SatelliteAssemblies.cs |
77, 134, 190, 211 |
Why HasAssemblyFiles is preferable:
- Semantically precise: Defined as
!string.IsNullOrEmpty(typeof(PlatformDetection).Assembly.Location)— it checks the exact condition these tests depend on. - Future-proof: Automatically covers any future platform or configuration (e.g., single-file deployment) where
Assembly.Locationis empty, without needing manual updates. - Less redundant:
IsNotMobileexcludes Browser/WASI, butIsCoreCLRalready excludes those (they don't use CoreCLR). The only net-new exclusions fromIsNotMobileare Android and AppleMobile running CoreCLR — whichHasAssemblyFileswould also handle.
Suggested change (both test methods at lines 280 and 301):
-[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsAssemblyLoadingSupported), nameof(PlatformDetection.IsCoreCLR), nameof(PlatformDetection.IsNotMobile))]
+[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsAssemblyLoadingSupported), nameof(PlatformDetection.IsCoreCLR), nameof(PlatformDetection.HasAssemblyFiles))]This is advisory, not merge-blocking — the current code is functionally correct.
✅ Correctness — Skip condition prevents the right failures
The change correctly prevents both tests from running on platforms where Assembly.Location returns empty string. The IsNotMobile check does exclude all the right platforms and will resolve the reported CI failures.
Generated by Code Review for issue #127082 · ◷
There was a problem hiding this comment.
Pull request overview
Skips two System.Runtime.Loader tests on mobile platforms where Assembly.Location is empty (assemblies loaded from app bundles/APKs), preventing LoadFromAssemblyPath("") failures in mobile CI.
Changes:
- Add
PlatformDetection.IsNotMobileto theConditionalFactguards for twoAssemblyLoadContextTesttest cases that rely onAssembly.Location.
…bly.Location Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/a80757bd-1d3b-4ee5-8794-14353d687456 Co-authored-by: kotlarmilos <11523312+kotlarmilos@users.noreply.github.com>
Fixes test failures on Android, iOS, tvOS, and MacCatalyst discovered in build #1383320.
Failure
Test:
System.Runtime.Loader.Tests.AssemblyLoadContextTestHelix job:
c1e7d30b-9f1d-4f11-9117-cfe78a4df8b9Work item:
System.Runtime.Loader.TestsPlatform: android-arm64 (also affects all mobile platforms)
Failing tests
InvalidCastException_DifferentALC_ShowsAssemblyInfoInvalidCastException_GenericTypeArg_DifferentALC_ShowsAssemblyInfoRoot cause
Both tests call
LoadFromAssemblyPath(typeof(AssemblyLoadContextTest).Assembly.Location). On mobile platforms,Assembly.Locationreturns an empty string because assemblies are loaded from the APK/app bundle, not from individual files on disk.Fix
Add
nameof(PlatformDetection.IsNotMobile)to both tests'ConditionalFactattributes to skip on mobile platforms whereAssembly.Locationis not available.Testing
These tests will be skipped on Android, iOS, tvOS, and MacCatalyst in the
runtime-extra-platformspipeline. Desktop platforms will continue to run the tests.Note
This PR was created by GitHub Copilot after analyzing mobile platform CI failures in the runtime-extra-platforms pipeline.
Contributes to #127080