-
Couldn't load subscription status.
- Fork 5.2k
Skip System.Security.Cryptography.OpenSsl.Tests on Apple platforms and Android #119107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
src/libraries/System.Security.Cryptography.OpenSsl/tests/AssemblyInfo.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
There was a problem hiding this comment.
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 improves the developer experience by ensuring that System.Security.Cryptography.OpenSsl.Tests are properly skipped on platforms where OpenSSL is not supported, rather than running and failing. The change addresses the issue where tests would run locally on macOS and Android despite being excluded from CI builds.
Key changes:
- Adds assembly-level platform skipping for unsupported platforms using Xunit's
SkipOnPlatformattribute - Updates CI exclusion conditions to include MacCatalyst for consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj |
Added MacCatalyst to the IgnoreForCI condition |
src/libraries/System.Security.Cryptography.OpenSsl/tests/AssemblyInfo.cs |
New file with assembly-level SkipOnPlatform attribute for unsupported platforms |
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
|
@copilot the new file you added, AssemblyInfo.cs, is not in System.Security.Cryptography.OpenSsl.Tests.csproj so it won't be included as part of compilation. Can you please add this file to the top of the |
Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>
Added AssemblyInfo.cs to the compilation ItemGroup at the top of the source files section. The file is now properly included in the build. (12fdcdd) |
The System.Security.Cryptography.OpenSsl.Tests were running locally on macOS and Android when developers executed
./build.sh -rc release -s libs.tests -test, even though these tests will always fail due to the removal of OpenSSL compatibility from Apple platforms and Android.While the tests were already excluded from CI builds via the
IgnoreForCIcondition in the project file, they would still run (and fail) in local development environments on these platforms.This change adds an
AssemblyInfo.csfile to the OpenSsl tests project with an assembly-levelSkipOnPlatformattribute:Additionally, the
IgnoreForCIcondition in the project file has been updated to include MacCatalyst, ensuring consistency between the CI exclusions and the runtime test skipping.This ensures that when tests are run locally on any of these platforms, the entire test assembly is skipped with a clear message explaining why. The solution follows the same pattern used by other libraries in the codebase (such as System.IO.Pipes and System.Net.Security).
The tests continue to run normally on Linux and other supported platforms where OpenSSL is available.
Fixes #118865.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.