sebastienros/fluid

ArrayValue and DictionaryValue Equals overrides return false for equal values

dkanashevich opened this issue · 5 comments

Unit tests demonstrating the problem:

using Fluid.Values;

namespace FluidTests;

public class ValueEqualityTests
{
    [Test]
    public void DictionaryValuesShouldBeEqual()
    {
        var actual = new DictionaryValue(new FluidValueDictionaryFluidIndexable(new Dictionary<string, FluidValue>
        {
            { "stringProperty", StringValue.Create("testValue") }
        }));

        var expected = new DictionaryValue(new FluidValueDictionaryFluidIndexable(new Dictionary<string, FluidValue>
        {
            { "stringProperty", StringValue.Create("testValue") }
        }));


        Assert.That(actual, Is.EqualTo(expected));
    }

    [Test]
    public void ArrayValuesShouldBeEqual()
    {
        var actual = new ArrayValue(new FluidValue[]
        {
            StringValue.Create("testValue")
        });

        var expected = new ArrayValue(new FluidValue[]
        {
            StringValue.Create("testValue")
        });


        Assert.That(actual, Is.EqualTo(expected));
    }
}

How does it reflect in a liquid template?

Asking because the test is convoluted, it uses Is.EqualTo from xunit and I have no idea (I will investigate) how it works. If it's just using reference equality, then it doesn't mean anything for Fluid. I know however that both ArrayValue and DictionaryValue implement IEquatable<FluidValue> and check for this exact test. So if there is an actual bug it would be reflected in a template (where xunit is not implying a behavior).

You could also change the test to use the FluidValue.Equals(FluidValue) to see if that actually works.

How does it reflect in a liquid template?

Maybe it doesn't. I noticed the problem while writing tests for mapping my data to fluid values.

Asking because the test is convoluted, it uses Is.EqualTo from xunit and I have no idea (I will investigate) how it works. If it's just using reference equality, then it doesn't mean anything for Fluid. I know however that both ArrayValue and DictionaryValue implement IEquatable<FluidValue> and check for this exact test. So if there is an actual bug it would be reflected in a template (where xunit is not implying a behavior).

You could also change the test to use the FluidValue.Equals(FluidValue) to see if that actually works.

Sorry for providing a misleading test. Here is the correct version:

using Fluid.Values;

namespace FluidTests;

public class ValueEqualityTests
{
    [Test]
    public void DictionaryValuesShouldBeEqual()
    {
        var actual = new DictionaryValue(new FluidValueDictionaryFluidIndexable(new Dictionary<string, FluidValue>
        {
            { "stringProperty", StringValue.Create("testValue") }
        }));

        var expected = new DictionaryValue(new FluidValueDictionaryFluidIndexable(new Dictionary<string, FluidValue>
        {
            { "stringProperty", StringValue.Create("testValue") }
        }));

        Assert.True(actual.Equals(expected));
    }

    [Test]
    public void ArrayValuesShouldBeEqual()
    {
        var actual = new ArrayValue(new FluidValue[]
        {
            StringValue.Create("testValue")
        });

        var expected = new ArrayValue(new FluidValue[]
        {
            StringValue.Create("testValue")
        });


        Assert.True(actual.Equals(expected));
    }
}

The result is the same:

Expected: True
But was:  False

Awesome, thank you.