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

add ComparingByMembers(Func<Type, bool> predicate) to SelfReferenceEq… #1383

Closed

Conversation

ursenzler
Copy link
Contributor

@ursenzler ursenzler commented Sep 10, 2020

…uivalencyAssertionOptions

Adds ComparingByMembers(Func<Type, bool> predicate) to SelfReferenceEquivalencyAssertionOptions<TSelf> so that it is possible to define the strategy used for comparison with a callback. This simplifies scenarios when a group of types should be handled differently than FluentAssertion's standard behaviour.

This is useful when for example structurally comparing F# record types that contain C# classes that do not implement equality.
F# records are by default treated as value types because they implement equality. As a result, the containing values are treated as value types, making structural comparison of the inner C# objects difficult (every type would have to be configured).
With the new callback, it is possible to define the behaviour for all record types at once:

actual.Should()
      .BeEquivalentTo(expected,
                      Func<_, _>(fun (config : EquivalencyAssertionOptions<'a>) ->
                          config.ComparingByMembers(fun t -> FSharpType.IsRecord t)),
                      "",
                      [])

IMPORTANT

  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used in this example.
  • If the contribution adds a feature or fixes a bug, please update the release notes, which are published on the website.
  • If the contribution changes the public API the changes needs to be included by running AcceptApiChanges.ps1/AcceptApiChanges.sh.
  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.

Copy link
Member

@dennisdoomen dennisdoomen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be willing to also update the release notes and documentation as listed in the PR template? Otherwise we have to pick up the PR ourselves.

@@ -538,6 +546,12 @@ public TSelf ComparingByMembers<T>()
return (TSelf)this;
}

public TSelf ComparingByMembers(Func<Type, bool> predicate)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Please document this new member in the same style as the other overloads.

@@ -596,6 +596,56 @@ public void When_the_graph_contains_guids_it_should_properly_format_them()
act.Should().Throw<XunitException>().WithMessage("Expected item[0].Id to be *-*, but found *-*");
}

[Fact]
public void When_defining_comparing_by_members_with_callback_that_returns_false_then_the_normal_equality_strategy_process_should_be_used()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙃 I'm nitpicking here, but we prefer to keep those test names functional and never refer to the literal name of an identifier, e.g. When_none_of_the_members_should_be_compared_by_members_it_should_succeed

}

[Fact]
public void When_defining_comparing_by_members_with_callback_it_should_use_force_members_strategy_if_a_callback_returning_true_is_found()
Copy link
Member

@dennisdoomen dennisdoomen Sep 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 So is there some implicit or explicit order? Would be nice to capture that in a functional test name.

@ursenzler
Copy link
Contributor Author

I close this PR because the change how ComparingByMember<object> handles basic types solves the same problem.

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

Successfully merging this pull request may close these issues.

None yet

2 participants