Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T>#5204
Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T>#5204
Conversation
There was a problem hiding this comment.
Code Review
Overall, this is a well-implemented addition that correctly extends to and . The PR follows existing patterns, correctly delegates to , handles AOT annotations, and includes good test coverage.
A few observations:
Issue: Missing .Using() fluent methods — API inconsistency
The IsEquivalentToAssertion<TCollection, TItem> for regular IEnumerable<T> exposes two fluent overrides:
public IsEquivalentToAssertion<TCollection, TItem> Using(IEqualityComparer<TItem> comparer) { ... }
public IsEquivalentToAssertion<TCollection, TItem> Using(Func<TItem?, TItem?, bool> equalityPredicate) { ... }The new MemoryIsEquivalentToAssertion<TMemory, TItem> lacks these methods entirely. This means users who want a fluent .Using() comparer on memory assertions must use the constructor overload — which breaks the consistency of the API and is a potential point of confusion since the same pattern works on collection assertions.
Suggested fix: Add the same .Using() methods to MemoryIsEquivalentToAssertion mirroring the collection version. This is especially important given the comparer is stored as a non-nullable IEqualityComparer<TItem> field, which implies mutation post-construction was anticipated.
Minor: Duplicated expression builder logic
Both IsEquivalentTo overloads in MemoryAssertionBase contain nearly-identical expression building blocks with an added flag pattern:
Context.ExpressionBuilder.Append(".IsEquivalentTo(");
var added = false;
if (expectedExpression != null) { ...; added = true; }
if (orderingExpression != null) { Context.ExpressionBuilder.Append(added ? ", " : ""); ... }
Context.ExpressionBuilder.Append(')');The second overload just adds a comparerExpression segment in the middle. This could be extracted to a small private helper method (e.g., AppendIsEquivalentToExpression) to reduce duplication and make future updates to expression formatting a single-place change.
Observation: metadata.Value == null is dead code for value types
TMemory is always Memory<T> or ReadOnlyMemory<T> — both structs — so metadata.Value == null can never be true. This null check is repeated across all other memory assertions in the file, so it's consistent with existing patterns, but worth noting it's unreachable.
Good
- Correct AOT annotation:
[RequiresUnreferencedCode]is applied only to the constructor that usesStructuralEqualityComparer(which requires reflection), and the matchingIsEquivalentTomethod overload. The custom-comparer path is correctly left unannotated. - Proper delegation to
CollectionEquivalencyChecker.AreEquivalent— no duplication of equivalency logic. - Public API snapshots updated for all three target frameworks.
- Test coverage is solid: unordered (default), ordered, different types (
int,byte), count mismatch, and value mismatch failure cases are all covered.
There was a problem hiding this comment.
Code Review
Overall, this is a well-implemented addition that correctly extends IsEquivalentTo to Memory<T> and ReadOnlyMemory<T>. The PR follows existing patterns, correctly delegates to CollectionEquivalencyChecker, handles AOT annotations, and includes good test coverage.
A few observations:
Issue: Missing .Using() fluent methods — API inconsistency
The IsEquivalentToAssertion<TCollection, TItem> for regular IEnumerable<T> exposes two fluent overrides:
public IsEquivalentToAssertion<TCollection, TItem> Using(IEqualityComparer<TItem> comparer) { ... }
public IsEquivalentToAssertion<TCollection, TItem> Using(Func<TItem?, TItem?, bool> equalityPredicate) { ... }The new MemoryIsEquivalentToAssertion<TMemory, TItem> lacks these methods. This means users who want fluent .Using() on memory assertions must use the constructor overload — breaking consistency with the collection assertion API. The comparer is even stored as a non-nullable IEqualityComparer<TItem> field, which suggests post-construction mutation was anticipated but not completed.
Suggested fix: Add the same .Using() methods to MemoryIsEquivalentToAssertion mirroring the collection version.
Minor: Duplicated expression builder logic
Both IsEquivalentTo overloads in MemoryAssertionBase contain nearly-identical expression building blocks with an added flag pattern. The second overload only adds a comparerExpression segment in the middle. This could be extracted to a small private helper to reduce duplication and make future formatting changes a single-place edit.
Observation: metadata.Value == null is dead code for value types
TMemory is always Memory<T> or ReadOnlyMemory<T> — both structs — so the null check can never be true. This is consistent with other memory assertions in the file, but it's unreachable.
Good
- Correct AOT annotation:
[RequiresUnreferencedCode]is applied only to the constructor that usesStructuralEqualityComparer(which requires reflection), and the matching method overload. The custom-comparer path is correctly left unannotated. - Proper delegation to
CollectionEquivalencyChecker.AreEquivalentwith no duplication of equivalency logic. - Public API snapshots updated for all three target frameworks.
- Test coverage is solid: unordered (default), ordered, different element types (
int,byte), count mismatch, and value mismatch failure cases are all covered.
Summary
IsEquivalentTomethods toMemoryAssertionBase<TMemory, TItem>, making them available on bothMemory<T>andReadOnlyMemory<T>assertionsCollectionOrdering, plus customIEqualityComparer<TItem>overloadCollectionEquivalencyCheckervia the adapter patternCloses #5171
Test plan
Memory<int>andReadOnlyMemory<byte>equivalency (ordered, unordered, failure cases)MemoryAssertionTestspass on net9.0