feat: changed CompareStrategy to return CompareResult

* Ensure that the correct diff is returned. * Use records. * Move kind to beginning of text. * Fix compilation error and test for custom diff. * Update src/AngleSharp.Diffing/Core/CompareResult.cs Co-authored-by: Egil Hansen <[email protected]> * Update src/AngleSharp.Diffing/Core/Comparison.cs Co-authored-by: Egil Hansen <[email protected]> * Update src/AngleSharp.Diffing/Core/AttributeComparison.cs Co-authored-by: Egil Hansen <[email protected]> * 1. Rename CompareResultDecision 2. Move method inside type. * Rename method. * Get rid of StylesOrder * More records. * refactor: move IsExternalInit to folder matching namespace * add comment to explain use of ReferenceEquals * Move types into two files * simplify attrdiff object hierarchi * remove diff type check * clean up using statements * refactor: IsSameOrSkip as property * refactor: normalize tests * feat: add CommentComparer tests * nodes do not have different closing tags, elements do, so switch to using ElementDiff when comparing elements * fix: override ComparisonSource.ToString for better output * fix: use unspecified when no diff is returned * override ToString in AttributeComparisonSource * bump version number --------- Co-authored-by: Egil Hansen <[email protected]>
AngleSharp · Jun 29, 2023 · 7a0c5e1 · 7a0c5e1
1 parent e514267
commit 7a0c5e1
Show file tree

Hide file tree

Showing 49 changed files with 688 additions and 345 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 0.18.2
+
+- Changed `CompareStrategy` such that it now can control the `IDiff` type that should be returned in case a difference is found in a comparison. This allows a comparer to embed additional context in the `IDiff` object. By [@SebastianStehle](https://github.com/SebastianStehle).
+- Changed `ElementComparer` to skip comparing two nodes of different types. By [@SebastianStehle](https://github.com/SebastianStehle).
+
 # 0.18.1
 
 - Fixed element comparer such that it can strictly check if the closing tags in the source markup is the same.

diff --git a/src/AngleSharp.Diffing.Tests/Core/HtmlDifferenceEngineTest.cs b/src/AngleSharp.Diffing.Tests/Core/HtmlDifferenceEngineTest.cs
@@ -103,17 +103,46 @@ public void WhenNodesAreDifferentADiffIsReturned()
         var results = sut.Compare(nodes, nodes).ToList();
 
         results.Count.ShouldBe(3);
-        results[0].ShouldBeOfType<NodeDiff>().ShouldSatisfyAllConditions(
+        results[0].ShouldBeAssignableTo<NodeDiff>().ShouldSatisfyAllConditions(
             diff => diff.Control.Node.NodeName.ShouldBe("P"),
             diff => diff.Result.ShouldBe(DiffResult.Different),
             diff => diff.Target.ShouldBe(DiffTarget.Element)
         );
-        results[1].ShouldBeOfType<NodeDiff>().ShouldSatisfyAllConditions(
+        results[1].ShouldBeAssignableTo<NodeDiff>().ShouldSatisfyAllConditions(
             diff => diff.Control.Node.NodeName.ShouldBe("#comment"),
             diff => diff.Result.ShouldBe(DiffResult.Different),
             diff => diff.Target.ShouldBe(DiffTarget.Comment)
         );
-        results[2].ShouldBeOfType<NodeDiff>().ShouldSatisfyAllConditions(
+        results[2].ShouldBeAssignableTo<NodeDiff>().ShouldSatisfyAllConditions(
+            diff => diff.Control.Node.NodeName.ShouldBe("#text"),
+            diff => diff.Result.ShouldBe(DiffResult.Different),
+            diff => diff.Target.ShouldBe(DiffTarget.Text)
+        );
+    }
+
+    [Fact(DisplayName = "When matched control/test nodes are different, a custom diff is returned")]
+    public void WhenNodesAreDifferentADiffIsReturnedWithCustomDiff()
+    {
+        var nodes = ToNodeList("<p></p><!--comment-->textnode");
+        var sut = CreateHtmlDiffer(
+            nodeMatcher: OneToOneNodeListMatcher,
+            nodeFilter: NoneNodeFilter,
+            nodeComparer: DiffResultCustomNodeComparer);
+
+        var results = sut.Compare(nodes, nodes).ToList();
+
+        results.Count.ShouldBe(3);
+        results[0].ShouldBeOfType<CustomNodeDiff>().ShouldSatisfyAllConditions(
+            diff => diff.Control.Node.NodeName.ShouldBe("P"),
+            diff => diff.Result.ShouldBe(DiffResult.Different),
+            diff => diff.Target.ShouldBe(DiffTarget.Element)
+        );
+        results[1].ShouldBeOfType<CustomNodeDiff>().ShouldSatisfyAllConditions(
+            diff => diff.Control.Node.NodeName.ShouldBe("#comment"),
+            diff => diff.Result.ShouldBe(DiffResult.Different),
+            diff => diff.Target.ShouldBe(DiffTarget.Comment)
+        );
+        results[2].ShouldBeOfType<CustomNodeDiff>().ShouldSatisfyAllConditions(
             diff => diff.Control.Node.NodeName.ShouldBe("#text"),
             diff => diff.Result.ShouldBe(DiffResult.Different),
             diff => diff.Target.ShouldBe(DiffTarget.Text)
@@ -237,6 +266,30 @@ public void WhenMatchedAttrsAreDiffAttrDiffIsReturned()
         );
     }
 
+    [Fact(DisplayName = "When matched control/test attributes are different, a diff is returned with custom diff")]
+    public void WhenMatchedAttrsAreDiffAttrDiffIsReturnedWithCustomDiff()
+    {
+        var nodes = ToNodeList(@"<p id=""foo""></p>");
+
+        var sut = CreateHtmlDiffer(
+            nodeMatcher: OneToOneNodeListMatcher,
+            nodeFilter: NoneNodeFilter,
+            nodeComparer: SameResultNodeComparer,
+            attrMatcher: AttributeNameMatcher,
+            attrFilter: NoneAttrFilter,
+            attrComparer: DiffResultCustomAttrComparer);
+
+        var results = sut.Compare(nodes, nodes).ToList();
+
+        results.Count.ShouldBe(1);
+        results[0].ShouldBeOfType<CustomAttrDiff>().ShouldSatisfyAllConditions(
+            diff => diff.Control.Attribute.Name.ShouldBe("id"),
+            diff => diff.Test.Attribute.Name.ShouldBe("id"),
+            diff => diff.Result.ShouldBe(DiffResult.Different),
+            diff => diff.Target.ShouldBe(DiffTarget.Attribute)
+        );
+    }
+
     [Fact(DisplayName = "When matched control/test attributes are the same, no diffs are returned")]
     public void WhenMatchedAttrsAreSameNoDiffIsReturned()
     {
@@ -268,11 +321,11 @@ public void WhenBothTestAndControlHaveChildNodesTheseAreCompared()
         var results = sut.Compare(nodes, nodes).ToList();
 
         results.Count.ShouldBe(5);
-        results[0].ShouldBeOfType<NodeDiff>().Control.Node.NodeName.ShouldBe("MAIN");
-        results[1].ShouldBeOfType<NodeDiff>().Control.Node.NodeName.ShouldBe("H1");
-        results[2].ShouldBeOfType<NodeDiff>().Control.Node.NodeValue.ShouldBe("foobar");
-        results[3].ShouldBeOfType<NodeDiff>().Control.Node.NodeName.ShouldBe("P");
-        results[4].ShouldBeOfType<NodeDiff>().Control.Node.NodeName.ShouldBe("#text");
+        results[0].ShouldBeAssignableTo<NodeDiff>().Control.Node.NodeName.ShouldBe("MAIN");
+        results[1].ShouldBeAssignableTo<NodeDiff>().Control.Node.NodeName.ShouldBe("H1");
+        results[2].ShouldBeAssignableTo<NodeDiff>().Control.Node.NodeValue.ShouldBe("foobar");
+        results[3].ShouldBeAssignableTo<NodeDiff>().Control.Node.NodeName.ShouldBe("P");
+        results[4].ShouldBeAssignableTo<NodeDiff>().Control.Node.NodeName.ShouldBe("#text");
     }
 
     [Theory(DisplayName = "When only one of the control or test node in a comparison has child nodes, a missing/unexpected diff is returned")]
@@ -288,7 +341,7 @@ public void OnlyOnePartHasChildNodes(string control, string test, Type expectedD
         var results = sut.Compare(ToNodeList(control), ToNodeList(test)).ToList();
 
         results.Count.ShouldBe(2);
-        results[0].ShouldBeOfType<NodeDiff>();
+        results[0].ShouldBeAssignableTo<NodeDiff>();
         results[1].ShouldBeOfType(expectedDiffType);
     }
 
@@ -309,8 +362,8 @@ public void ComparisonSourcesHaveCorrectType()
 
         results.Count.ShouldBe(2);
 
-        results[0].ShouldBeOfType<NodeDiff>().Control.SourceType.ShouldBe(ComparisonSourceType.Control);
-        results[0].ShouldBeOfType<NodeDiff>().Test.SourceType.ShouldBe(ComparisonSourceType.Test);
+        results[0].ShouldBeAssignableTo<NodeDiff>().Control.SourceType.ShouldBe(ComparisonSourceType.Control);
+        results[0].ShouldBeAssignableTo<NodeDiff>().Test.SourceType.ShouldBe(ComparisonSourceType.Test);
         results[1].ShouldBeOfType<AttrDiff>().Control.SourceType.ShouldBe(ComparisonSourceType.Control);
         results[1].ShouldBeOfType<AttrDiff>().Test.SourceType.ShouldBe(ComparisonSourceType.Test);
     }
@@ -349,14 +402,14 @@ public void Test2()
     }
 
     [Theory(DisplayName = "When comparer returns SkipChildren flag from an element comparison, child nodes are not compared")]
-    [InlineData(CompareResult.Same | CompareResult.SkipChildren)]
-    [InlineData(CompareResult.Skip | CompareResult.SkipChildren)]
-    public void Test3(CompareResult compareResult)
+    [InlineData(CompareDecision.Same | CompareDecision.SkipChildren)]
+    [InlineData(CompareDecision.Skip | CompareDecision.SkipChildren)]
+    public void Test3(CompareDecision decision)
     {
         var sut = CreateHtmlDiffer(
             nodeMatcher: OneToOneNodeListMatcher,
             nodeFilter: NoneNodeFilter,
-            nodeComparer: c => c.Control.Node.NodeName == "P" ? compareResult : throw new Exception("NODE COMPARER SHOULD NOT BE CALLED ON CHILD NODES"),
+            nodeComparer: c => c.Control.Node.NodeName == "P" ? new CompareResult(decision) : throw new Exception("NODE COMPARER SHOULD NOT BE CALLED ON CHILD NODES"),
             attrMatcher: AttributeNameMatcher,
             attrFilter: NoneAttrFilter,
             attrComparer: SameResultAttrComparer
@@ -368,14 +421,14 @@ public void Test3(CompareResult compareResult)
     }
 
     [Theory(DisplayName = "When comparer returns SkipAttributes flag from an element comparison, attributes are not compared")]
-    [InlineData(CompareResult.Same | CompareResult.SkipAttributes)]
-    [InlineData(CompareResult.Skip | CompareResult.SkipAttributes)]
-    public void Test4(CompareResult compareResult)
+    [InlineData(CompareDecision.Same | CompareDecision.SkipAttributes)]
+    [InlineData(CompareDecision.Skip | CompareDecision.SkipAttributes)]
+    public void Test4(CompareDecision decision)
     {
         var sut = CreateHtmlDiffer(
             nodeMatcher: OneToOneNodeListMatcher,
             nodeFilter: NoneNodeFilter,
-            nodeComparer: c => compareResult,
+            nodeComparer: c => new CompareResult(decision),
             attrMatcher: AttributeNameMatcher,
             attrFilter: NoneAttrFilter,
             attrComparer: SameResultAttrComparer
@@ -411,6 +464,7 @@ private static IEnumerable<Comparison> OneToOneNodeListMatcher(
     #region NodeComparers
     private static CompareResult SameResultNodeComparer(Comparison comparison) => CompareResult.Same;
     private static CompareResult DiffResultNodeComparer(Comparison comparison) => CompareResult.Different;
+    private static CompareResult DiffResultCustomNodeComparer(Comparison comparison) => CompareResult.FromDiff(new CustomNodeDiff(comparison));
     #endregion
 
     #region AttributeMatchers
@@ -452,5 +506,22 @@ private static Func<AttributeComparisonSource, FilterDecision> SpecificAttrFilte
     #region AttributeComparers
     public static CompareResult SameResultAttrComparer(AttributeComparison comparison) => CompareResult.Same;
     public static CompareResult DiffResultAttrComparer(AttributeComparison comparison) => CompareResult.Different;
+    public static CompareResult DiffResultCustomAttrComparer(AttributeComparison comparison) => CompareResult.FromDiff(new CustomAttrDiff(comparison));
+    #endregion
+
+    #region CustomDiff
+    public record CustomNodeDiff : NodeDiff
+    {
+        public CustomNodeDiff(in Comparison comparison) : base(comparison)
+        {
+        }
+    }
+
+    public record CustomAttrDiff : AttrDiff
+    {
+        public CustomAttrDiff(in AttributeComparison comparison) : base(comparison, AttrDiffKind.Unspecified)
+        {
+        }
+    }
     #endregion
 }
diff --git a/src/AngleSharp.Diffing.Tests/DiffingTestBase.cs b/src/AngleSharp.Diffing.Tests/DiffingTestBase.cs
@@ -68,4 +68,10 @@ protected SourceMap ToSourceMap(string html, ComparisonSourceType sourceType = C
         var source = ToComparisonSource(html, sourceType);
         return new SourceMap(source);
     }
+
+    public static TheoryData<CompareResult> SameAndSkipCompareResult = new TheoryData<CompareResult>
+    {
+        CompareResult.Same,
+        CompareResult.Skip,
+    };
 }
diff --git a/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/AttributeComparerTest.cs b/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/AttributeComparerTest.cs
@@ -1,20 +1,21 @@
 namespace AngleSharp.Diffing.Strategies.AttributeStrategies;
 
-
 public class AttributeComparerTest : DiffingTestBase
 {
     public AttributeComparerTest(DiffingTestFixture fixture) : base(fixture)
     {
     }
 
-    [Fact(DisplayName = "When compare is called with a current decision of Same or Skip, the current decision is returned")]
-    public void Test001()
+    [Theory(DisplayName = "When current result is same or skip, the current decision is returned")]
+    [MemberData(nameof(SameAndSkipCompareResult))]
+    public void Test001(CompareResult currentResult)
     {
         var comparison = ToAttributeComparison(@"<b foo>", "foo",
-                                                "<b bar>", "bar");
+                                        "<b bar>", "bar");
 
-        AttributeComparer.Compare(comparison, CompareResult.Same).ShouldBe(CompareResult.Same);
-        AttributeComparer.Compare(comparison, CompareResult.Skip).ShouldBe(CompareResult.Skip);
+        new BooleanAttributeComparer(BooleanAttributeComparision.Strict)
+            .Compare(comparison, currentResult)
+            .ShouldBe(currentResult);
     }
 
     [Fact(DisplayName = "When two attributes has the same name and no value, the compare result is Same")]
@@ -23,7 +24,9 @@ public void Test002()
         var comparison = ToAttributeComparison(@"<b foo>", "foo",
                                                 "<b foo>", "foo");
 
-        AttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Same);
+        AttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Same);
     }
 
     [Fact(DisplayName = "When two attributes does not have the same name, the compare result is Different")]
@@ -32,7 +35,9 @@ public void Test003()
         var comparison = ToAttributeComparison(@"<b foo>", "foo",
                                                 "<b bar>", "bar");
 
-        AttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        AttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Name)));
     }
 
     [Fact(DisplayName = "When two attribute values are the same, the compare result is Same")]
@@ -41,7 +46,9 @@ public void Test004()
         var comparison = ToAttributeComparison(@"<b foo=""bar"">", "foo",
                                                @"<b foo=""bar"">", "foo");
 
-        AttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Same);
+        AttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Same);
     }
 
     [Fact(DisplayName = "When two attribute values are different, the compare result is Different")]
@@ -50,7 +57,9 @@ public void Test005()
         var comparison = ToAttributeComparison(@"<b foo=""bar"">", "foo",
                                                @"<b foo=""baz"">", "foo");
 
-        AttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        AttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Value)));
     }
 
     [Fact(DisplayName = "When the control attribute is postfixed with :ignoreCase, " +

diff --git a/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/BooleanAttributeComparerTest.cs b/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/BooleanAttributeComparerTest.cs
@@ -8,13 +8,25 @@ public BooleanAttributeComparerTest(DiffingTestFixture fixture) : base(fixture)
     {
     }
 
+    [Theory(DisplayName = "When current result is same or skip, the current decision is returned")]
+    [MemberData(nameof(SameAndSkipCompareResult))]
+    public void Test000(CompareResult currentResult)
+    {
+        var comparison = ToAttributeComparison(@"<b allowfullscreen=""false"">", "allowfullscreen", @"<b allowfullscreen>", "allowfullscreen");
+
+        new BooleanAttributeComparer(BooleanAttributeComparision.Strict)
+            .Compare(comparison, currentResult)
+            .ShouldBe(currentResult);
+    }
+
     [Fact(DisplayName = "When attribute names are not the same comparer returns different")]
     public void Test001()
     {
         var sut = new BooleanAttributeComparer(BooleanAttributeComparision.Strict);
         var comparison = ToAttributeComparison("<b foo>", "foo", "<b bar>", "bar");
 
-        sut.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        sut.Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Name)));
     }
 
     [Fact(DisplayName = "When attribute name is not an boolean attribute, its current result is returned")]

