8000 Added `BeOneOf` for object comparisons with custom comparer support by xmarshal · Pull Request #2111 · fluentassertions/fluentassertions · GitHub
[go: up one dir, main page]

Skip to content

Added BeOneOf for object comparisons with custom comparer support#2111

Closed
xmarshal wants to merge 0 commit intofluentassertions:developfrom
xmarshal:develop
Closed

Added BeOneOf for object comparisons with custom comparer support#2111
xmarshal wants to merge 0 commit intofluentassertions:developfrom
xmarshal:develop

Conversation

@xmarshal
Copy link
Contributor
@xmarshal xmarshal commented Jan 20, 2023

Added BeOneOf for object comparisons with custom comparer support
provided api for #2112

@IT-VBFK
Copy link
Contributor
IT-VBFK commented Jan 20, 2023

Hmm.. I think this should be discussed in an issue first (aka api review process ;) )

@xmarshal
Copy link
Contributor Author

Api proposal added #2112

@coveralls
Copy link
coveralls commented Jan 21, 2023

Pull Request Test Coverage Report for Build 3968029095

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.9%) to 96.961%

Totals Coverage Status
Change from base Build 3937335010: 0.9%
Covered Lines: 12473
Relevant Lines: 12712

💛 - Coveralls

@IT-VBFK
Copy link
Contributor
IT-VBFK commented Jan 24, 2023

@xmarshal Do you mind covering your latest changes with tests? 😉

@jnyrup jnyrup added the feature label Jan 24, 2023
}

/// <summary>
/// Asserts that a <typeparamref name="TExpectation"/> equals another <typeparamref name="TExpectation"/> using its <see cref="object.Equals(object)" /> implementation.
Copy link
Member

Choose a reason for hiding this comment

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

🔧 You're not using object.Equals here. You're using IEqualityComparer<T>.

.BecauseOf(because, becauseArgs)
.ForCondition(Subject is TExpectation subject && comparer.Equals(subject, expected))
.WithDefaultIdentifier(Identifier)
.FailWith("Expected {context} to be {0}{reason}, but found {1}.", expected,
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I would also expect some reference to the comparer, should I not?

Copy link
Member

Choose a reason for hiding this comment

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

You mean something like including comparer.GetType()?

We do something similar when using an IEqualityComparer<T> in equivalency assertions.
https://github.com/fluentassertions/fluentassertions/pull/1284/files#diff-f060184f7198367f57bfcf2bbfc8ce3ddc5ce5db1949c93e376d6d3c780c53b8R37

}
}

internal class SomeClass
Copy link
Member

Choose a reason for hiding this comment

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

🔧 We prefer to keep the test classes as simple and specific for the individual test cases as possible, and also next to them (in the same grouping class, e.g. BeOneOf. See also

To assert that an object is equal to one of the provided objects with custom equality comparer, you can use

```csharp
theObject.Should().BeOneOf<TObj>(new[] { obj1, obj2, obj3 } , new ObjEqualityCompmarer());
Copy link
Member

Choose a reason for hiding this comment

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

🤔 And what about the new Be and NotBe overloads?

## Unreleased

### What's new
* Added `BeOneOf` for object comparisons with custom comparer support -[#2111](https://github.com/fluentassertions/fluentassertions/pull/2111)
Copy link
Member

Choose a reason for hiding this comment

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

🤔 And what about the new Be and NotBe overloads?

Comment on lines +194 to +222
[Fact]
public void When_a_value_is_not_one_of_the_specified_values_and_equality_comparer_passed_it_should_throw()
{
// Arrange
var value = new SomeClass(3);

// Act
Action act = () => value.Should().BeOneOf(new[] { new SomeClass(4), new SomeClass(5) }, new SomeClassEqualityComparer());

// Assert
act
.Should().Throw<XunitException>()
.WithMessage("Expected value to be one of {SomeClass(4), SomeClass(5)}, but found SomeClass(3).");
}

[Fact]
public void When_a_value_is_not_one_of_the_specified_values_and_equality_comparer_passed_it_should_throw_with_descriptive_message()
{
// Arrange
var value = new SomeClass(3);

// Act
Action act = () => value.Should().BeOneOf(new[] { new SomeClass(4), new SomeClass(5) }, "because those are the valid values");

// Assert
act
.Should().Throw<XunitException>()
.WithMessage("Expected value to be one of {SomeClass(4), SomeClass(5)} because those are the valid values, but found SomeClass(3).");
}
Copy link
Member

Choose a reason for hiding this comment

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

You can merge these two tests, there's no reason to have two tests here as they're testing a single concept.
I have written myself a note to clean this up.

The same comment applies for the two tests below.

@jnyrup jnyrup linked an issue Mar 26, 2023 that may be closed by this pull request
@jnyrup
Copy link
Member
jnyrup commented May 29, 2023

@xmarshal Do you plan continuing this or anything blocking you?

@jnyrup
Copy link
Member
jnyrup commented Jul 14, 2023

@xmarshal checking in one more time to see if you'd like to drive this PR to completion.
I really like the addition, so if you're no longer interested in or have the time to finish this, I'll pick it up and finish the last bits.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BeOneOf for object comparisons with custom comparer support

5 participants

0