Skip to content

Skip UnmanagedCallersOnlyAssociatedSourceType test on Android#129402

Open
MichalStrehovsky wants to merge 2 commits into
mainfrom
MichalStrehovsky-patch-1
Open

Skip UnmanagedCallersOnlyAssociatedSourceType test on Android#129402
MichalStrehovsky wants to merge 2 commits into
mainfrom
MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky

Copy link
Copy Markdown
Member

No description provided.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Copilot AI 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.

Pull request overview

This PR updates a NativeAOT smoke test to skip execution on Android due to a tracked platform issue.

Changes:

  • Add an Android-specific early-exit in UnmanagedCallersOnlyAssociatedSourceType.Run() with a link to the tracking issue.

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings June 15, 2026 05:51

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #129402

Holistic Assessment

Motivation: Justified — issue #129366 documents a persistent CI failure (5+ builds since June 8) where NativeAOT on Android trims UnmanagedCallersOnly exports that the test expects to find. Skipping the test to unblock CI while the root cause is tracked is standard practice.

Approach: Correct — the runtime platform check with early return 100 matches the established pattern in this test area (e.g., Ordering.cs lines 11-12 skip on WASM/Browser identically). These are non-xUnit smoke tests using the return 100 convention, so [ActiveIssue] attributes don't apply here.

Summary: ✅ LGTM. The change is minimal, matches established codebase patterns, correctly returns the "pass" exit code (100), and references the tracking issue. The compilation error from the first commit has been addressed.


Detailed Findings

✅ Correctness — Platform skip pattern is valid

The Main.cs test runner checks t() == 100 for pass/skip, and return 100 is correctly used. The early return prevents the test from reaching NativeLibrary.GetExport calls that would throw EntryPointNotFoundException on Android.

✅ Consistency — Matches sibling test patterns

The identical pattern (OperatingSystem.Is*()return 100) is used in Ordering.cs (WASM/Browser), and other NativeAOT smoke tests use OperatingSystem.IsAndroid() guards for platform-specific behavior (e.g., Exceptions.cs:167, Threading.cs:49).

✅ Issue tracking — Properly linked

The // ActiveIssue: comment references #129366, which is open and labeled disabled-test + blocking-clean-ci, ensuring visibility for re-enablement once the root cause is fixed.

Generated by Code Review for issue #129402 · ● 2.7M ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants