Re: [boost] [test] Interest in 'hamcrest'-style checks?

Subject: Re: [boost] [test] Interest in 'hamcrest'-style checks?
From: Jared Grubb (jared.grubb_at_[hidden])
Date: 2013-09-15 20:45:38

On Sep 14, 2013, at 23:45, Gennadiy Rozental <rogeeff_at_[hidden]> wrote:

> Jared Grubb <jared.grubb <at> gmail.com> writes:
> 
>>> 3. Boost.Test already have this macro. It is called 
>>> BOOST_CHECK_PREDICATE
>>> 
>>> BOOST_CHECK_PREDICATE( P, (a)(b)(c) );
>>> 
>>> Very nice symmetry and most of your your other points covered
>>> (stackability etc)
>> 
>> I hadnt seen that one; I tried it, but it does require extra 
>> parens that I find distracting:
> 
> I find your asymmetry more distructing
>> BOOST_CHECK_PREDICATE(vec, (Contains(42)));
> 
> This obviously is wrong syntax.

My mistake.

>> However, there are a class of assertions that get a bit ugly in 
>> the BOOST_TEST form:
>>  * Contains(42)   vs   vec.find(42) != vec.end()
>>  * Any(Contains(42), Contains(43))   vs.  ( vec.find(42) != vec.end() || 
> vec.find(43) != vec.end())
> 
> I would use || and && and ! instead of Any, All, Not:
> 
> BOOST_CHECK_PREDICATE( contains(42) || contains(65), (container) );
> 
> BOOST_CHECK_PREDICATE( contains(5) && !contains(6), (container) );

I like that suggestion.

>> Using a higher level concept, rather than raw STL algorithm, 
>> also lets the failure messages be a bit more
>> coherent. 
> 
> There are 3 issues:
> 
> * We probably to do not want to replicate whole STL with "better error 
> message" predicates operating on containers instead of iterators
> * What if I do want to use iterators?

If iterator predicates are helpful and worth the header space, we add them. If they're not, then users use the existing macros or write their own predicates. I'm not suggesting we remove any of the current macros, which still are the primary solutions. But, I think adding hamcrest-style operators for the test cases where it does provide extra benefit is worth doing.

If a user wants special predicates that boost is unable/unwilling to include, we would at least provide the API, examples, and basis functions that users can extend to create their own predicates for their own applications. This infrastructure starts exactly with what you've already done to support custom predicates. But, there's a bit more in order to support composability and to support good diagnostics, which brings us to your third point:

> * I wonder how will you combine generically messages from 3 different 
> predicates in And predicate? We failed because "error msg 1" and "err msg 2" 
> and "err msg 3". This can become ugly very fast.

All my examples have been pseudo-diagnostics so far because I dont have a full solution to propose, and it is tricky. But I dont think it must be ugly and I think it's possible to have nice diagnostics in most cases.

The && operator actually isnt all that bad, since you would use short-circuit evaluation. So, "A && B && C" would result in one of "A failed", "A passed, but B failed", or "A and B passed, but C failed".

The || failure is more verbose, since a failure happens only when all atoms fail and you'd have to print a failure message for each atom.

Now, what exactly these messages would look like is up for debate, but maybe "contains(42) && (contains(43) || contains(44))" could print a failure like:

    Assertion failed. Expected all of the following to pass:
      * contains(42) - passed
      * any of the following conditions:
          * contains(43) - failed
          * contains(44) - failed

There's other ways, and there's more questions: should the full vector be printed? Always or only if it's short? How exactly do you format the nested failure? How do you show the passed atoms? Do you show atoms that are not evaluated at all due to short-circuit rules? etc.

>> The partially-bound nature of these assertions ( where _THAT( a, A(x) ) 
>> =>  "A(x)(a)" ) is useful because it
>> means the value-under-test can be written once and, even more important,
>> the value's expression is evaluated only once. 
> 
> This is also true in BOOST_CHECK_PREDICATE case (both parts). I do not see 
> any single reason to use this asymmetric syntax. 

I must not understand what "asymmetry" you mean, since in my mind, BOOST_CHECK_PREDICATE has the exact same asymmetry, just in opposite order:

BOOST_CHECK_THAT( container, contains(42) || contains(65) );
BOOST_CHECK_PREDICATE( contains(42) || contains(65),  (container) );

I personally prefer the first, but that's a style difference and not critical to the idea.

> Another complication to keep in mind is that you need to make all your 
> predicate "silently" bindable (here silently means an absence of bind 
> invocation. In other words both these should be valid:
> 
> BOOST_CHECK_PREDICATE( contains(1), (container) );
> BOOST_CHECK_PREDICATE( contains, (container, 1) );
> 
> BOOST_CHECK_PREDICATE( within_range(1,10), (value) );
> BOOST_CHECK_PREDICATE( !within_range, (value,1,10) );

Why is that required? That would make the implementation much harder and make compilation errors very ugly. It also seems fragile and easy to make a mistake (eg, "(within_range && contains, (value, 1, 10))" doesnt make sense and wouldnt compile). Plus, I would guess the binding order would result in the following, which arent consistent:
BOOST_CHECK_PREDICATE(contains(1), (container)) // equivalent to contains(1, container)?
BOOST_CHECK_PREDICATE(contains, (container, 1)  // equivalent to contains(container, 1)? 

If something like that is really required, maybe using "_1"-style placeholders would be a better approach. But, then I would worry the headers would end up being a brand new meta-language like Phoenix, which is interesting, but maybe overkill.

Jared