Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dstj opened this issue Jul 13, 2018 · 12 comments
Open

Comments

@dstj
Copy link
Contributor

dstj commented Jul 13, 2018

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
Copy link
Member

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
Copy link

LordLyng commented Feb 4, 2019

Any workarounds?

@codialsoftware
Copy link

codialsoftware commented Jul 9, 2019

@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
Copy link
Member

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
Copy link

codialsoftware commented Jul 10, 2019

@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
Copy link

TheYarin commented Jun 1, 2020

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

@mtaghavi2005
Copy link

@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
Copy link
Member

If anybody is up for a PR...

@mattfennerjesi
Copy link

mattfennerjesi commented Apr 23, 2021

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
Copy link

gvdmaaden commented Nov 16, 2023

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
Copy link
Member

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
Copy link

michaelgwelch commented Jul 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants