Skip to content

[TrimmableTypeMap] Add CoreCLRTrimmable Mono.Android.NET-Tests lane#11091

Open
simonrozsival wants to merge 5 commits intomainfrom
dev/simonrozsival/trimmable-test-plumbing
Open

[TrimmableTypeMap] Add CoreCLRTrimmable Mono.Android.NET-Tests lane#11091
simonrozsival wants to merge 5 commits intomainfrom
dev/simonrozsival/trimmable-test-plumbing

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented Apr 8, 2026

Summary

Add the CoreCLRTrimmable Mono.Android.NET-Tests CI lane and fix the Release regressions exposed by the trimmable typemap path.

This PR includes

[TrimmableTypeMap] Fix bugs exposed by Release+CoreCLR+trimmable test run

Generator fixes:

  • Use (managedName, assemblyName) tuple as scanner dictionary key — prevents duplicate-type crashes when two assemblies define the same managed name (e.g. Java.Lang.Throwable in both Java.Interop and Mono.Android)
  • Add MergeCrossAssemblyAliases to propagate aliases across assembly boundaries before splitting into per-assembly typemap universes (Release only)
  • Emit TypeMapAssociationAttribute for all entries with proxies so the runtime proxy type map is populated correctly (fixes CreatePeer for interface types)
  • ForceUnconditionalEntries=true workaround for dotnet/runtime#127004 — trimmer strips TypeMapAssociation attributes when TypeMap references the same type
  • Fix invalid ParameterHandle (was default/row-0); use valid 1-based row index

Runtime fixes:

  • Add JavaPeerProxy.ShouldSkipActivation(IntPtr) to detect existing managed peers and prevent duplicate peer creation during UCO constructor callbacks

Build fixes:

  • Add _PrepareTrimmableNativeConfigAssemblies target to populate native config unconditionally (not inside _GenerateJavaStubs which can be skipped)
  • Add typemap DLLs to _ShrunkAssemblies to keep _RemoveRegisterAttribute counts in sync
  • Skip _GenerateTrimmableTypeMap in inner per-RID builds

[tests] Add CoreCLRTrimmable test plumbing

[CI] Add CoreCLRTrimmable lane

  • Runs Mono.Android.NET-Tests with _AndroidTypeMapImplementation=trimmable and UseMonoRuntime=false

Excluded tests (48)

All 48 excluded tests are precisely named in NUnitInstrumentation.cs — no over-broad prefix matching. They are excluded only for trimmable builds (guarded by RuntimeFeature.TrimmableTypeMap). See #11170 for full inventory with root causes.

Root Cause Count
JavaProxyObject/JavaProxyThrowable not in APK 24
Missing net.dot.jni.test.* / net.dot.jni.internal.* Java classes 5
Proxy resolution / trimmer gaps 4
JavaCast proxy resolution 4
Open generic types / registration 4
JNI method remapping not supported 4
Other 3

Validation

Local Release + CoreCLR + trimmable run:

@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from f8106b0 to a437455 Compare April 9, 2026 09:19
@simonrozsival simonrozsival added trimmable-type-map copilot `copilot-cli` or other AIs were used to author this labels Apr 9, 2026
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-runtime-fixes branch from d5aefaa to 3a8d509 Compare April 10, 2026 09:49
Base automatically changed from dev/simonrozsival/trimmable-runtime-fixes to main April 10, 2026 22:50
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch 2 times, most recently from 1814a6d to d0e30f6 Compare April 11, 2026 09:42
@simonrozsival simonrozsival changed the title [TrimmableTypeMap] Test plumbing and CI lane for Mono.Android.NET-Tests [TrimmableTypeMap] Stabilize CoreCLR Mono.Android.NET-Tests lane Apr 11, 2026
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from 17b123b to e2c70fd Compare April 11, 2026 23:28
@simonrozsival simonrozsival changed the base branch from main to dev/simonrozsival/root-manifest-referenced-types April 11, 2026 23:28
simonrozsival added a commit that referenced this pull request Apr 11, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from e2c70fd to 87c531f Compare April 11, 2026 23:50
@simonrozsival simonrozsival changed the title [TrimmableTypeMap] Stabilize CoreCLR Mono.Android.NET-Tests lane [TrimmableTypeMap] Add CoreCLR Mono.Android.NET-Tests lane plumbing Apr 11, 2026
@simonrozsival simonrozsival marked this pull request as ready for review April 12, 2026 08:01
Copilot AI review requested due to automatic review settings April 12, 2026 08:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds CI/test-lane plumbing to run Mono.Android.NET-Tests on CoreCLR with the trimmable typemap implementation, keeping currently-unsupported test buckets explicitly excluded to maintain green coverage while follow-up runtime/generator work lands separately.

