Added ContainsAny and DoesNotContainAny to CollectionAssert

[tbenne10]

Mentioned in #4187 - Added feature to allow CollectionAssert to verify if a IEnumerable contains any or does not contain any elements from a provided IEnumerable

[dnfadmin]

All CLA requirements met.

[MoritzHayden]

@tbenne10 Thank you for adding this!

[OsirisTerje]

@stevenaw Can you have a look at this?

[OsirisTerje]

I am struggling a bit with the syntax of these.
The first looks fine: I would write Assert.That(collectionA,Does.ContainsAny(collectionB)).
The second becomes strange, because I would expect to be writing Assert.That(collectionA,Does.Not.ContainsAny(collectionB)). So when the keyword is DoesNotContainsAny it looks like a legacy collection assert, and not a constraint based one. The last one could also be Assert.That(collectionA,Has.No.Members(collectionB)), which would then require a new Members constraint for a collection, instead of the Member for a single item.

[mr-russ]

I also recommend Charlie Pool's comment on #4187 , and decide if we are going down the road of new constraint.

[stevenaw]

Thanks for taking initiative on this @tbenne10
Apologies for a lack of up-front clarity on this. As a team we're trying to adopt a constraint-first approach to new features. As a team we also need to figure out how to make this more well-known, which is on us. We have #3688 to track doing that.

After this PR was created, Charlie has also weighed in in the issue about a previous bit of discussion to freeze the CollectionAssert API and that the team would need to decide if a change in direction were to happen. I think that would be an important conversation to have, but it wouldn't need to block adding this in as constraints.

Would it be possible to adjust the added tests to target the new constraint instead of CollectionAssert itself? In lieu of a change in direction about CollectionAssert it may also help to remove any changes to CollectionAssert in the PR.

[stevenaw]

Thanks for adding this. I often also use the Contains class for this sort of check too. Would it be possible to also expose a Contains.All(IEnumerable) function there too?

[tbenne10]

Anytime - just want to clarify, do you use Contains with iteration to perform the checks you need? Just want to make sure we are on the same page since they perform different actions.

The naming might have been a bit misleading here - Contains.All(IEnumerable) already exists but is instead named SupersetOf(IEnumerable) within this class.

[stevenaw]

@tbenne10 Thanks for asking, I appreciate you coming back to this PR after all this time.

You make a good point. I may've mistyped my earlier suggestion in haste. I'd suggest we disregard my suggestion around Contains.All() in the interest of keeping this PR simple. Having thought it through, I agree with @OsirisTerje 's suggestion that it might make sense to expose your new constraint as a Does.ContainAny() method, which could then be easily inverted to be Does.Not.ContainAny() using the already-existing Not constraint.

I think my original comment came from a place of concern around ambiguity of the name Is.ContainedIn in relation to some other API operations when both arguments are collections. If I understand your PR right, the following will pass the first assertion but fail the second and third because they apply fundamentally different checks, though it might not be clear simply by reading them that this will happen.

var a = new[] { 1, 2, 3 };
var b = new[] { 2, 4 };

Assert.That(a, Is.ContainedIn(b));
Assert.That(a, Is.SubsetOf(b));
Assert.That(b, Contains.All(b));

[tbenne10]

Thank you all for your input. I will remove changes to CollectionAssert and retarget the tests per @stevenaw's comment.

Before I complete updates, I wanted to circle back and check with everyone if we want to support adding methods in Assert.cs utilizing the constrains within this MR. If so I will add those with naming suggestions from @OsirisTerje unless anyone has differing naming suggestions.

[OsirisTerje]

@tbenne10 Seems this has been dormant for some time. Do you think you have time to get it up to shape?

[OsirisTerje]

@tbenne10 Do you think you can resolve the remaining comments?

[mithileshz]

@tbenne10 @OsirisTerje would you mind if I take over and complete the changes on this?

[OsirisTerje]

@tbenne10 haven't responded since May last year, so I think that is ok.
I'll make sure both of you get credit for this in the release notes.

Please read through everything above so that you're sure about how the constraint approach should work here.