Skip to content

Target framework checks for System.Security.Crypto.Pkcs.Tests #118104

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Target framework checks for System.Security.Crypto.Pkcs.Tests #118104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...s/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/ContentEncryptionAlgorithmTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace System.Security.Cryptography.Pkcs.EnvelopedCmsTests.Tests
public static partial class ContentEncryptionAlgorithmTests
{
public static bool SupportsRc2 => PlatformSupport.IsRC2Supported;
public static bool SupportsRc4 => PlatformDetection.IsWindows;
public static bool SupportsRc4 => TargetFrameworkHelpers.TargetsWindows;
Copy link
Member

@bartonjs bartonjs Jul 28, 2025

Choose a reason for hiding this comment

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

I don't think we should take this change, at all.

A change which makes the VS runner see the same assets as the CLI runner, sure. But random extra hacking on the side, no. It will just leave behind a continual specter of "should this use TargetsWindows, or IsWindows?".

public static bool DoesNotSupportRc4 => !SupportsRc4;

[ConditionalFact(nameof(SupportsRc2))]
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/GeneralTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace System.Security.Cryptography.Pkcs.EnvelopedCmsTests.Tests
public static partial class GeneralTests
{
public static bool SupportsDiffieHellman { get; } = KeyAgreeRecipientInfoTests.SupportsDiffieHellman;
public static bool SupportsRsaOaepCerts => PlatformDetection.IsWindows;
public static bool SupportsRsaOaepCerts => TargetFrameworkHelpers.TargetsWindows;

[Fact]
public static void DefaultEncryptionAlgorithm()
Expand Down
2 changes: 1 addition & 1 deletion ...raries/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/KeyAgreeRecipientInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static partial class KeyAgreeRecipientInfoTests
internal static readonly AlgorithmIdentifier TripleDesAlgId =
new AlgorithmIdentifier(new Oid(Oids.TripleDesCbc, null));

public static bool SupportsDiffieHellman => PlatformDetection.IsWindows;
public static bool SupportsDiffieHellman => TargetFrameworkHelpers.TargetsWindows;
public static bool DoesNotSupportDiffieHellman => !SupportsDiffieHellman;

[ConditionalFact(nameof(DoesNotSupportDiffieHellman))]
Expand Down
2 changes: 1 addition & 1 deletion ...em.Security.Cryptography.Pkcs/tests/EnvelopedCms/KeyTransRecipientInfoRsaOaepCertTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace System.Security.Cryptography.Pkcs.EnvelopedCmsTests.Tests
{
public static partial class KeyTransRecipientInfoRsaOaepCertTests
{
public static bool SupportsRsaOaepCerts => PlatformDetection.IsWindows;
public static bool SupportsRsaOaepCerts => TargetFrameworkHelpers.TargetsWindows;
public static bool DoesNotSupportRsaOaepCerts => !SupportsRsaOaepCerts;

[ConditionalFact(nameof(SupportsRsaOaepCerts))]
Expand Down
2 changes: 1 addition & 1 deletion ...Security.Cryptography.Pkcs/tests/EnvelopedCms/KeyTransRecipientInfoRsaPaddingModeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace System.Security.Cryptography.Pkcs.EnvelopedCmsTests.Tests
{
public static partial class KeyTransRecipientInfoRsaPaddingModeTests
{
public static bool SupportsRsaOaepCerts => PlatformDetection.IsWindows;
public static bool SupportsRsaOaepCerts => TargetFrameworkHelpers.TargetsWindows;

[Theory]
[MemberData(nameof(TestKeyTransEncryptedKey_RsaAlgorithmTypes))]
Expand Down
6 changes: 6 additions & 0 deletions ...es/System.Security.Cryptography.Pkcs/tests/System.Security.Cryptography.Pkcs.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@
<Compile Include="Pkcs8PrivateKeyInfoTests.cs" />
<Compile Include="PrivateKeyHelpers.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' or '$(TargetPlatformIdentifier)' == 'windows'">
Copy link

Copilot AI Jul 27, 2025

Choose a reason for hiding this comment

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

The condition should use lowercase 'windows' for consistency with MSBuild conventions. Consider using '$(TargetPlatformIdentifier)' == 'Windows' with proper casing, or verify that the lowercase 'windows' is intentional and documented.

Suggested change
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' or '$(TargetPlatformIdentifier)' == 'windows'">
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' or '$(TargetPlatformIdentifier)' == 'Windows'">

Copilot uses AI. Check for mistakes.
<Compile Include="TargetFrameworkHelpers.Windows.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework' and '$(TargetPlatformIdentifier)' != 'windows'">
Copy link

Copilot AI Jul 27, 2025

Choose a reason for hiding this comment

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

Same casing issue as above - the condition should use consistent casing for 'windows'. This condition is the logical inverse of line 79, so both should use the same casing convention.

Copilot uses AI. Check for mistakes.
<Compile Include="TargetFrameworkHelpers.AnyOS.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\src\System.Security.Cryptography.Pkcs.csproj" />
</ItemGroup>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Security.Cryptography.Pkcs/tests/TargetFrameworkHelpers.AnyOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.Security.Cryptography.Pkcs.Tests
{
internal class TargetFrameworkHelpers
{
internal static bool TargetsWindows => false;
}
}
10 changes: 10 additions & 0 deletions src/libraries/System.Security.Cryptography.Pkcs/tests/TargetFrameworkHelpers.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.Security.Cryptography.Pkcs.Tests
{
internal class TargetFrameworkHelpers
{
internal static bool TargetsWindows => true;
}
}
Loading