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

ContainItemsAssignableTo Now Expects At Least One Item Assignable to T #1765

Merged
merged 9 commits into from Jan 10, 2022
26 changes: 12 additions & 14 deletions Src/FluentAssertions/Collections/GenericCollectionAssertions.cs
Expand Up @@ -965,20 +965,18 @@ public AndConstraint<TAssertions> ContainItemsAssignableTo<TExpectation>(string

if (success)
{
MullerWasHere marked this conversation as resolved.
Show resolved Hide resolved
int index = 0;
foreach (T item in Subject)
{
if (item is not TExpectation)
{
Execute.Assertion
.BecauseOf(because, becauseArgs)
.FailWith(
"Expected {context:collection} to contain only items of type {0}{reason}" +
", but item {1} at index {2} is of type {3}.", typeof(TExpectation), item, index, item.GetType());
}

++index;
}
Execute.Assertion
.BecauseOf(because, becauseArgs)
.WithExpectation("Expected {context:collection} to contain element assignable to type {0}{reason}, ", typeof(TExpectation).FullName)
MullerWasHere marked this conversation as resolved.
Show resolved Hide resolved
.Given(() => Subject)
.ForCondition(subject => subject.Any())
.FailWith("but was empty.")
.Then
.ForCondition(subject => subject.Any(x => typeof(TExpectation).IsAssignableFrom(GetType(x))))
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I think we need to avoid enumerating the collection twice.

Copy link
Member

Choose a reason for hiding this comment

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

FYI, this implementation is almost identical to the existing AllBeAssignableTo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dennisdoomen the last changes in f4f87ba no longer enumerate the collection twice:
https://github.com/MullerWasHere/fluentassertions/blob/f4f87bad178ba8552c47743844ec0b35e61807af/Src/FluentAssertions/Collections/GenericCollectionAssertions.cs#L968-L976

@jnyrup Should I refactor both AllBeAssignableTo and ContainItemsAssignableTo to reduce duplication?

Copy link
Member

Choose a reason for hiding this comment

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

Should I refactor both AllBeAssignableTo and ContainItemsAssignableTo to reduce duplication?

No, they solve different problems.

Copy link
Member

Choose a reason for hiding this comment

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

the last changes in f4f87ba no longer enumerate the collection twice:

It still does. You first do an Any() and then an Any(predicate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dennisdoomen I misunderstood that as a potential issue with IEnumerables. Removed any() and adjusted unit tests in 60a8890.

.FailWith("but found {0}.",
subject => $"[{string.Join(", ", subject.Select(x => GetType(x).FullName))}]")
dennisdoomen marked this conversation as resolved.
Show resolved Hide resolved
.Then
.ClearExpectation();
}

return new AndConstraint<TAssertions>((TAssertions)this);
Expand Down
Expand Up @@ -24,7 +24,7 @@ public void Should_succeed_when_asserting_collection_with_all_items_of_same_type
}

[Fact]
public void Should_fail_when_asserting_collection_with_items_of_different_types_only_contains_item_of_one_type()
public void Should_succeed_when_asserting_collection_with_items_of_different_types_only_contains_item_of_one_type()
MullerWasHere marked this conversation as resolved.
Show resolved Hide resolved
{
// Arrange
var collection = new List<object>
Expand All @@ -33,68 +33,50 @@ public void Should_fail_when_asserting_collection_with_items_of_different_types_
"2"
};

// Act
Action act = () => collection.Should().ContainItemsAssignableTo<string>();

// Assert
act.Should().Throw<XunitException>();
// Act / Assert
collection.Should().ContainItemsAssignableTo<string>();
}

[Fact]
public void When_a_collection_contains_anything_other_than_strings_it_should_throw_and_report_details()
public void When_asserting_collection_contains_item_assignable_to_against_null_collection_it_should_throw()
{
// Arrange
var collection = new List<object>
{
1,
"2"
};
int[] collection = null;

// Act
Action act = () => collection.Should().ContainItemsAssignableTo<string>();
Action act = () =>
{
using var _ = new AssertionScope();
collection.Should().ContainItemsAssignableTo<string>("because we want to test the behaviour with a null subject");
};

// Assert
act.Should().Throw<XunitException>().WithMessage(
"Expected collection to contain only items of type System.String, but item 1 at index 0 is of type System.Int32.");
"Expected collection to contain element assignable to type System.String because we want to test the behaviour with a null subject, but found <null>.");
}

[Fact]
public void When_a_collection_contains_anything_other_than_strings_it_should_use_the_reason()
public void When_a_collection_is_empty_an_exception_should_be_thrown()
{
// Arrange
var collection = new List<object>
{
1,
"2"
};
int[] collection = Array.Empty<int>();

// Act
Action act = () => collection.Should().ContainItemsAssignableTo<string>(
"because we want to test the failure {0}", "message");
Action act = () => collection.Should().ContainItemsAssignableTo<int>();

// Assert
act.Should().Throw<XunitException>()
.WithMessage(
"Expected collection to contain only items of type System.String because we want to test the failure message" +
", but item 1 at index 0 is of type System.Int32.");
act.Should().Throw<XunitException>().WithMessage("Expected collection to contain element assignable to type \"System.Int32\", but was empty.");
}

[Fact]
public void When_asserting_collection_contains_item_assignable_to_against_null_collection_it_should_throw()
public void Should_throw_exception_when_asserting_collection_for_missing_item_type()
{
// Arrange
int[] collection = null;
var collection = new object[] { "1", 1.0m };

// Act
Action act = () =>
{
using var _ = new AssertionScope();
collection.Should().ContainItemsAssignableTo<string>("because we want to test the behaviour with a null subject");
};
Action act = () => collection.Should().ContainItemsAssignableTo<int>();

// Assert
act.Should().Throw<XunitException>().WithMessage(
"Expected collection to contain element assignable to type System.String because we want to test the behaviour with a null subject, but found <null>.");
act.Should().Throw<XunitException>()
.WithMessage("Expected collection to contain element assignable to type \"System.Int32\", but found \"[System.String, System.Decimal]\".");
}

#endregion
Expand Down
1 change: 1 addition & 0 deletions docs/_pages/releases.md
Expand Up @@ -12,6 +12,7 @@ sidebar:
### What's New

### Fixes
* `ContainItemsAssignableTo` Now Expects At Least One Item Assignable to `T` - [#1765](https://github.com/fluentassertions/fluentassertions/pull/1765)
dennisdoomen marked this conversation as resolved.
Show resolved Hide resolved

## 6.3.0

Expand Down