Uh oh!
There was an error while loading. Please reload this page.
Compile UpdateOptions_ServerCertificateContextProvided test on desktop only - #132277
Merged
Conversation
…p only The UpdateOptions_ServerCertificateContextProvided_DoesNotDisposeCallerContext test exercises SslStreamCertificateContext.Create with intermediate certificates, which pulls in a generic Dictionary instantiation whose IL body is stripped from the Apple-mobile CoreCLR ReadyToRun composite image, crashing System.Net.Security.Unit.Tests at startup before test discovery. Move the single failing test into a new SslAuthenticationOptionsTests.Desktop.cs partial class file and register it via an MSBuild Compile condition limited to desktop platforms (windows/unix/osx). The rest of the test suite keeps running on mobile. Fixes#131922 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
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. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the System.Net.Security.Unit.Tests unit test project to exclude a single SslAuthenticationOptionsTests test from non-desktop target frameworks by splitting it into a desktop-only source file and compiling that file conditionally.
Changes:
- Made
SslAuthenticationOptionsTestsapartialclass to allow splitting tests across platform-targeted source files. - Moved
UpdateOptions_ServerCertificateContextProvided_DoesNotDisposeCallerContextinto a newSslAuthenticationOptionsTests.Desktop.cs. - Added an MSBuild
Compileinclude conditioned onTargetPlatformIdentifier(windows/unix/osx) so only desktop builds compile the moved test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj | Conditionally compiles the new desktop-only test file for windows/unix/osx TFMs. |
| src/libraries/System.Net.Security/tests/UnitTests/SslAuthenticationOptionsTests.Desktop.cs | Introduces the desktop-only partial class file containing the moved [Fact] test. |
| src/libraries/System.Net.Security/tests/UnitTests/SslAuthenticationOptionsTests.cs | Converts the test class to partial and removes the moved test from the shared file. |
Contributor
Tagging subscribers to 'os-tvos': @vitek-karas, @kotlarmilos, @steveisok, @akoeplinger |
matouskozak
approved these changes
Aug 14, 2026
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 freeto 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.
Alternative to #131929 and #132125 for #131922.
System.Net.Security.Unit.Testscrashes during runtime init on Apple-mobile CoreCLR (e.g. tvOS), before test discovery runs. The single testUpdateOptions_ServerCertificateContextProvided_DoesNotDisposeCallerContextexercisesSslStreamCertificateContext.Createwith intermediate certificates, which pulls in a genericDictionaryinstantiation whose IL body is stripped from the Apple-mobile CoreCLR ReadyToRun composite image, crashing the whole app at startup. Because the app crashes before test discovery, per-test[Fact]/attribute-based skipping can't help.Approach
Rather than disabling the entire test project on non-desktop platforms (as the alternatives do), this keeps the rest of the suite running on mobile and only excludes the one problematic test from the mobile build:
SslAuthenticationOptionsTestsapartialclass.UpdateOptions_ServerCertificateContextProvided_DoesNotDisposeCallerContextverbatim into a newSslAuthenticationOptionsTests.Desktop.cs.Compilecondition limited to desktop platforms (windows/unix/osx). An MSBuild condition is used rather than a#ifbecause theTARGET_*C# defines aren't reliably present in this test project.TargetFrameworksand the existingIgnoreForCIline are untouched, so the app still builds and all other unit tests still run on mobile.Comparison to alternatives
PlatformNotSupportedExceptionbut not the tvOS R2R startup crash, which is unrelated to the test's certificate inputs.System.Net.Security.Unit.Testsproject in CI on non-desktop platforms, losing all mobile coverage of these unit tests.This PR is the most surgical option: it drops only the one test on mobile and preserves the remaining coverage.
Validation
Test-only change; no product code touched. Not built locally (test project requires the full libraries baseline build in this environment); the change is a mechanical move plus a build-condition addition. CI will exercise the desktop and mobile legs.
Note
This PR was generated with the assistance of GitHub Copilot.