fluentassertions / fluentassertions.json

NewtonSoft.Json extensions for FluentAssertions
Apache License 2.0
72 stars 26 forks source link

ShouldBeEquivalentTo() forces array strict ordering. Should it? I think not. #25

Open dstj opened 6 years ago

dstj commented 6 years ago

If I have an array property, the order of its values needs to be the same when asserting with Should().BeEquivalentTo(). I'm arguing that it shouldn't. That would be more in-line with "regular" FluentAssertions.

[Test]
public void TestArrayOrder()
{
   var expected =
@"{
   ""data"": [
      { ""value"": 1 },
      { ""value"": 2 }
   ]
}";
   var actual =
@"{
   ""data"": [
      { ""value"": 2 },
      { ""value"": 1 }
   ]
}";

   JsonParsingHelper.Parse(actual).Should().BeEquivalentTo(JsonParsingHelper.Parse(expected));
}

results in

Message: Expected JSON document {
  "data": [
    {
      "value": 2
    },
    {
      "value": 1
    }
  ]
} to be equivalent to {
  "data": [
    {
      "value": 1
    },
    {
      "value": 2
    }
  ]
}, but the value is different at $.data[0].value.

"Regular" FluentAssertions Should().BeEquivalentTo() does not enforce strict ordering on array properties. As such, the following test passes:

class TestClass
{
    public int[] Values { get; set; }
}

[Test]
public void Test()
{
    var expected = new TestClass { Values = new[] { 1, 2 } };
    var actual = new TestClass { Values = new[] { 2, 1 } };
    actual.Should().BeEquivalentTo(expected);
}

This causes me problems because the data I'm comparing comes from a database and the order cannot be guaranteed. I'll need to add ordering in the service I'm testing, but that's modifying the business logic to suit the testing framework. :/

dennisdoomen commented 6 years ago

That's an interesting statement. If we would change that, we would also need to expose some options to force strict ordening. But that all depends on how you define equivalent.

LordLyng commented 5 years ago

Any workarounds?

codialsoftware commented 5 years ago

@dennisdoomen why would you like to add any configuration for that? Equivalence is not be equal to so order should not be taken into consideration.

@LordLyng I also encountered this problem and the workaround is actually trivial. Just use BeEquivalentTo from FluentAssertions itself. It works fine.

EDIT This is what I'm currently doing in my code to overcome this issue. Ugly, but works totally fine.

((object) token).Should().BeEquivalentTo(expected)
dennisdoomen commented 5 years ago

why would you like to add any configuration for that? Equivalence is not be equal to so order should not be taken into consideration.

That's exactly what @LordLyng is proposing...

codialsoftware commented 5 years ago

@dennisdoomen Yes, I know. I just wanted to express my surprise for your proposal of exposing additional option. This might make sense for the sake of backward compatibility but how about publishing new major version where default behavior is aligned with the one from ObjectAssertions.BeEquivalentTo?

TheYarin commented 4 years ago

I agree, this should behave the same as the main library's BeEquivalentTo().

mtaghavi2005 commented 3 years ago

@dennisdoomen why would you like to add any configuration for that? Equivalence is not be equal to so order should not be taken into consideration.

@LordLyng I also encountered this problem and the workaround is actually trivial. Just use BeEquivalentTo from FluentAssertions itself. It works fine.

EDIT This is what I'm currently doing in my code to overcome this issue. Ugly, but works totally fine.

((object) token).Should().BeEquivalentTo(expected)

Unfortunately, this workaround doesn't work because it will be switched to the main library's BeEquaivalentTo. Like what mentioned here, false positives may occur.

dennisdoomen commented 3 years ago

If anybody is up for a PR...

mattfennerjesi commented 3 years ago

It isn't pretty, but this workaround works for me:

private void AssertJArraysEquivalentIgnoringOrder(JToken actual, JArray expected)
{
    var newActual = new JArray(actual.Children().OrderBy(c => c.ToString()));
    var newExpected = new JArray(expected.Children().OrderBy(c => c.ToString()));
    JAssert.Should(newActual).BeEquivalentTo(newExpected);
}
gvdmaaden commented 10 months ago

Hi @dennisdoomen, has this already been solved in a later release (and if so, what do you need to do)?

Right now I have also a workaround. A bit similar like the one above but mine re-orders the json tree on a recursive way. The key thing here is to first order all the properties and objects in the json tree. After that you can do the sorting of the array elements on the basis of the whole string representation of one array item.

Here is how I do it:

var jtActualOrdered = JsonHelper.OrderJson(jtActual);
var jtExpectedOrdered = JsonHelper.OrderJson(jtExpected);

jtActualOrdered.Should().BeEquivalentTo(jtExpectedOrdered);

internal static class JsonHelper
    {
        /// <summary>
        /// A helper method that orders a JSon and its children (properties and arrays).
        /// This is handy when you want to compare two identical Json objects that are not ordered the same way.
        /// </summary>
        /// <param name="token"></param>
        /// <returns></returns>
        public static JToken OrderJson(JToken token)
        {
            return OrderArraysItemsInJson(OrderObjectsInJson(token));
        }

        private static JToken OrderArraysItemsInJson(JToken token)
        {
            if (token.Children().Any())
            {
                for (int i = 0; i < token.Children().Count(); i++)
                {
                    var child = token.Children().ElementAt(i);
                    child = OrderArraysItemsInJson(child);
                    token.Children().ElementAt(i).Replace(child);
                }
            }

            if (token.Type == JTokenType.Array)
            {
                // order all elements in the array based on the string content of the element.                
                token = new JArray(token.Children()
                    .OrderBy(a => a.ToString())
                    );
            }

            return token;
        }

        private static JToken OrderObjectsInJson(JToken token)
        {
            if (token.Children().Any())
            {
                for (int i = 0; i < token.Children().Count(); i++)
                {
                    var child = token.Children().ElementAt(i);
                    child = OrderObjectsInJson(child); //go deeper in the json tree
                    token.Children().ElementAt(i).Replace(child);
                }

                if (token.Type == JTokenType.Object)
                {
                    // reorder all children in the object on type (descending so that arrays end up last) and name
                    token = new JObject(((JObject)token)
                        .Properties()
                        .OrderByDescending(p => p.First?.Type)
                        .ThenBy(p => p.Name).ToList());
                }
            }

            return token;
        }
    }

Make sure to include FluentAssertions.Json in your using sections

dennisdoomen commented 10 months ago

No, the JSON version of this library didn't get much attention. In v7 of the main library, we plan to have System.Text.Json support.

michaelgwelch commented 1 month ago

I'm late to the party but I'm just suggesting that strict ordering is often the correct choice for JSON arrays as they are also used for tuples where order definitely matters. So if the behavior were to change you would definitely want an option to still enforce ordering.