Repository navigation
Drop JVM-only Java.Interop test runs - #12894
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One critical and two moderate findings remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Removes obsolete JVM-only Java.Interop test infrastructure and CI stages while retaining supported Android coverage.
Changes:
- Removes obsolete typemap exclusions and test fixtures.
- Removes Java.Interop pipeline stages and their template.
- Updates Android tests, project files, and test documentation.
| File | Reviewed changes |
|---|---|
tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs |
Removes obsolete category and test-name exclusions. |
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj |
❌ bug (critical, 1 vote): Removing Mono.Android.Export.dll breaks ExportTests.cs runtime coverage. |
tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj |
|
external/Java.Interop/tests/Java.Interop-Tests/java/net/dot/jni/test/CrossReferenceBridge.java |
Removes obsolete Java fixture. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/TestTypeTests.cs |
Removes obsolete category. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/MethodBindingTests.cs |
Removes obsolete category. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniValueMarshalerContractTests.cs |
Restricts unsupported Android contracts. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeTest.cs |
Restricts desktop-only tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs |
Restricts desktop-only value-manager tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntime.JniTypeManagerTests.cs |
Restricts desktop-only tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs |
Restricts desktop-only peer tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniInstanceMethodIDTest.cs |
Removes obsolete category. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs |
Removes deleted fixture mapping. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs |
Restricts unsupported Android tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaManagedGCBridgeTests.cs |
Removes obsolete GC bridge tests. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/InvokeVirtualFromConstructorTests.cs |
Removes obsolete category. |
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj |
❌ bug (moderate, 1 vote): Incremental packaging can retain the removed Java class because source deletions are not tracked. |
build-tools/automation/yaml-templates/stage-java-interop-tests.yaml |
Removes unused pipeline template. |
build-tools/automation/azure-pipelines.yaml |
Removes the official Java.Interop stage. |
build-tools/automation/azure-pipelines-public.yaml |
Removes the public Java.Interop stage. |
build-tools/automation/azure-pipelines-internal.yaml |
Removes the internal Java.Interop stage. |
.github/skills/tests/references/test-catalog.md |
Updates documented runtime exclusions. |
9ad62ef to
9926d98
Compare
1c780a7 to
06db1b0
Compare
06db1b0 to
9a8b7be
Compare
9a8b7be to
fceb155
Compare
fceb155 to
53b2943
Compare
53b2943 to
3a0b91f
Compare
3a0b91f to
b301a72
Compare
Stacked on #12887; **base: `simonrozsival-default-trimmable-typemap`**. ## Summary - Remove the obsolete llvm-ir typemap targets and generators, native typemap output, marshal-method assembly rewriting, legacy Java-stub tasks, and dead after-link chain. Keep shared application configuration, NativeAOT bootstrap, Java remapping, and the empty native symbols still required until #12890. - Preserve the trimmable build behavior previously mixed into #12887 as six separate, scoped commits: CoreCLR additional process providers; pre-ILLink abstract-method repair on project-local assembly copies; effective-RID post-trim JCW selection and Proguard inputs; typemap assembly metadata and FastDeploy preference for linked/ReadyToRun DLLs; Java-library categorization before nested D8; and the deserialization callback trimmer root. - Keep the shared post-ILLink steps, `LinkDescription`, `LinkerDumpDependencies`, and incremental file tracking. Reject `AndroidEnableMarshalMethods=true` with `XA1049` rather than accepting an unsupported path. The default property and `XA4265` validation remain in the small bottom PR. JVM-only test cleanup is in #12894; this PR does not replay the old bottom branch's broad CI and test-fixture changes. ## Validation - 1,025 standalone trimmable typemap tests and 11 focused build-task tests passed (7 fixture-dependent cases skipped); the FastDeploy task project built without warnings or errors. - Changed XML and the target dependency graph were checked, as was the diff. - Full build-task, host, and device integration tests could not run here without the built in-tree Android SDK and generated `generator.dll`; CI must validate the combined path.
Retain the main branch test-library trimmer root while rooting NUnit assemblies at ILLink and NativeAOT input collection time. Co-authored-by: Copilot App <[email protected]>
ae01c2a to
24ea425
Compare
The demo marshaler was moved from a removed contract-test fixture solely to keep this constructor test compiling. Use the existing JniInt32ValueMarshaler instead. Co-authored-by: Copilot App <[email protected]>
With trimmable as the only supported type map, NUnit roots no longer need late target hooks. Retain the existing roots at evaluation so NativeAOT ILC and ILLink both receive them. Co-authored-by: Copilot App <[email protected]>
|
@dalexsoto review |
dalexsoto
left a comment
There was a problem hiding this comment.
The complete current 36-file/48-hunk review, separate Android test-discovery/integration pass and final independent sweep found no high-confidence blocker. The retired desktop/JVM stage and legacy fixtures have no remaining supported Android consumers; Android device lanes, reflection-discovered test roots, generated trimmable export dispatch, and the Android-safe GetThis source/JAR selection remain consistent.
The obsolete non-trimmable Mono.Android.Export concern is superseded by the current supported trimmable policy and generated dispatch, not carried forward as a blocker. This is a static source/contract review, not executed build or device validation. CI is not claimed green: the observed Gradle test failure stopped on external Maven acquisition with "No route to host" and "Connection reset"; no retry was triggered by this review.
The Android production runtime uses the trimmable typemap managers rather than `Java.Interop.JniRuntime.ReflectionJniValueManager` or `Java.Interop.JniRuntime.ReflectionJniTypeManager`. Following the removal of JVM-only test runs in [#12894](#12894), remove these unused production implementations and their remaining host-JVM consumers instead of retaining reflection-based runtime infrastructure solely for dormant tests. Remove both managers, their generated helper tables and matching T4 templates, unshipped public API declarations, and obsolete suppressions. Delete the standalone `TestJVM` harness and host interop test project, while retaining the shared test sources compiled into the Android test assembly. Remove `JniProxyRuntime`, its custom manager test doubles, and its sole dependent test, `JniRuntimeTest.Dispose_ClearsJniEnvironment`, rather than preserving a second runtime implementation for that test. Delete the now-unnecessary `JavaVMFixture` base class and its remaining inheritance clauses, together with the always-skipped SafeHandle-dependent `JniTransitionTests.Dispose_ClearsLocalReferences` test. The remaining runtime tests use the active Android runtime. Update the solution, Android test project, and test documentation accordingly. This branch is rebased onto `main` after #12894 merged. No issue is closed by this cleanup. ### Validation - **Passed after rebasing and again after removing the proxy runtime:** `./dotnet-local.sh build external/Java.Interop/src/Java.Interop/Java.Interop.csproj -c Debug -v quiet -p:JavaCPath=/usr/bin/javac -p:JarPath=/usr/bin/jar` — zero warnings/errors. - **Passed after rebasing:** `./dotnet-local.sh build external/Java.Interop/src/Java.Interop/Java.Interop.csproj -c Release -v quiet -p:JavaCPath=/usr/bin/javac -p:JarPath=/usr/bin/jar` — zero warnings/errors. - **Passed:** `git diff --check origin/main...HEAD`; repository search found no remaining references to the removed managers, projects, proxy runtime, shared JVM fixture, capability flags, or dependent disposal tests. - **Blocked:** `make prepare` — NU1102: configured feeds could not restore `Microsoft.NETCore.App.Ref` version `10.0.13`; full SDK build and device tests were not reached. - **Blocked, including a fresh attempt after removing the shared fixture:** `./dotnet-local.sh build tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj -c Debug -v quiet` — NETSDK1147: the local Android workload is unavailable. The remaining interop tests have not been executed on Android.
The Android runtime uses the trimmable typemap managers, and the host-JVM test runs have been removed. Delete the remaining reflection managers, their generated helper tables, and the dormant JVM harness. Keep runtime-disposal coverage with non-reflection test doubles, remove unshipped API declarations and suppressions, and update test guidance. Context: #12894 Co-authored-by: Copilot App <[email protected]>

Summary
Stack: follows #12892 on
simonrozsival-document-trimmable-jni-interop(native stack #12891). The obsoleteTestInstrumentationcategory/name exclusions were removed in the parent branch.Validation