fluentassertions / fluentassertions

A very extensive set of extension methods that allow you to more naturally specify the expected outcome of a TDD or BDD-style unit tests. Targets .NET Framework 4.7, as well as .NET Core 2.1, .NET Core 3.0, .NET 6, .NET Standard 2.0 and 2.1. Supports the unit test frameworks MSTest2, NUnit3, XUnit2, MSpec, and NSpec3.
https://www.fluentassertions.com
Apache License 2.0
3.72k stars 544 forks source link

BeEquivalentTo incorrectly compare JsonElements? #2374

Open Smurfa opened 11 months ago

Smurfa commented 11 months ago

Description

I am trying to compare if two json strings are equivalent (don't care about whitespaces or order). Using suggestion from previous issue: https://github.com/fluentassertions/fluentassertions/issues/1212 yields incorrect results.

Reproduction Steps

[Fact]
public void Test()
{
    var foo = JsonDocument.Parse("""{"test":"something"}""").RootElement;
    var bar = JsonDocument.Parse("""{"test":"something else"}""").RootElement;

    foo.Should().BeEquivalentTo(bar, opt => opt.ComparingByMembers<JsonElement>());
}

Expected behavior

Would expect this to fail, "something" != "something else".

Actual behavior

Test passes.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 7.0 xunit 2.5.0 FluentAssertions 6.12.0

Other information

No response

Are you willing to help with a pull-request?

No

jnyrup commented 11 months ago

My snippet in #1212 only compares the ValueKind property of a JsonElement. The design of JsonElement doesn't let us use the built-in structural equivalency which reflects over the properties on the type. To make it work, you'll have to write a custom IEquivalencyStep.

Here's some code I hacked together in five minutes. It works for your example, but it most likely doesn't work in the general case, so no guarantees are given.

[Fact]
public void Test()
{
    var foo = JsonDocument.Parse("""{"test":"something"}""").RootElement;
    var bar = JsonDocument.Parse("""{"test":"something else"}""").RootElement;

    foo.Should().BeEquivalentTo(bar, opt => opt.Using(new JsonObjectEquivalencyStep()));
}

public class JsonObjectEquivalencyStep : IEquivalencyStep
{
    public EquivalencyResult Handle(Comparands comparands, IEquivalencyValidationContext context, IEquivalencyValidator nestedValidator)
    {
        if (comparands.Subject is not JsonElement subject || comparands.Expectation is not JsonElement expectation)
        {
            return EquivalencyResult.ContinueWithNext;
        }

        if (subject.ValueKind is JsonValueKind.Object && expectation.ValueKind is JsonValueKind.Object)
        {
            var newComparands = new Comparands(subject.EnumerateObject(), expectation.EnumerateObject(), typeof(IEnumerable<JsonProperty>));
            nestedValidator.RecursivelyAssertEquality(newComparands, context);
            return EquivalencyResult.AssertionCompleted;
        }

        if (subject.ValueKind is JsonValueKind.Array && expectation.ValueKind is JsonValueKind.Array)
        {
            var newComparands = new Comparands(subject.EnumerateArray(), expectation.EnumerateArray(), typeof(IEnumerable<JsonElement>));
            nestedValidator.RecursivelyAssertEquality(newComparands, context);
            return EquivalencyResult.AssertionCompleted;
        }

        return EquivalencyResult.ContinueWithNext;
    }
}

Also the failure message is also not as pretty as it could be, as it uses some internal index over the json property name

Expected root[0] to be "test":"something else", but found "test":"something".
dennisdoomen commented 11 months ago

This should also be covered by #2205

Smurfa commented 10 months ago

Sorry for late response and thanks for your replies @jnyrup and @dennisdoomen. Wanted to make sure if I was missing something obvious to make it work out of the box. But then I know it would require some custom comparison implementation to get it working. Cheers!

dennisdoomen commented 7 months ago

@jnyrup what do you think of the PR? It could be a first, rather independent step to proper support for System.Text.Json?

dennisdoomen commented 4 months ago

@jnyrup ?