jamesfoster/DeepEqual

Possibly unintended breaking change between 2.0.0 and 4.2.1

asgerhallas opened this issue · 4 comments

The following test passed in 2.0.0 but fails in 4.2.1 as it returns a Pass now. Is this intended?

    [Fact]
    public void ComparingObjectsWithNoProperties_DifferentType()
    {
        var comparer = (IComparison)new ComparisonBuilder().Create();
        var context = new ComparisonContext();

        Assert.Equal(
            ComparisonResult.Inconclusive, 
            comparer.Compare(context, new object(), new Blah()).result);
    }
    
    public class Blah { }

I suspect it is this addition that makes the difference:

if (propertyMap.Count == 0) return (ComparisonResult.Pass, context);

Allow me to bump this question :)

Is it intentional that two classes of different types, but both with no properties, should return a Pass?

I'd say it would be more natural with Inconclusive, to be able to fall back to the DefaultComparer.

Hi, apologies for abandoning issues on this project for so long.

I believe this was an intended breaking change. If 2 types have no properties to compare (because they've all been ignored or are simply empty objects) then they should be treated as equal.

I do try to use semantic versioning, so if the major version number changes - it's because there's a breaking change.

If this is not the behaviour you want it should be possible to override it.

If there's a good case for reverting this I'm happy to have that discussion. If so feel free to re-open this issue.

This makes sense, thanks! I don't have a good case for reverting, that can not be handled another way 😊