diff --git a/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/ClassAttributeComparerTest.cs b/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/ClassAttributeComparerTest.cs
@@ -6,6 +6,18 @@ public ClassAttributeComparerTest(DiffingTestFixture fixture) : base(fixture)
     {
     }
 
+    [Theory(DisplayName = "When current result is same or skip, the current decision is returned")]
+    [MemberData(nameof(SameAndSkipCompareResult))]
+    public void Test000(CompareResult currentResult)
+    {
+        var comparison = ToAttributeComparison($@"<p class=""foo"">", "class",
+                                               $@"<p class=""foo"">", "class");
+
+        ClassAttributeComparer
+            .Compare(comparison, currentResult)
+            .ShouldBe(currentResult);
+    }
+
     [Theory(DisplayName = "When a class attribute is compared, the order of individual " +
                           "classes and multiple whitespace is ignored")]
     [InlineData("", "")]
@@ -18,7 +30,9 @@ public void Test009(string controlClasses, string testClasses)
         var comparison = ToAttributeComparison($@"<p class=""{controlClasses}"">", "class",
                                                $@"<p class=""{testClasses}"">", "class");
 
-        ClassAttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Same);
+        ClassAttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Same);
     }
 
     [Fact(DisplayName = "When a class attribute is matched up with another attribute, the result is different")]
@@ -27,7 +41,9 @@ public void Test010()
         var comparison = ToAttributeComparison(@"<p class=""foo"">", "class",
                                                @"<p bar=""bar"">", "bar");
 
-        ClassAttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        ClassAttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Name)));
     }
 
     [Theory(DisplayName = "When there are different number of classes in the class attributes the result is different")]
@@ -38,7 +54,9 @@ public void Test011(string controlClasses, string testClasses)
         var comparison = ToAttributeComparison($@"<p class=""{controlClasses}"">", "class",
                                                $@"<p class=""{testClasses}"">", "class");
 
-        ClassAttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        ClassAttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Value)));
     }
 
     [Theory(DisplayName = "When the classes in the class attributes are different the result is different")]
@@ -51,6 +69,8 @@ public void Test012(string controlClasses, string testClasses)
         var comparison = ToAttributeComparison($@"<p class=""{controlClasses}"">", "class",
                                                $@"<p class=""{testClasses}"">", "class");
 
-        ClassAttributeComparer.Compare(comparison, CompareResult.Unknown).ShouldBe(CompareResult.Different);
+        ClassAttributeComparer
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.FromDiff(new AttrDiff(comparison, AttrDiffKind.Value)));
     }
 }
diff --git a/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/IgnoreAttributeComparerTest.cs b/src/AngleSharp.Diffing.Tests/Strategies/AttributeStrategies/IgnoreAttributeComparerTest.cs
@@ -8,6 +8,20 @@ public IgnoreAttributeComparerTest(DiffingTestFixture fixture) : base(fixture)
     {
     }
 
+    [Theory(DisplayName = "When current result is same or skip, the current decision is returned")]
+    [MemberData(nameof(SameAndSkipCompareResult))]
+    public void Test000(CompareResult currentResult)
+    {
+        var comparison = ToAttributeComparison(
+            @"<p foo=""bar""></p>", "foo",
+            @"<p foo=""bar""></p>", "foo"
+        );
+
+        IgnoreAttributeComparer
+            .Compare(comparison, currentResult)
+            .ShouldBe(currentResult);
+    }
+
     [Fact(DisplayName = "When a attribute does not contain have the ':ignore' postfix, the current decision is returned")]
     public void Test003()
     {