[tests] Cover interface-valued Java collections

[simonrozsival]

Summary

• add a minimal MSBuildDeviceIntegration app that isolates interface-valued Java collection wrappers from the monolithic runtime test app and its RootMode=All closure
• cover interface elements, dictionary keys/values/both, nulls, duplicate and distinct identities, enumeration, mutation, repeated lookup identity, inherited interfaces, exact invoker types, and Java round trips
• classify [TrimmableTypeMap] Support Java collections of interface element types under NativeAOT (JavaList<TInterface, TInvoker>) #11770 as stale on current main: all focused fixtures pass unchanged, so no production change is needed

The generated app binds only ValueProvider and ExtendedValueProvider. Its holder and base/derived concrete peers are package-private AndroidJavaSource items with Bind=false, retained only by focused ProGuard rules and invoked through raw JNI. The app calls JavaConvert.FromJniHandle(..., Type) with only user-facing IList<T>, ICollection<T>, or IDictionary<TKey,TValue> targets and asserts open JavaList<>, JavaCollection<>, or JavaDictionary<,> definitions plus runtime GenericTypeArguments.

Base-interface cases use a peer implementing only ValueProvider and assert exact IValueProviderInvoker activation. A separate peer implements ExtendedValueProvider and asserts exact IExtendedValueProviderInvoker activation. Dictionary keys, values, and pairs are traversed through raw java.util.Map.Entry JNI calls; the fixture does not use Keys, Values, or the managed dictionary enumerator, so no convenience property can construct another collection wrapper.

For NativeAOT, the host test streams the focused single-RID scan DGML and verifies the complete dependency chain for each wrapper:

SafeJavaCollectionFactory.CreateReference* -> IJavaPeerable constructed type -> generic dictionary -> canonical compiled constructor -> generic dependency -> activation constructor

Every incoming edge at each stage is checked against its exact expected source and reason. The test also rejects every other closed reference-wrapper activation constructor backed by the same canonical code. This proves the constructors come exclusively from SafeJavaCollectionFactory, not the app, generated bindings, blanket roots, unrelated collection tests, or dictionary convenience properties.

Validation

Lane	Focused app result
llvm-ir, CoreCLR, Release, arm64	6/6 passed
trimmable, CoreCLR, Release, arm64	6/6 passed
trimmable, NativeAOT, Release, arm64	6/6 passed; all 3 exclusive DGML rooting chains verified
MSBuildDeviceIntegration host cases	3/3 passed
Microsoft.Android.Sdk.TrimmableTypeMap.Tests	776/776 passed

Each device case starts from an empty project directory and uses emulator-5554, JDK /Users/simon/android-toolchain/jdk-25, explicit AndroidSdkDirectory/JavaSdkDirectory/JavaCPath/JarPath, targeted failure-safe adb uninstall, and an isolated APFS-cloned SDK/NuGet/build overlay. Result polling also honors the explicit ADB target when other emulators are attached.

Out of scope

JavaCollection<T>.Remove has a pre-existing runtime-independent JNI descriptor bug: it looks up Collection.remove(int) as (I)Ljava/lang/Object; and fails under the llvm-ir baseline before interface marshaling. This fixture uses Clear for collection mutation; remove remains covered by JavaList<T> and all dictionary shapes.

Fixes #11770

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	tests/​Mono.Android-Tests/​Mono.Android-Tests/​Java.Interop/​InterfaceCollectionMarshallingTests.cs — 🤖 💡 suggestion Testing — Add an assertion message here so failures from JNIEnv.IsSameObject…

What changed in this PR

Adds device-level regression coverage in tests/Mono.Android-Tests to ensure Java-created collections whose element (and/or key/value) types are Java interfaces marshal correctly in managed code (identity preservation, null handling, enumeration/mutation, inherited interfaces, and Java round-trips). This supports confidence that the interface-collection scenario described in #11770 behaves correctly on current main without requiring production/runtime changes.

Changes:

