Skip to content

Conversation

vcsjones
Copy link
Member

MLDsaAlgorithm does not implement equality nor have any tests. Let's fix that.

@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 15:48
@vcsjones vcsjones requested review from bartonjs and Copilot and removed request for Copilot June 17, 2025 15:49
Copy link
Contributor

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

Copy link
Contributor

@Copilot Copilot AI left a 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 adds value equality support to MLDsaAlgorithm and introduces tests to validate the new behavior.

  • Implemented IEquatable<MLDsaAlgorithm>, overrides for Equals, GetHashCode, ToString, and equality/inequality operators in MLDsaAlgorithm.
  • Added a DebuggerDisplay attribute for better debugging.
  • Created MLDsaAlgorithmTests.cs and wired it into both test projects to cover reference equality, Equals, hash code consistency, operator overloads, and ToString.

Reviewed Changes

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

File Description
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj Included MLDsaAlgorithmTests.cs in test compilation
src/libraries/Microsoft.Bcl.Cryptography/tests/Microsoft.Bcl.Cryptography.Tests.csproj Included MLDsaAlgorithmTests.cs in test compilation
src/libraries/Common/src/System/Security/Cryptography/MLDsaAlgorithm.cs Added equality members (IEquatable, overrides, operators), and DebuggerDisplay
src/libraries/Common/tests/System/Security/Cryptography/MLDsaAlgorithmTests.cs New tests for equality, GetHashCode, operators, and ToString
Comments suppressed due to low confidence (1)

src/libraries/Common/tests/System/Security/Cryptography/MLDsaAlgorithmTests.cs:58

  • Consider adding tests for operator == and != when comparing with null (e.g., Assert.True((MLDsaAlgorithm?)null == null) and Assert.True(MLDsaAlgorithm.MLDsa44 != null)), to validate null-handling behavior.
            AssertExtensions.TrueExpression(MLDsaAlgorithm.MLDsa87 != MLDsaAlgorithm.MLDsa44);

@vcsjones vcsjones merged commit 55abc37 into dotnet:main Jun 20, 2025
80 of 86 checks passed
@vcsjones vcsjones deleted the mldsaalgorithm-tests branch June 20, 2025 22:15
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2025
@vcsjones vcsjones added this to the 10.0.0 milestone Jul 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants