DeepEqual icon indicating copy to clipboard operation
DeepEqual copied to clipboard

Possibly unintended breaking change between 2.0.0 and 4.2.1

Open asgerhallas opened this issue 3 years ago • 2 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 { }

asgerhallas avatar Nov 30 '22 12:11 asgerhallas

I suspect it is this addition that makes the difference: https://github.com/jamesfoster/DeepEqual/blob/dbec89dae6bfeea89abb1422e57fa17cac987fdb/src/DeepEqual/ComplexObjectComparer.cs#L33

asgerhallas avatar Nov 30 '22 12:11 asgerhallas

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.

asgerhallas avatar Apr 17 '23 13:04 asgerhallas

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.

jamesfoster avatar May 29 '24 16:05 jamesfoster

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

asgerhallas avatar May 29 '24 19:05 asgerhallas