[TrimmableTypeMap] Integration parity tests#10827
[TrimmableTypeMap] Integration parity tests#10827simonrozsival wants to merge 3 commits intodotnet:mainfrom
Conversation
7dafb1e to
810c7e5
Compare
There was a problem hiding this comment.
Pull request overview
Adds an integration test slice for TrimmableTypeMap scanner parity by building a real-user-types fixture assembly and running side-by-side comparisons against the legacy Cecil-based scanner, then wires these integration tests into the solution and Windows CI.
Changes:
- Add
Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTestsproject with scanner parity/comparison tests (Mono.Android + user fixture). - Add
UserTypesFixtureproject to produce a “real user types” assembly referencing Mono.Android/Java.Interop. - Wire the integration tests into
Xamarin.Android.sln, addInternalsVisibleTo, and run/publish results in the Windows CI steps.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj | New fixture project that compiles user-type edge cases and outputs DLL next to integration tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypes.cs | User-type fixture source covering component attributes, nested types, [Export], etc. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerComparisonTests.cs | New parity tests comparing legacy scanner output vs new SRM-based scanner across multiple dimensions. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/MockBuildEngine.cs | Minimal IBuildEngine stub for TaskLoggingHelper usage in scanner tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests.csproj | New xUnit integration test project referencing TrimmableTypeMap + Build.Tasks + fixture build output. |
| src/Xamarin.Android.Build.Tasks/Properties/AssemblyInfo.cs | Adds InternalsVisibleTo for the new integration test assembly. |
| build-tools/automation/yaml-templates/build-windows-steps.yaml | Runs/publishes TrimmableTypeMap integration tests in Windows CI. |
| Xamarin.Android.sln | Adds the new integration test and fixture projects to the solution. |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerComparisonTests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerComparisonTests.cs
Show resolved
Hide resolved
| [Fact] | ||
| public void ExactTypeMap_MonoAndroid () | ||
| { | ||
| var (legacy, _) = RunLegacyScanner (MonoAndroidAssemblyPath); | ||
| var (newEntries, _) = RunNewScanner (AllAssemblyPaths); | ||
| output.WriteLine ($"Legacy: {legacy.Count} entries, New: {newEntries.Count} entries"); | ||
| AssertTypeMapMatch (legacy, newEntries); | ||
| } |
There was a problem hiding this comment.
These integration tests repeatedly re-scan Mono.Android.dll (and often re-run the legacy scanner) in multiple [Fact]s, which can add significant CI time since the scans traverse thousands of types. Consider caching the legacy/new scan results in static Lazy fields (or using an xUnit class fixture/collection fixture) so the expensive scans run once per test class and the individual assertions reuse the results.
...osoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj
Show resolved
Hide resolved
| var le = legacyEntries!.OrderBy (e => e.ManagedName).First (); | ||
| var ne = newEntriesForName!.OrderBy (e => e.ManagedName).First (); | ||
|
|
||
| if (le.ManagedName != ne.ManagedName) | ||
| managedNameMismatches.Add ($"{javaName}: legacy='{le.ManagedName}' new='{ne.ManagedName}'"); | ||
|
|
||
| if (le.SkipInJavaToManaged != ne.SkipInJavaToManaged) | ||
| skipMismatches.Add ($"{javaName}: legacy.skip={le.SkipInJavaToManaged} new.skip={ne.SkipInJavaToManaged}"); |
There was a problem hiding this comment.
AssertTypeMapMatch() groups entries by JavaName but then compares only the first ManagedName within each group. This can miss alias differences where both scanners produce the same first managed type but differ in additional managed types for the same JNI name, which undermines the “ExactTypeMap” intent. Compare the full per-JavaName entry sets (including counts) rather than only the first element.
| var le = legacyEntries!.OrderBy (e => e.ManagedName).First (); | |
| var ne = newEntriesForName!.OrderBy (e => e.ManagedName).First (); | |
| if (le.ManagedName != ne.ManagedName) | |
| managedNameMismatches.Add ($"{javaName}: legacy='{le.ManagedName}' new='{ne.ManagedName}'"); | |
| if (le.SkipInJavaToManaged != ne.SkipInJavaToManaged) | |
| skipMismatches.Add ($"{javaName}: legacy.skip={le.SkipInJavaToManaged} new.skip={ne.SkipInJavaToManaged}"); | |
| var legacySorted = legacyEntries! | |
| .OrderBy (e => e.ManagedName) | |
| .ThenBy (e => e.SkipInJavaToManaged) | |
| .ToList (); | |
| var newSorted = newEntriesForName! | |
| .OrderBy (e => e.ManagedName) | |
| .ThenBy (e => e.SkipInJavaToManaged) | |
| .ToList (); | |
| var legacyManagedList = string.Join (", ", legacySorted.Select (e => e.ManagedName)); | |
| var newManagedList = string.Join (", ", newSorted.Select (e => e.ManagedName)); | |
| if (legacyManagedList != newManagedList) | |
| managedNameMismatches.Add ($"{javaName}: legacy='{legacyManagedList}' new='{newManagedList}'"); | |
| var legacySkipList = string.Join (", ", legacySorted.Select (e => $"{e.ManagedName}:{e.SkipInJavaToManaged}")); | |
| var newSkipList = string.Join (", ", newSorted.Select (e => $"{e.ManagedName}:{e.SkipInJavaToManaged}")); | |
| if (legacySkipList != newSkipList) | |
| skipMismatches.Add ($"{javaName}: legacy.skip={legacySkipList} new.skip={newSkipList}"); |
Add IntegrationTests and UserTypesFixture projects to Xamarin.Android.sln. Add CI step to run integration tests and publish results. Add InternalsVisibleTo for the integration test assembly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
810c7e5 to
d1debed
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Stacked on #10823.
Part of #10798
Scope
Review guide
Two commits — review in order: