From 5c60822607198a99577b94e89f3532f7827ef2a6 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Wed, 24 Jan 2024 16:27:00 +0100 Subject: [PATCH 1/4] PropertiesComparer: Add support for nested/cyclic references. --- .../Comparers/EqualMethodResult.cs | 9 +++-- .../Constraints/NUnitEqualityComparer.cs | 3 +- .../tests/Assertions/AssertThatTests.cs | 34 +++++++++++++++++++ .../tests/Constraints/EqualConstraintTests.cs | 7 ---- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/NUnitFramework/framework/Constraints/Comparers/EqualMethodResult.cs b/src/NUnitFramework/framework/Constraints/Comparers/EqualMethodResult.cs index d168570c40..4f8f93e639 100644 --- a/src/NUnitFramework/framework/Constraints/Comparers/EqualMethodResult.cs +++ b/src/NUnitFramework/framework/Constraints/Comparers/EqualMethodResult.cs @@ -23,8 +23,13 @@ internal enum EqualMethodResult ComparedEqual, /// - /// Method is appropriate and the items are consisdered different. + /// Method is appropriate and the items are considered different. /// - ComparedNotEqual + ComparedNotEqual, + + /// + /// Method is appropriate but the class has cyclic references. + /// + ComparisonPending } } diff --git a/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs b/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs index 0f0b83212a..04e97a498d 100644 --- a/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs +++ b/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs @@ -148,6 +148,7 @@ public bool AreEqual(object? x, object? y, ref Tolerance tolerance) throw new NotSupportedException($"Specified Tolerance not supported for instances of type '{GetType(x)}' and '{GetType(y)}'"); case EqualMethodResult.ComparedEqual: return true; + case EqualMethodResult.ComparisonPending: case EqualMethodResult.ComparedNotEqual: default: return false; @@ -170,7 +171,7 @@ internal EqualMethodResult AreEqual(object? x, object? y, ref Tolerance toleranc return EqualMethodResult.ComparedEqual; if (state.DidCompare(x, y)) - return EqualMethodResult.ComparedNotEqual; + return EqualMethodResult.ComparisonPending; EqualityAdapter? externalComparer = GetExternalComparer(x, y); diff --git a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs index 73de80735c..9fc09d9e06 100644 --- a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs +++ b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs @@ -526,5 +526,39 @@ public override string ToString() return $"{ValueA} {ValueB} '{ValueC}' [{Chained}]"; } } + + [Test] + public void AssertWithRecursiveClass() + { + LinkedList list1 = new(1, new(2, new(3))); + LinkedList list2 = new(1, new(2, new(3))); + + Assert.That(list1, Is.EqualTo(list2)); + } + + [Test] + public void AssertWithCyclicRecursiveClass() + { + LinkedList list1 = new(1); + LinkedList list2 = new(1); + + list1.Next = list1; + list2.Next = list2; + + Assert.That(list1, Is.EqualTo(list2)); + } + + private sealed class LinkedList + { + public LinkedList(int value, LinkedList? next = null) + { + Value = value; + Next = next; + } + + public int Value { get; } + + public LinkedList? Next { get; set; } + } } } diff --git a/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs b/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs index 3512e4ceb7..733166b2f7 100644 --- a/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs +++ b/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs @@ -1008,13 +1008,6 @@ public void TestSameValueDifferentTypeUsingGenericTypes() Assert.That(ex?.Message, Does.Contain(expectedMsg)); } - [Test] - public void SameValueAndTypeButDifferentReferenceShowNotShowTypeDifference() - { - var ex = Assert.Throws(() => Assert.That(Is.Zero, Is.EqualTo(Is.Zero))); - Assert.That(ex?.Message, Does.Contain(" Expected: <>" + NL + " But was: <>" + NL)); - } - [Test, TestCaseSource(nameof(DifferentTypeSameValueTestData))] public void SameValueDifferentTypeRegexMatch(object expected, object actual) { From eec5c3403dc14765cd9640ad2089df11de5616c1 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Wed, 24 Jan 2024 17:08:16 +0100 Subject: [PATCH 2/4] Make PropertiesComparer optional --- .../framework/Constraints/AnyOfConstraint.cs | 13 ++++++++ .../CollectionItemsEqualConstraint.cs | 13 ++++++++ .../framework/Constraints/EqualConstraint.cs | 14 +++++++- .../Constraints/NUnitEqualityComparer.cs | 23 ++++++++++++- .../Constraints/SomeItemsConstraint.cs | 14 ++++++++ .../tests/Assertions/AssertThatTests.cs | 32 ++++++++++--------- .../tests/Constraints/AnyOfConstraintTests.cs | 18 +++++++++++ .../DictionaryContainsValueConstraintTests.cs | 25 +++++++++++++-- 8 files changed, 133 insertions(+), 19 deletions(-) diff --git a/src/NUnitFramework/framework/Constraints/AnyOfConstraint.cs b/src/NUnitFramework/framework/Constraints/AnyOfConstraint.cs index 2de8ac02d8..bed68c18fb 100644 --- a/src/NUnitFramework/framework/Constraints/AnyOfConstraint.cs +++ b/src/NUnitFramework/framework/Constraints/AnyOfConstraint.cs @@ -130,6 +130,19 @@ public AnyOfConstraint Using(Func comparer) return this; } + /// + /// Enables comparing of instance properties. + /// + /// + /// This allows comparing classes that don't implement + /// without having to compare each property separately in own code. + /// + public AnyOfConstraint UsingPropertiesComparer() + { + _comparer.CompareProperties = true; + return this; + } + #endregion } } diff --git a/src/NUnitFramework/framework/Constraints/CollectionItemsEqualConstraint.cs b/src/NUnitFramework/framework/Constraints/CollectionItemsEqualConstraint.cs index f823174d74..e28afb29be 100644 --- a/src/NUnitFramework/framework/Constraints/CollectionItemsEqualConstraint.cs +++ b/src/NUnitFramework/framework/Constraints/CollectionItemsEqualConstraint.cs @@ -127,6 +127,19 @@ internal CollectionItemsEqualConstraint Using(EqualityAdapter adapter) return this; } + /// + /// Enables comparing of instance properties. + /// + /// + /// This allows comparing classes that don't implement + /// without having to compare each property separately in own code. + /// + public CollectionItemsEqualConstraint UsingPropertiesComparer() + { + _comparer.CompareProperties = true; + return this; + } + #endregion /// diff --git a/src/NUnitFramework/framework/Constraints/EqualConstraint.cs b/src/NUnitFramework/framework/Constraints/EqualConstraint.cs index 8b97128487..31eee03212 100644 --- a/src/NUnitFramework/framework/Constraints/EqualConstraint.cs +++ b/src/NUnitFramework/framework/Constraints/EqualConstraint.cs @@ -268,7 +268,6 @@ public EqualConstraint Ticks return this; } } - /// /// Flag the constraint to use the supplied IComparer object. /// @@ -346,6 +345,19 @@ public EqualConstraint Using(Func + /// Enables comparing of instance properties. + /// + /// + /// This allows comparing classes that don't implement + /// without having to compare each property separately in own code. + /// + public EqualConstraint UsingPropertiesComparer() + { + _comparer.CompareProperties = true; + return this; + } + #endregion #region Public Methods diff --git a/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs b/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs index 04e97a498d..4d05f2262a 100644 --- a/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs +++ b/src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs @@ -51,7 +51,6 @@ public sealed class NUnitEqualityComparer EquatablesComparer.Equal, EnumerablesComparer.Equal, EqualsComparer.Equal, - PropertiesComparer.Equal, }; /// @@ -65,6 +64,12 @@ public sealed class NUnitEqualityComparer /// private bool _compareAsCollection; + /// + /// If true, when a class does not implement + /// it will be compared property by property. + /// + private bool _compareProperties; + /// /// Comparison objects used in comparisons for some constraints. /// @@ -96,6 +101,16 @@ public bool IgnoreCase set => _caseInsensitive = value; } + /// + /// Gets and sets a flag indicating whether an instance properties + /// should be compared when determining equality. + /// + public bool CompareProperties + { + get => _compareProperties; + set => _compareProperties = value; + } + /// /// Gets and sets a flag indicating that arrays should be /// compared as collections, without regard to their shape. @@ -133,6 +148,7 @@ public bool CompareAsCollection #endregion #region Public Methods + /// /// Compares two objects for equality within a tolerance. /// @@ -195,6 +211,11 @@ internal EqualMethodResult AreEqual(object? x, object? y, ref Tolerance toleranc return result; } + if (_compareProperties) + { + return PropertiesComparer.Equal(x, y, ref tolerance, state, this); + } + if (tolerance.HasVariance) return EqualMethodResult.ToleranceNotSupported; diff --git a/src/NUnitFramework/framework/Constraints/SomeItemsConstraint.cs b/src/NUnitFramework/framework/Constraints/SomeItemsConstraint.cs index 068ca391b0..b3032d2cfb 100644 --- a/src/NUnitFramework/framework/Constraints/SomeItemsConstraint.cs +++ b/src/NUnitFramework/framework/Constraints/SomeItemsConstraint.cs @@ -127,6 +127,20 @@ public SomeItemsConstraint Using(IEqualityComparer comparer) return this; } + /// + /// Enables comparing of instance properties. + /// + /// + /// This allows comparing classes that don't implement + /// without having to compare each property separately in own code. + /// + public SomeItemsConstraint UsingPropertiesComparer() + { + CheckPrecondition(nameof(UsingPropertiesComparer)); + _equalConstraint.UsingPropertiesComparer(); + return this; + } + [MemberNotNull(nameof(_equalConstraint))] private void CheckPrecondition(string argument) { diff --git a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs index 9fc09d9e06..38c0069c05 100644 --- a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs +++ b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs @@ -377,13 +377,13 @@ public void AssertThatEqualsWithClassWithSomeToleranceAwareMembers() Assert.Multiple(() => { - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "1.1", zero), Is.EqualTo(instance)); - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.2, "1.1", zero), Is.Not.EqualTo(instance)); - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.2, "1.1", zero), Is.EqualTo(instance).Within(0.1)); - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "1.1", null), Is.Not.EqualTo(instance)); - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "2.2", zero), Is.Not.EqualTo(instance)); - Assert.That(new ClassWithSomeToleranceAwareMembers(1, 2.2, "1.1", zero), Is.Not.EqualTo(instance)); - Assert.That(new ClassWithSomeToleranceAwareMembers(2, 1.1, "1.1", zero), Is.Not.EqualTo(instance)); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "1.1", zero), Is.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.2, "1.1", zero), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.2, "1.1", zero), Is.EqualTo(instance).Within(0.1).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "1.1", null), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 1.1, "2.2", zero), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(1, 2.2, "1.1", zero), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new ClassWithSomeToleranceAwareMembers(2, 1.1, "1.1", zero), Is.Not.EqualTo(instance).UsingPropertiesComparer()); }); } @@ -415,12 +415,12 @@ public void AssertThatEqualsWithStructWithSomeToleranceAwareMembers() Assert.Multiple(() => { - Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.1, "1.1", SomeEnum.One), Is.EqualTo(instance)); - Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.2, "1.1", SomeEnum.One), Is.Not.EqualTo(instance)); - Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.2, "1.1", SomeEnum.One), Is.EqualTo(instance).Within(0.1)); - Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.1, "1.1", SomeEnum.Two), Is.Not.EqualTo(instance).Within(0.1)); - Assert.That(new StructWithSomeToleranceAwareMembers(1, 2.2, "1.1", SomeEnum.One), Is.Not.EqualTo(instance)); - Assert.That(new StructWithSomeToleranceAwareMembers(2, 1.1, "1.1", SomeEnum.One), Is.Not.EqualTo(instance)); + Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.1, "1.1", SomeEnum.One), Is.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.2, "1.1", SomeEnum.One), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.2, "1.1", SomeEnum.One), Is.EqualTo(instance).Within(0.1).UsingPropertiesComparer()); + Assert.That(new StructWithSomeToleranceAwareMembers(1, 1.1, "1.1", SomeEnum.Two), Is.Not.EqualTo(instance).Within(0.1).UsingPropertiesComparer()); + Assert.That(new StructWithSomeToleranceAwareMembers(1, 2.2, "1.1", SomeEnum.One), Is.Not.EqualTo(instance).UsingPropertiesComparer()); + Assert.That(new StructWithSomeToleranceAwareMembers(2, 1.1, "1.1", SomeEnum.One), Is.Not.EqualTo(instance).UsingPropertiesComparer()); }); } @@ -533,7 +533,8 @@ public void AssertWithRecursiveClass() LinkedList list1 = new(1, new(2, new(3))); LinkedList list2 = new(1, new(2, new(3))); - Assert.That(list1, Is.EqualTo(list2)); + Assert.That(list1, Is.Not.EqualTo(list2)); + Assert.That(list1, Is.EqualTo(list2).UsingPropertiesComparer()); } [Test] @@ -545,7 +546,8 @@ public void AssertWithCyclicRecursiveClass() list1.Next = list1; list2.Next = list2; - Assert.That(list1, Is.EqualTo(list2)); + Assert.That(list1, Is.Not.EqualTo(list2)); + Assert.That(list1, Is.EqualTo(list2).UsingPropertiesComparer()); } private sealed class LinkedList diff --git a/src/NUnitFramework/tests/Constraints/AnyOfConstraintTests.cs b/src/NUnitFramework/tests/Constraints/AnyOfConstraintTests.cs index 3c553ae775..7fed988e6a 100644 --- a/src/NUnitFramework/tests/Constraints/AnyOfConstraintTests.cs +++ b/src/NUnitFramework/tests/Constraints/AnyOfConstraintTests.cs @@ -63,5 +63,23 @@ public void MissingMember() { Assert.That(42, Is.Not.AnyOf(0, -1, 100)); } + + [Test] + public void ValidMemberUsingPropertiesComparer() + { + Assert.That(new XY(5, 12), Is.AnyOf(new XY(3, 4), new XY(5, 12)).UsingPropertiesComparer()); + } + + private sealed class XY + { + public XY(int x, int y) + { + X = x; + Y = y; + } + + public int X { get; } + public int Y { get; } + } } } diff --git a/src/NUnitFramework/tests/Constraints/DictionaryContainsValueConstraintTests.cs b/src/NUnitFramework/tests/Constraints/DictionaryContainsValueConstraintTests.cs index 3e1cb6f7e5..78c66f2777 100644 --- a/src/NUnitFramework/tests/Constraints/DictionaryContainsValueConstraintTests.cs +++ b/src/NUnitFramework/tests/Constraints/DictionaryContainsValueConstraintTests.cs @@ -29,14 +29,14 @@ public void FailsWhenValueIsMissing() } [Test] - public void SucceedsWhenValueIsPresentUsingContainKey() + public void SucceedsWhenValueIsPresentUsingContainValue() { var dictionary = new Dictionary { { "Hello", "World" }, { "Hola", "Mundo" } }; Assert.That(dictionary, Does.ContainValue("Mundo")); } [Test] - public void SucceedsWhenValueIsNotPresentUsingContainKey() + public void SucceedsWhenValueIsNotPresentUsingContainValue() { var dictionary = new Dictionary { { "Hello", "World" }, { "Hola", "Mundo" } }; Assert.That(dictionary, Does.Not.ContainValue("NotValue")); @@ -77,5 +77,26 @@ public void UsingIsHonored() Assert.That(dictionary, new DictionaryContainsValueConstraint("UNIVERSE").Using((x, y) => StringUtil.Compare(x, y, true))); } + + [Test] + public void UsingPropertiesComparerIsHonored() + { + var dictionary = new Dictionary { { "5", new(3, 4) }, { "13", new(5, 12) } }; + var value = new XY(5, 12); + Assert.That(dictionary, Does.Not.ContainValue(value)); + Assert.That(dictionary, Does.ContainValue(value).UsingPropertiesComparer()); + } + + private sealed class XY + { + public XY(double x, double y) + { + X = x; + Y = y; + } + + public double X { get; } + public double Y { get; } + } } } From 9c9dc02dbe1f0776762fdb43219c91832c9c3ea1 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Wed, 24 Jan 2024 18:15:07 +0100 Subject: [PATCH 3/4] Exclude indexers from PropertiesComparer --- .../Comparers/PropertiesComparer.cs | 4 ++- .../tests/Assertions/AssertThatTests.cs | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/NUnitFramework/framework/Constraints/Comparers/PropertiesComparer.cs b/src/NUnitFramework/framework/Constraints/Comparers/PropertiesComparer.cs index ea70b9185c..eb3beef0e6 100644 --- a/src/NUnitFramework/framework/Constraints/Comparers/PropertiesComparer.cs +++ b/src/NUnitFramework/framework/Constraints/Comparers/PropertiesComparer.cs @@ -1,6 +1,7 @@ // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt using System; +using System.Linq; using System.Reflection; namespace NUnit.Framework.Constraints.Comparers @@ -29,9 +30,10 @@ public static EqualMethodResult Equal(object x, object y, ref Tolerance toleranc } PropertyInfo[] properties = xType.GetProperties(BindingFlags.Instance | BindingFlags.Public); - if (properties.Length == 0 || properties.Length >= 32) + if (properties.Length == 0 || properties.Length >= 32 || properties.Any(p => p.GetIndexParameters().Length > 0)) { // We can't compare if there are no (or too many) properties. + // We also can't deal with indexer properties as we don't know the range of valid values. return EqualMethodResult.TypesNotSupported; } diff --git a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs index 38c0069c05..b07180eea2 100644 --- a/src/NUnitFramework/tests/Assertions/AssertThatTests.cs +++ b/src/NUnitFramework/tests/Assertions/AssertThatTests.cs @@ -546,7 +546,7 @@ public void AssertWithCyclicRecursiveClass() list1.Next = list1; list2.Next = list2; - Assert.That(list1, Is.Not.EqualTo(list2)); + Assert.That(list1, Is.Not.EqualTo(list2)); // Reference comparison Assert.That(list1, Is.EqualTo(list2).UsingPropertiesComparer()); } @@ -562,5 +562,28 @@ public LinkedList(int value, LinkedList? next = null) public LinkedList? Next { get; set; } } + + [Test] + public void EqualMemberWithIndexer() + { + var members = new Members("Hello", "World", "NUnit"); + var copy = new Members("Hello", "World", "NUnit"); + + Assert.That(members[1], Is.EqualTo("World")); + Assert.That(copy, Is.Not.EqualTo(members)); + Assert.That(() => Assert.That(copy, Is.EqualTo(members).UsingPropertiesComparer()), Throws.InstanceOf()); + } + + private sealed class Members + { + private readonly string[] _members; + + public Members(params string[] members) + { + _members = members; + } + + public string this[int index] => _members[index]; + } } } From ef7814979c7c3bf758644ea7cecaaba7072a64ad Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Wed, 24 Jan 2024 18:33:08 +0100 Subject: [PATCH 4/4] Restore accidentally deleted test --- .../tests/Constraints/EqualConstraintTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs b/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs index 733166b2f7..3512e4ceb7 100644 --- a/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs +++ b/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs @@ -1008,6 +1008,13 @@ public void TestSameValueDifferentTypeUsingGenericTypes() Assert.That(ex?.Message, Does.Contain(expectedMsg)); } + [Test] + public void SameValueAndTypeButDifferentReferenceShowNotShowTypeDifference() + { + var ex = Assert.Throws(() => Assert.That(Is.Zero, Is.EqualTo(Is.Zero))); + Assert.That(ex?.Message, Does.Contain(" Expected: <>" + NL + " But was: <>" + NL)); + } + [Test, TestCaseSource(nameof(DifferentTypeSameValueTestData))] public void SameValueDifferentTypeRegexMatch(object expected, object actual) {