-
-
Notifications
You must be signed in to change notification settings - Fork 108
Fix IsEquivalentTo to respect IEquatable<T> for value types like Vector2 #4263
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: thomhurst <[email protected]>
Co-authored-by: thomhurst <[email protected]>
SummaryFixes IsEquivalentTo assertion to respect IEquatable for value types like Vector2 that have indexed properties. Critical IssuesNone found ✅ Suggestions1. AOT Annotation Missing on ImplementsSelfEquatableLocation: TUnit.Assertions/Conditions/Helpers/TypeHelper.cs:97 The new ImplementsSelfEquatable method calls type.GetInterfaces() which requires reflection. It should have the [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] annotation on its parameter for AOT compatibility (per TUnit rule #5). The existing UnconditionalSuppressMessage on line 63 of IsPrimitiveOrWellKnownType may not propagate properly to the called method. 2. Test Comment Has Wrong Issue NumberLocation: TUnit.Assertions.Tests/CollectionStructuralEquivalenceTests.cs:191 The comment references issue #3722 but the PR description says this fixes #4081. The comment should be updated for consistency. Technical AnalysisThe changes correctly address the root cause:
The fix aligns with TUnit principles:
Verdict💬 COMMENT - Excellent fix overall. The AOT annotation is a minor improvement that would make the code more robust for Native AOT scenarios per TUnit mandatory rules. |
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 fixes a bug where IsEquivalentTo throws "Parameter count mismatch" when comparing collections of types with indexed properties, such as Vector2 or Uri. The issue occurred because the structural equality comparer attempted to reflect over indexer properties that require parameters.
Key changes:
- Value types implementing
IEquatable<T>now useEqualityComparer<T>.Defaultinstead of structural comparison - Indexed properties are filtered out during structural comparison to prevent reflection errors
- Test coverage added for Vector2 (value type) and Uri (reference type) scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| TUnit.Assertions/Conditions/Helpers/TypeHelper.cs | Added logic to detect value types implementing IEquatable for themselves and route them to use EqualityComparer.Default instead of structural comparison. Includes proper AOT/trimming annotations. |
| TUnit.Assertions/Conditions/Helpers/ReflectionHelper.cs | Modified GetMembersToCompare to filter out indexed properties (properties with parameters) that would cause reflection errors during structural comparison. |
| TUnit.Assertions.Tests/CollectionStructuralEquivalenceTests.cs | Added comprehensive test cases for Vector2 and Uri collection equivalence, covering both positive (equivalent) and negative (not equivalent) scenarios. |
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests for issue #3722: IsEquivalentTo does not respect IEquatable{T} for types like Vector2 |
Copilot
AI
Jan 8, 2026
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.
The issue number in the comment is incorrect. The PR description indicates this fixes issue #4081, but the comment references #3722.
| /// Tests for issue #3722: IsEquivalentTo does not respect IEquatable{T} for types like Vector2 | |
| /// Tests for issue #4081: IsEquivalentTo does not respect IEquatable{T} for types like Vector2 |
| foreach (var prop in properties) | ||
| { | ||
| if (prop.GetIndexParameters().Length == 0) | ||
| { | ||
| members.Add(prop); | ||
| } | ||
| } |
Copilot
AI
Jan 8, 2026
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.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var iface in type.GetInterfaces()) | ||
| { | ||
| if (iface.IsGenericType | ||
| && iface.GetGenericTypeDefinition() == typeof(IEquatable<>) | ||
| && iface.GenericTypeArguments[0] == type) | ||
| { | ||
| return true; | ||
| } | ||
| } |
Copilot
AI
Jan 8, 2026
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.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
IsEquivalentTothrows "Parameter count mismatch" when comparing collections of value types likeVector2that have indexed properties. The structural comparer was attempting to reflect over the indexerthis[int]and failing.Changes
TypeHelper.cs: Value types implementing
IEquatable<T>for themselves now useEqualityComparer<T>.Defaultinstead of structural comparison. Limited to value types to preserve structural comparison for records/classes with collection properties.ReflectionHelper.cs: Filter out indexed properties from structural comparison to prevent reflection errors on types with indexers.
Tests: Added test cases for
Vector2andUricollection equivalence.Original prompt
This section details on the original issue you should resolve
<issue_title>[Bug]:
IsEquivalentTodoes not respectIEquatable<T></issue_title><issue_description>### Description
When using
await Assert.That(array).IsEquivalentTo(array2);on a type which implement
IEquatable<T>, TUnit does not use theIEquatable<T>to check item is equal or not.Expected Behavior
Test should pass.
Actual Behavior
Test failed.
Steps to Reproduce
TUnit Version
1.5.60
.NET Version
.NET9.0
Operating System
Windows
IDE / Test Runner
JetBrains Rider
Error Output / Stack Trace
Additional Context
If I change
await Assert.That(array).IsEquivalentTo(array2);to
await Assert.That(array).IsEquivalentTo(array2,EqualityComparer<Vector2>.Default);test will pass.
There are a lot of type (
Uri,CultureInfo,Vector3,Matrix3x2,etc...) has this issue, not justVector2.Additionally,
IComparable<T>should be considered as well.IDE-Specific Issue?
dotnet testordotnet run, not just in my IDE</issue_description>Comments on the Issue (you are @copilot in this section)
IsEquivalentTodoes not respectIEquatable<T>#4081💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.