Changes:

  • Adds a new CoreCLRTrimmable APK instrumentation lane and pins the existing CoreCLR lane to llvm-ir.
  • Updates Mono.Android.NET-Tests.csproj to default to CoreCLR + exclude trimmable-specific buckets when _AndroidTypeMapImplementation=trimmable.
  • Centralizes additional trimmable-mode runtime test exclusions in NUnitInstrumentation, and adjusts a few Java.Interop tests/projects to avoid unsupported coverage.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/NUnitInstrumentation.cs Adds runtime-switch-driven exclusions for categories and specific Java.Interop test names under trimmable typemap.
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj Sets defaults/exclusions for trimmable typemap runs (incl. CoreCLRTrimmable flavor).
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs Marks specific tests as TrimmableIgnore.
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaObjectExtensionsTests.cs Marks a specific test as TrimmableIgnore (and normalizes file header).
tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj Removes JniTypeUtf8Test.cs from compilation due to unsupported APIs in the product assembly.
build-tools/automation/yaml-templates/stage-package-tests.yaml Updates CoreCLR lane args and adds the new CoreCLRTrimmable lane.

Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review Summary

Verdict: ✅ LGTM

Found 1 suggestion:

  • 💡 Code organization — the trimmable category filtering is now split between Mono.Android.NET-Tests.csproj and NUnitInstrumentation.cs (NUnitInstrumentation.cs:29). Keeping one source of truth would make the temporary exclusions easier to retire.

The lane wiring itself looks good, and I like that the unsupported cases stay explicit instead of being hidden behind runtime fallback behavior.


Review generated by android-reviewer from review guidelines.

Base automatically changed from dev/simonrozsival/root-manifest-referenced-types to main April 13, 2026 11:47
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch 2 times, most recently from 02780bb to c3a7ebc Compare April 13, 2026 12:15
Comment thread build-tools/automation/yaml-templates/stage-package-tests.yaml Outdated
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from 619991d to 6a0644f Compare April 13, 2026 16:39
@simonrozsival simonrozsival changed the base branch from main to dev/simonrozsival/fix-deferred-registration-propagation April 13, 2026 16:39
Base automatically changed from dev/simonrozsival/fix-deferred-registration-propagation to main April 14, 2026 14:19
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from 19ec2fa to e084238 Compare April 14, 2026 14:32
@simonrozsival simonrozsival changed the title [TrimmableTypeMap] Add CoreCLR Mono.Android.NET-Tests lane plumbing [TrimmableTypeMap] Add CoreCLRTrimmable Mono.Android.NET-Tests lane plumbing Apr 14, 2026
@simonrozsival
Copy link
Copy Markdown
Member Author

simonrozsival commented Apr 17, 2026

Splitting this PR into focused, reviewable pieces so each fix gets the attention it deserves. This PR will stay open and become a thin, clean addition to CI configuration once all the children land on main.

Child PRs

# Scope Status
#11142 Package CoreCLR preserve list in SDK pack (+<None> with Link + test asserting the file is actually packaged) merged
#11144 Fix IL1034 by excluding the app assembly from trimmer roots (Option A — preserves the whole point of the trimmable typemap by not rooting everything) merged
#11143 Manifest generator fixes: default instrumentation targetPackage, stale-manifest sentinel on mode switch, outer IntermediateOutputPath, CRC name rewriting in manifest template merged
#11145 Runtime and typemap generator fixes: init ordering, generic type proxy loading, ClassLoader mismatch in RegisterNatives, abstract Instrumentation/Application subtypes, proxy self-application as [JavaPeerProxy] + regression test merged

Once the children merge, this PR will be rebased to include only:

  • The new CoreCLRTrimmable CI lane + TestEnvironment plumbing.
  • Test-side adjustments and CoreCLRTrimmable-specific skips for failing JavaCast/JavaAs/JavaObjectArray tests.

