Skip to content

[tests] Cover AppCompat Java alias casts - #12609

Merged
jonathanpeppers merged 6 commits into
mainfrom
simonrozsival-java-alias-cast-parity
Sep 3, 2026
Merged

[tests] Cover AppCompat Java alias casts#12609
jonathanpeppers merged 6 commits into
mainfrom
simonrozsival-java-alias-cast-parity

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Summary

  • add a focused MSBuildDeviceIntegration AppCompat app for Java alias/cast parity
  • inflate real Toolbar and AppCompatImageButton views from XML, then verify most-derived binding selection and repeated peer identity
  • cover JavaCast<T> and JavaAs<T> for a concrete same-JNI-name alias, an AppCompat Java interface, managed-created peers, and a caller-directed closed generic JavaList<string> wrapper
  • run the same Release fixture with llvm-ir/CoreCLR, trimmable/CoreCLR, and trimmable/NativeAOT

No production change is included: the unchanged focused fixture is green in all three configurations.

Investigation

The existing NativeAOT skips all record the same historical startup failure:

  • InstallAndRunTests.CheckXamarinFormsAppDeploysAndAButtonWorks
  • BundleToolNoAbiSplitTests.InstallAndRun
  • DebuggingTest.ApplicationRunsWithoutDebugger

The reported exception converts AndroidX.AppCompat.Widget.AppCompatImageButton to AndroidX.AppCompat.Widget.Toolbar inside Xamarin.Forms.Platform.Android.FormsAppCompatActivity.OnCreate. The BundleTool/Debugging skips were introduced by b58d4606e6 (#10632); the InstallAndRun skip was added when that test gained NativeAOT coverage in 847c5fe161 (#10635).

The mismatch is stale/not reproducible for the current AppCompat binding and focused view-inflation path. The three broad Xamarin.Forms 5.0 skips remain unchanged: that exact legacy package is not present in the required offline package cache, and those tests also combine unrelated Forms, bundle-tool, and debugger behavior. They remain separate follow-up validation rather than being removed without executing the original fixtures.

Artifact evidence

For trimmable NativeAOT:

  • _UnnamedProject.TypeMap.dll contains an AppCompatImageButton alias holder with [0] targeting AndroidX.AppCompat.Widget.AppCompatImageButton and [1] targeting UnnamedProject.AppCompatImageButtonAlias
  • acw-map.txt maps both managed types to androidx.appcompat.widget.AppCompatImageButton
  • DEX contains the real androidx/appcompat/widget/AppCompatImageButton, androidx/appcompat/widget/Toolbar, and generated MainActivity
  • startup logs contain APPCOMPAT_ALIAS_CASTS_PASS

Test plan

  • llvm-ir / CoreCLR baseline: passed
  • trimmable / CoreCLR: passed
  • trimmable / NativeAOT: passed
  • final combined focused matrix: 3 passed
  • git diff --check

Add a focused AppCompat view-inflation fixture that exercises JavaCast and JavaAs across concrete aliases, Java interfaces, most-derived binding lookup, managed-created peers, repeated identity, and closed generic wrappers under llvm-ir, trimmable CoreCLR, and NativeAOT.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI lite review requested due to automatic review settings September 1, 2026 01:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 Medium severity · 1 Low severity

New issues introduced by this change (2)
SeverityFinding
Medium severitytests/​MSBuildDeviceIntegration/​Tests/​InstallAndRunTests.cs — ❌ error Testing — This doesn’t actually validate JavaAs&lt;T&gt; alias selection from an untyped…
Low severitytests/​MSBuildDeviceIntegration/​Tests/​InstallAndRunTests.cs — 💡 suggestion Testing — This adds a new hard-pinned Xamarin.AndroidX.AppCompat version…
What changed in this PR

Adds a new focused MSBuildDeviceIntegration device test that builds and runs an AppCompat app to validate Java alias/cast behavior (inflation + peer identity) across multiple typemap/runtime configurations, aiming to cover scenarios related to historical AppCompat cast mismatches under NativeAOT.

Changes:

  • Adds AppCompatJavaAliasCastsAndInflation test covering JavaCast<T> / JavaAs<T> behavior for inflated views, a same-JNI-name alias type, an AppCompat Java interface, managed-created peers, and a closed generic wrapper (JavaList<string>).
  • Runs the same Release test app under llvm-ir/CoreCLR, trimmable/CoreCLR, and trimmable/NativeAOT configurations.
  • Asserts successful startup via a specific logcat marker (APPCOMPAT_ALIAS_CASTS_PASS).
FileDescription
tests/​MSBuildDeviceIntegration/​Tests/​InstallAndRunTests.csAdds a new device integration test app exercising AppCompat inflation + alias/interface casts across runtime/typemap configurations.

Comment threadtests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs Outdated
Comment threadtests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Track the exact runtime-specific package, pre-clean stale installs, and force-stop and uninstall in a finally block. Exercise JavaAs alias selection from the untyped peer instead of its typed fast path.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsivaland others added 2 commits September 1, 2026 12:43
The legacy llvm-ir typemap can select the registered app alias as the canonical wrapper for the shared Java class. Accept both valid canonical wrappers while retaining peer identity and handle-constructor coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Allow legacy llvm-ir alias inflation without weakening the deterministic built-in-first assertion for trimmable type maps.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival

Copy link
Copy Markdown
MemberAuthor

/review

@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12609

Avoid disposing JavaCast results when they borrow the inflated view peer, while still releasing caller-created wrappers. Use direct adb cleanup so an uninstall build cannot overwrite run artifacts or mask the original test failure.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival

Copy link
Copy Markdown
MemberAuthor

/review

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Needs Changes

Findings: 0 errors · 1 warning · 0 suggestions

The realistic AppCompat inflation fixture and CoreCLR/NativeAOT type-map matrix are valuable. One coverage gap remains: the closed-generic JavaAs check only tests the already-typed identity fast path, not caller-directed generic wrapper resolution from an untyped JNI peer.

CI build 1576441 is still in progress; the completed checks inspected so far are passing and no failing check is currently reported.

Generated by Android PR Reviewer for #12609 · gpt56 · 159.2 AIC · ⌖ 8.88 AIC · ⊞ 25.7K
Comment /review to run again

Comment threadtests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12609

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Needs Changes

This pass: 1 error · 0 warnings · 0 suggestions

The focused AppCompat matrix is valuable, but the shared success marker is read from an uncleared logcat buffer, so one successful configuration can make later broken configurations pass. The existing unresolved closed-generic JavaAs coverage warning also still applies.

CI build 1576556 is in progress; the four completed checks are passing, with no failure currently reported.

Generated by Android PR Reviewer for #12609 · gpt56 · 189.7 AIC · ⌖ 16.9 AIC · ⊞ 25.7K
Comment /review to run again

Comment threadtests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Exercise closed-generic JavaAs construction from the untyped peer and clear logcat immediately before launch so each runtime configuration must emit its own success marker.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival

Copy link
Copy Markdown
MemberAuthor

/review

@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12609

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Needs Changes (CI pending)

No blocking correctness issues found. The focused fixture covers both value-manager APIs across llvm-ir/CoreCLR, trimmable/CoreCLR, and trimmable/NativeAOT, with isolated logcat signaling and careful JNI wrapper cleanup.

Findings: 0 errors · 0 warnings · 1 suggestion

CI build 1576631 is still running: CLA and Android Tools Tests Mac have passed; six jobs are in progress and the aggregate dotnet-android check is queued. Final merge readiness cannot be confirmed until the matrix completes.

Generated by Android PR Reviewer for #12609 · gpt56 · 174.3 AIC · ⌖ 8.85 AIC · ⊞ 25.7K
Comment /review to run again

Require (ReferenceEquals (imageView, alias), "JavaCast did not preserve the inflated alias peer.");
}
Require (
AppCompatImageButtonAlias.HandleConstructorCalls == 2,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 💡 Testing — This aggregate count couples the cast assertions to runtime-specific inflation behavior: llvm-ir can contribute one constructor call during inflation, while trimmable expects both calls from the two conversions. Please reset/capture the counter and assert the delta immediately after each JavaCast/JavaAs operation so a failure identifies the regressed activation path and an inflation-selection change does not obscure the cast result.

Rule: Test assertions must be specific

@jonathanpeppers
jonathanpeppers merged commit 5c74beb into mainSep 3, 2026
44 checks passed
@jonathanpeppers
jonathanpeppers deleted the simonrozsival-java-alias-cast-parity branch September 3, 2026 19:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simonrozsival@jonathanpeppers