Skip to content

Exclude desktop-only Security test from mobile builds - #132467

Draft
davidnguyen-tech wants to merge 1 commit into
dotnet:mainfrom
davidnguyen-tech:davidnguyen-tech-mobile-ci-triage
Draft

Exclude desktop-only Security test from mobile builds#132467
davidnguyen-tech wants to merge 1 commit into
dotnet:mainfrom
davidnguyen-tech:davidnguyen-tech-mobile-ci-triage

Conversation

@davidnguyen-tech

Copy link
Copy Markdown
Member

Summary

  • exclude SslAuthenticationOptionsTests.Desktop.cs from all mobile builds
  • use TargetsMobile because tvOS builds this project as net11.0-unix, making the previous TargetPlatformIdentifier == 'unix' condition ineffective

Follow-up to #131922 and #132277.

Validation

  • ./build.sh clr+libs -rc release
  • System.Net.Security.Unit.Tests: 134 total, 0 failed, 3 skipped
  • MSBuild compile matrix: excluded on tvOS, Android, and browser; included on Linux, macOS, and Windows
  • fixed tvOS build binlog contains no SslAuthenticationOptionsTests.Desktop.cs compile input

Note

This pull request was prepared with GitHub Copilot assistance and reviewed by the submitting developer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c120fa18-835a-418d-ae4c-d5888e699e7c
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@davidnguyen-tech

Copy link
Copy Markdown
Member Author

/azp run runtime

Note

This pipeline trigger was posted by GitHub Copilot at the user's request.

@davidnguyen-tech

Copy link
Copy Markdown
Member Author

/azp run runtime-extra-platforms

Note

This pipeline trigger was posted by GitHub Copilot at the user's request.

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

1 similar comment
@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

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 the build condition for System.Net.Security.Unit.Tests so a desktop-only test file (SslAuthenticationOptionsTests.Desktop.cs) is excluded from mobile (and related) build legs, preventing it from being compiled where it can trigger startup failures.

Changes:

  • Switch the SslAuthenticationOptionsTests.Desktop.cs Compile condition from TargetPlatformIdentifier-based checks to TargetsMobile-based gating.
  • Ensure tvOS builds (which can build this project as net11.0-unix) still treat the build as “mobile” for exclusion purposes.

@davidnguyen-tech

Copy link
Copy Markdown
Member Author

/azp run runtime-extra-platforms

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@@ -135,7 +135,7 @@
</ItemGroup>

<!-- Compiled on desktop only: crashes Apple-mobile CoreCLR at startup due to R2R IL-body stripping. -->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This justification does not make sense. R2R IL-body stripping should not be stripping IL for generic instantiations.

Is this just trying to hide a real product bug in R2R?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a good point. If it's a bug in R2R, I think we should keep #131922 opened as a KBE until it's properly fixed, instead of closing it after just disabling the failing test. This way we don't block the CI while still tracking the bug.

cc @rzikm

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.

3 participants