[tests] Enable compiler warnings-as-errors for Mono.Android tests - #12761
Open
simonrozsival wants to merge 2 commits into
Open
simonrozsival wants to merge 2 commits into
simonrozsival wants to merge 2 commits into
Conversation
Recurring warnings obscure new diagnostics in the Mono.Android device tests. Fix nullable contexts, unused declarations, member hiding, equality hashing, obsolete certificate loading, and Java compilation settings so a clean Debug/CoreCLR build is warning-free. Preserve intentional obsolete-API coverage with narrowly scoped, documented suppressions. Limit generated binding suppressions to the test project, where friend access exposes internal Mono.Android members. Replace avoidable reflection and describe constructor requirements to the trimmer without breaking the shared NativeAOT probe. Remove the compiler-warning exemption from the repository CI policy. Keep existing linker warnings visible and non-fatal for intentional trim-unsafe regression coverage and dependencies; IL2037 remains an error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ConstructorActivationTests.cs — Do not cast ConcurrentDictionary to non-generic IDictionary |
What changed in this PR
Enables compiler warnings-as-errors for Mono.Android tests while preserving intentional linker and obsolete-API coverage.
Changes:
- Adds nullable checks, warning suppressions, and an equality/hash-code regression test.
- Replaces avoidable reflection and improves trimming annotations.
- Updates test JAR compilation to use
javac --releaseand track project inputs.
Review finding: reflection activation tests will fail because ConcurrentDictionary is cast to non-generic IDictionary.
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/NonJavaObject.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/MyFragment.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/TrustManagerMarshallingTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System/TimeZoneTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System/ExceptionTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System/AppDomainTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Xml/XmlSerializer.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/SslTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/ProxyTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/NetworkInterfaces.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Linq/LinqExpressionTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TrimmableTypeMapTypeManagerTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TrimmableTypeMapRuntimeCoverageTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/RawInterfaceCollectionHolder.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaObjectExtensionsTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ExportTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ConstructorActivationTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/JnienvArrayMarshaling.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Android.OS/BundleTest.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Android.Graphics/NinePatchTests.cs | Updated as part of this pull request. |
| tests/Mono.Android-Tests/Mono.Android-Tests/.editorconfig | Updated as part of this pull request. |
| Directory.Build.props | Updated as part of this pull request. |
| build-tools/scripts/Jar.targets | Updated as part of this pull request. |
Comment on lines
+677
to
+678
| return cacheField.GetValue (null) as IDictionary ?? | ||
| throw new InvalidOperationException ("The reflection activation constructor cache is not a dictionary."); |
The NativeAOT lane reports IL3050 for the XML serialization regression. RequiresUnreferencedCode only describes trimming requirements; also declare RequiresDynamicCode without excluding the test. NativeAOT analysis inherits TreatWarningsAsErrors independently of ILLink. Keep its expected regression-suite and dependency diagnostics visible and non-fatal, matching the existing linker policy, while C# compiler diagnostics and IL2037 remain errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recurring warnings in the Mono.Android device tests obscure new diagnostics and keep the project on the repository's compiler-warning exemption list. Clean up those warnings and remove the exemption so CI, and local builds using
$(_AndroidTreatWarningsAsErrors), enforce compiler diagnostics.javac --release, including Java 8 version notation, and include build configuration files in incremental inputs.This enables C# compiler warnings-as-errors, not blanket enforcement for all build tools. Runtime regression tests deliberately exercise trim-unsafe and AOT-unsafe APIs and dependencies. Their linker and NativeAOT analysis warnings remain visible and non-fatal through
$(ILLinkTreatWarningsAsErrors)=falseand$(IlcTreatWarningsAsErrors)=false; IL2037 remains an error in both pipelines. NativeAOT needs its own setting because it otherwise inherits$(TreatWarningsAsErrors)independently of ILLink.Java compilation rationale and compatibility
The change in
build-tools/scripts/Jar.targetsaddresses the JDK 25 warning[options] location of system modules is not set in conjunction with -source 17. Both$(JavacSourceVersion)and$(JavacTargetVersion)resolve to 17 in the build verified here.-source 17 -target 17selects Java 17 syntax and class-file format, but still compiles against the host JDK's platform APIs.--release 17selects those same language and bytecode versions and also uses Java 17 platform API signatures. This removes the warning without disabling it and prevents accidental references to newer host-JDK APIs. The1.8special case translates the older version notation to--release 8.This is not an equivalent replacement for every configuration. It narrows the available Java platform APIs and uses
$(JavacTargetVersion)for the language level too, so the unconditional replacement no longer honors an independently configured$(JavacSourceVersion). Java SE release compatibility is also not an Android API-level check. The verified 17/17 configuration works, but preserving independent source/target settings in this shared helper would require retaining the original flags for that case rather than silently ignoring the source version; that guard is not part of this change.Validation
$(_AndroidTreatWarningsAsErrors)=true, and MSBuild-warnaserror: 0 warnings, 0 errors, including after the NativeAOT follow-up fix.$(TestsFlavor)=NativeAOTand$(PublishAot)=truesettings. After the fix, the NativeAOT package builds and installs with C# warnings-as-errors enabled; expected NativeAOT analysis warnings remain visible.TrimmingDefaultValueAttributepassed rather than being excluded.InterfaceInfo_EqualValuesHaveEqualHashCodesregression passed on an API 35 emulator in Release.Validation used the local .NET 11 RC2 SDK and covers CoreCLR/trimmable and NativeAOT execution. Mono was not executed locally; the SDK's default check rejects
$(UseMonoRuntime)=truewith NETSDK1242.