• Add a new NUnit fixture covering JavaList<TInterface>, JavaCollection<TInterface>, and JavaDictionary<…> shapes where TInterface is a bound Java interface, including round-trips and identity checks.
• Add Java-side deterministic fixtures (InterfaceCollectionHolder, package-private InterfaceCollectionPeer) to produce stable interface-typed collections for testing.
• Wire the new test fixture into Mono.Android.NET-Tests.csproj.

File	Description
tests/​Mono.Android-Tests/​Mono.Android-Tests/​Mono.Android.NET-Tests.csproj	Includes the new interface-collection marshalling test in the device test project.
tests/​Mono.Android-Tests/​Mono.Android-Tests/​java/​net/​dot/​android/​test/​InterfaceCollectionPeer.java	Adds a package-private concrete peer implementing the test interfaces.
tests/​Mono.Android-Tests/​Mono.Android-Tests/​java/​net/​dot/​android/​test/​InterfaceCollectionHolder.java	Adds Java factory/round-trip methods producing deterministic interface-typed collections/maps.
tests/​Mono.Android-Tests/​Mono.Android-Tests/​Java.Interop/​InterfaceCollectionMarshallingTests.cs	New NUnit coverage for interface-valued collection/list/dictionary marshaling, identity, nulls, and inherited-interface invokers.

Suppressed comments (1)

tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/InterfaceCollectionMarshallingTests.cs:251

• 🤖 💡 suggestion Testing — Add an assertion message here so a failure clearly indicates that two distinct Java handles unexpectedly compared as the same object.

			var firstPeer = (IJavaObject) first;
			var secondPeer = (IJavaObject) second;
			Assert.IsFalse (JNIEnv.IsSameObject (firstPeer.Handle, secondPeer.Handle));

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12614

[github-actions]

⚠️ Needs Changes

Findings: 0 errors · 1 warning · 0 suggestions

The focused fixture is well isolated, exercises the intended interface wrapper shapes, and carefully validates JNI identity and NativeAOT rooting. The remaining issue is the shortened device-result timeout, which undercuts the suite shared protection against slow CI emulator launches.

CI build 1576508 is still in progress: four Android tools/Java.Interop jobs and CLA have passed, three platform build jobs are running, and no failures are currently reported.

Generated by Android PR Reviewer for #12614 · gpt56 · 148.8 AIC · ⌖ 8.86 AIC · ⊞ 25.7K
_{Comment /review to run again}

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12614

[github-actions]

⚠️ Needs Changes

Found 1 warning and 0 errors. The fixture isolates the interface-valued collection paths well and the JNI local-reference cleanup is careful, but the result polling should use one target-aware logcat stream rather than repeatedly spawning full-buffer adb logcat -d snapshots.

CI is still in progress for build 1576613: the completed Android Tools macOS check and CLA have passed; six checks are running and the aggregate check is queued.

Generated by Android PR Reviewer for #12614 · gpt56 · 122.9 AIC · ⌖ 9.3 AIC · ⊞ 25.7K
_{Comment /review to run again}

[github-actions]

🤖 ⚠️ Testing — This predicate can launch up to 600 separate adb logcat -d processes, and each call may itself block for the 30-second RunAdbCommand timeout; WaitFor subtracts only the fixed 250 ms interval, not predicate runtime. A slow or wedged adb can therefore turn the intended 150-second wait into an hours-long lane, while repeatedly copying the whole log buffer. Please use a single streaming logcat process instead (for example, extend MonitorAdbLogcat to honor ADB_TARGET, then match the result token in its callback).

Rule: Reuse shared device monitoring infrastructure

[jonathanpeppers]

I don't think I would use a WaitFor() and then run an adb command inside.

Could we use this instead?

android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs

Lines 464 to 467 in a5808ff

	protected static bool WaitForActivityToStart (string activityNamespace, string activityName, string logcatFilePath, int timeout = 120)
	{
	return WaitForActivityToStart (activityNamespace, activityName, logcatFilePath, out TimeSpan time, timeout);
	}

Then check for the other message after?

You might just check if copilot can align with what other tests do.