Follow-ups (issues)

  • Unify the native-method registration approach: always defer via __md_registerNatives () for all types, remove the CannotRegisterInStaticConstructor flag and its propagation helpers, and revisit static { registerNatives } as a later optimization for provably safe types. ([TrimmableTypeMap] Runtime and typemap generator fixes #11145 notes the known gap this creates for concrete descendants of abstract Application/Instrumentation.)

@simonrozsival simonrozsival marked this pull request as draft April 17, 2026 10:35
simonrozsival added a commit that referenced this pull request Apr 18, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival added a commit that referenced this pull request Apr 20, 2026
Keep the trimmable typemap PR focused on the CoreCLRTrimmable lane and its targeted test-side plumbing. Move the generic NUnit runner and logcat auditing work to PR #11162, and drop the leftover Preserve-related noise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival marked this pull request as ready for review April 20, 2026 10:46
Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Needs Changes

Nice split between the trimmable typemap/product work here and the generic harness changes in #11162 — that makes this PR much easier to reason about. I left one inline suggestion on the new regression tests.

CI note: the public checks look good, but the internal Xamarin.Android-PR pipeline is still pending, so this is not merge-ready quite yet.

jonathanpeppers added a commit that referenced this pull request Apr 20, 2026
## Summary
Improve the Android NUnit harness so trimmable and test-lane investigations can audit discovery and exclusions reliably.

## Changes
- fix final Passed/Failed/Skipped summary counts to match NUnit aggregate results
- add a discovery-only dry-run mode
- add noexclusions=true to bypass built-in exclusions for auditing
- surface excluded tests as ignored results with explicit reasons
- overwrite logcat output per run so stale logs do not masquerade as current crashes

## Why
These generic test-runner improvements were useful while working on #11091. I had problems identifying which tests were discovered, but did not run, as passed + skipped + failed < total.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
@simonrozsival
Copy link
Copy Markdown
Member Author

failing tests are unrelated

@simonrozsival
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2026

Android PR Reviewer completed successfully!

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — well-structured change

Summary: Adds the CoreCLRTrimmable Mono.Android.NET-Tests CI lane and fixes several trimmable typemap issues exposed by running Release + CoreCLR + trimmable builds. Clean and well-organized PR.

What works well

  • PropagateCannotRegisterToDescendants — correct bidirectional propagation (up to base classes, then down to descendants) ensures the complete transitive closure. Algorithm is simple and appropriate for the small number of types involved.
  • CollectApplicationRegistrationTypes — smart filtering that keeps only the two framework roots plus app/runtime types, avoiding legacy android.test.* descendants that don't exist on modern API surfaces. Well-tested.
  • AdditionalResolvedAssemblies on GenerateNativeApplicationConfigSources — clean separation of counting (for native config) from assembly store inclusion (handled by _AddTrimmableTypeMapAssembliesToStore). Fixes the incremental build issue where the old approach inside _GenerateJavaStubs could be skipped.
  • Test coverage — new unit tests, build-level regression tests for assembly store mappings, and incremental build verification.
  • TrimmerRoots.xml — targeted preservation of only the types needed by the test suite via reflection.

Issues found

Severity Count
💡 Suggestion 5

No bugs or correctness issues found. All suggestions are minor formatting/style items.

CI Status

Public checks (license/cla, dotnet-android) are ✅ passing. Internal Azure DevOps build status should be verified separately.

Generated by Android PR Reviewer for issue #11091 · ● 9.1M

skipped on incremental builds, which would leave the item group empty.
-->
<Target Name="_PrepareTrimmableNativeConfigAssemblies"
BeforeTargets="_GeneratePackageManagerJava">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 💡 MSBuild targets_PrepareTrimmableNativeConfigAssemblies uses BeforeTargets="_GeneratePackageManagerJava", but $(_GeneratePackageManagerJavaDependsOn) exists and is the preferred extension point. Since this file is imported after Common.targets, you could append to the property:

<PropertyGroup>
  <_GeneratePackageManagerJavaDependsOn>
    _PrepareTrimmableNativeConfigAssemblies;
    $(_GeneratePackageManagerJavaDependsOn)
  </_GeneratePackageManagerJavaDependsOn>
</PropertyGroup>

That said, BeforeTargets is simpler here and the comment explains the rationale well, so this is a "consider" not a "must fix."

Rule: Avoid BeforeTargets/AfterTargets (Postmortem #33)

}

var environmentText = File.ReadAllText (environmentFile);
var runtimeDataMatch = Regex.Match (environmentText, @"assembly_store_bundled_assemblies.*\[(\d+)\s+x");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 💡 Performance — The two Regex patterns inside the foreach loop are recompiled on every iteration. For a test this isn't critical, but consider extracting them to static readonly Regex fields (or using [GeneratedRegex]) for cleanliness:

static readonly Regex AssemblyStoreCountRegex = new (@"assembly_store_bundled_assemblies.*\[(\d+)\s+x", RegexOptions.Compiled);
static readonly Regex MappingIndexRegex = new (@"\bmi:(\d+)\b", RegexOptions.Compiled);

Rule: Avoid unnecessary allocations

@simonrozsival
Copy link
Copy Markdown
Member Author

@copilot implement the suggestions in review comments

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

@copilot implement the suggestions in review comments

Implemented in 36b783e. Tests in our project that are trimmable-incompatible now use [Category("TrimmableIgnore")] (5 SSL cert callback tests in AndroidMessageHandlerTests.cs, 5 JavaCast/JavaAs tests in JavaObjectExtensionsTests.cs, and 2 typemap tests in JnienvTest.cs). The TrimmableIgnore category is excluded via ExcludeCategories in the csproj. NUnitInstrumentation.cs now only lists the 10 tests from the external Java.Interop-Tests assembly that we can't categorize without modifying the upstream submodule, with a comment explaining why.

🤖 AI Review Summary

Verdict: ✅ LGTM...

Addressed in 36b783e — tests in this project that are trimmable-incompatible now carry [Category("TrimmableIgnore")] and are excluded via ExcludeCategories in the csproj. Only the external Java.Interop-Tests fixtures (from the upstream submodule we don't control) remain listed by name in NUnitInstrumentation.cs.

simonrozsival and others added 3 commits April 24, 2026 19:10
… run

Fix several trimmable typemap generator and runtime bugs that prevented
Mono.Android.NET-Tests from passing in Release+CoreCLR+trimmable mode:

Generator:
- Use (managedName, assemblyName) tuple as scanner dictionary key to prevent
  duplicate-type crashes when two assemblies define the same managed type name
  (e.g. Java.Lang.Throwable in both Java.Interop and Mono.Android)
- Add MergeCrossAssemblyAliases to propagate aliases across assembly boundaries
  before splitting peers into per-assembly typemap universes (Release only)
- Emit TypeMapAssociationAttribute for all entries with proxies so the runtime
  proxy type map is populated correctly (fixes CreatePeer for interface types)
- ForceUnconditionalEntries=true workaround for dotnet/runtime#127004 (trimmer
  strips TypeMapAssociation attributes when TypeMap references the same type)
- Fix invalid ParameterHandle (was default/row-0); use valid 1-based row index
- Skip inner per-RID builds in _GenerateTrimmableTypeMap (they lack the full
  assembly set needed for correct deferred-registration propagation)

Runtime:
- Add JavaPeerProxy.ShouldSkipActivation(IntPtr) to detect existing managed
  peers and prevent duplicate peer creation during UCO constructor callbacks
- Use ShouldSkipActivation in UCO nctor callbacks instead of WithinNewObjectScope

Build:
- Add _PrepareTrimmableNativeConfigAssemblies target to populate native config
  with typemap DLL paths (must run unconditionally, not inside _GenerateJavaStubs)
- Add typemap DLLs to _ShrunkAssemblies to keep _RemoveRegisterAttribute counts
  in sync with _ResolvedAssemblies

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Configure Mono.Android.NET-Tests to build and run successfully with
_AndroidTypeMapImplementation=trimmable and UseMonoRuntime=false.

Changes:
- Root test and runner assemblies in TrimmerRoots.xml so NUnit can
  discover and execute tests after trimming
- Exclude 48 tests that are incompatible with the trimmable typemap
  (tracked in #11170):
  - 5 tests that require net.dot.jni.test.*/net.dot.jni.internal.* Java classes
  - 24 tests relying on JavaProxyObject/JavaProxyThrowable (not in APK)
  - 4 proxy resolution / trimmer gaps
  - 4 JavaCast proxy resolution failures
  - 4 open generic type / registration failures
  - 4 JNI method remapping not supported
  - 3 other
- Exclusions are guarded by RuntimeFeature.TrimmableTypeMap so they
  only apply to trimmable builds
- Add TODO comments to tests with known trimmable-specific failures
  (issue #11170) so they are visible to contributors

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a new CI lane that runs Mono.Android.NET-Tests with the trimmable
typemap implementation on CoreCLR:
  _AndroidTypeMapImplementation=trimmable
  UseMonoRuntime=false

The lane runs after the existing CoreCLR lane and uses the same APK
artifact from the CoreCLRTrimmable build flavor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-test-plumbing branch from fd05865 to 2d750b0 Compare April 24, 2026 17:11
simonrozsival and others added 2 commits April 24, 2026 22:33
In non-trimmed (Debug) builds, _AndroidEnableObjectReferenceLogging defaults
to true, so RuntimeFeature.ObjectReferenceLogging returns true. The existing
Meter test already uses the same #if DEBUG pattern; apply it here too.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The feature switch (ObjectReferenceLoggingEnabledByDefault) is the source of
truth; its default value varies by build configuration (true in non-trimmed
Debug builds, false in trimmed Release builds). Hardcoding an expected value
in the test is fragile. Remove the test case entirely.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copilot `copilot-cli` or other AIs were used to author this trimmable-type-map

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants