Added ElementsAre and UnorderedElementsAre

[johnbeard]

[This is a continuations of the solid start made in #2133 by @Garz4 in 2020, with some modifications]

Added ElementsAre and UnorderedElementsAre generic range matchers which take a range during construction and their match method take a range as well.

Addresses issue #2087

As before, no matcher constructor analogous to ContainsMatcherMatcher is provided, as this is just the same as AllMatch.

Differences to the previous PR:

• Add unit tests and docs
• Differing types are permitted, so you can compare, e.g. a vector of ints to an array of chars, or any other comparable types.
• STL-like predicates are also permitted instead of the default std::equal_to

[codecov]

Codecov Report

Merging #2377 (ccb4f47) into devel (dd36f83) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head ccb4f47 differs from pull request most recent head b5ee85d. Consider uploading reports for the commit b5ee85d to get more accurate results

@@            Coverage Diff             @@
##            devel    #2377      +/-   ##
==========================================
+ Coverage   91.15%   91.19%   +0.05%     
==========================================
  Files         187      188       +1     
  Lines        7691     7719      +28     
==========================================
+ Hits         7010     7039      +29     
+ Misses        681      680       -1     

[horenmar]

This should also have (and support) rangeish types that don't have member begin (e.g. they provide ADL-findable begin only), and have different types for begin and end (the support for these is currently spotty, but the goal is to provide it eventually).

Honestly the test utils for this could use some refactoring and cleanup in general, so I don't blame you if you don't want to write the tests, but the impl should reasonably support these.

[horenmar]

huh, is this actually the formatting given by clang-format?

I would swear the clang-format had nested namespace indents disabled.

[johnbeard]

It seems not: NamespaceIndentation: All is specified (should be None for zero indent) :

Catch2/.clang-format

Line 16 in 61d0f7a

NamespaceIndentation: All

[horenmar]

I didn't check the code in too much detail, but here is a big note on this:

As before, no matcher constructor analogous to ContainsMatcherMatcher is provided, as this is just the same as AllMatch.

I want these to support matcher inputs on the expected side, so something like this can be written: REQUIRE_THAT( foo(), ElementsAre(IsOdd(), IsEven(), IsOdd());. Ideally I would also like something like this to work: REQUIRE_THAT( foo(), ElementsAre(1, 2, 3)); but that is secondary and I am fine with forcing a different syntax there.

Alternatively, I am willing to hear an argument on why the functionality above should have a different name (or, the functionality in this PR could be renamed to something like Equals and UnorderedEquals).

[johnbeard]

Thank you for the careful review!

I want these to support matcher inputs on the expected side, so something like this can be written: REQUIRE_THAT( foo(), ElementsAre(IsOdd(), IsEven(), IsOdd());.

OK, so just to check, the difference is then that AllMatch applies a single matcher to each element:

`REQUIRE_THAT( foo(), AllMatch( IsOdd() );

And ElementsAre uses a variadic form where the matchers are applied element-wise:

REQUIRE_THAT( foo(), ElementsAre(IsOdd(), IsEven(), IsOdd());

Makes sense to me, though I'll have to have a think about how to do it!

If I can indeed do such a variadic matcher syntax, then it feel like REQUIRE_THAT( foo(), ElementsAre(1, 2, 3)); would be just one SFINAE away at that point.

[horenmar]

Thank you for the careful review!

Oh don't worry, that will come later. 🙃 And you are correct about the differences between AllMatch (Any, None) matcher and this one.

I do think you might be underestimating the complexity to support both direct-element comparison, and matcher-based comparison. For the latter, most work can be reused from the support code in && and || operators, but when I did a quick prototype, the work to intermix them proved to be pretty annoying. Do note that mixing both in single matcher should be also supported, so that e.g. ElementsAre(1, IsOdd(), 3) is valid.

[johnbeard]

I do think you might be underestimating the complexity to support both direct-element comparison, and matcher-based comparison.

Probably! But worth a try!

I don't mind to split to the work to Equals/UnorderedEquals and something else, or ElementsAre and something else. If you'd like to go that way, I have no objection. But it seems like if the constructors look like:

• ElementsAre( other_container )
• ElementsAre( 1, 2, 3 ) // module a set of {}, see below

Then having separate names for each would be a shame and a bit confusing since they're so similar.

Would it make sense to use an initializer-list constructor like std::vector: ElementsAre{ 1, IsOdd(), 3 } rather than a variadic constructor? That might make it possible to also do ElementsAre( {1,2,3}, predicate ), since the init list is just the first argument. In the same way that the init-list ctor of std::vector has an optional allocator argument. If it's just variadic, it'll certainly be (at least) awkward to separate the last one if it's a predicate.

[johnbeard]

I'm still trying to think of how to do this, but it probably will take me a while to figure it out (I also will never complain if someone wants to Just Do It and preempt it, I claim no dibs!).

Maybe it makes more sense to tidy and move to merge what exists here? This interface remains the same?:

REQUIRE_THAT( range, ElementsAre(otherRange);