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-13 11:38:11

On Sep 10, 2013, at 21:12, Gennadiy Rozental <rogeeff_at_[hidden]> wrote:

> Jared Grubb <jared.grubb <at> gmail.com> writes:
> 
>> 
>> I recently read "Modern C++ Programming with Test-Driven Development",
> 
> ... All these books... trying to reinvent the wheel.
> 
>> and I really enjoyed this book and
>> highly recommend it. One of the styles I saw for the first time was 
>> "hamcrest"-style assertions.
>> ("Hamcrest" is an anagram of "matchers"; also see 
>> http://en.wikipedia.org/wiki/Hamcrest.)
>> 
>> For a quick idea, imagine there is a BOOST_CHECK_THAT macro 
>> that looks like this: (excuse my choice of names
>> here, we can pick better ones if people like the idea)
>> 
>> BOOST_CHECK_THAT( str, IsEqualTo(expStr) );
>> BOOST_CHECK_THAT( str, HasSubstring(needleStr) );
>> BOOST_CHECK_THAT( str, Not(HasSubstring(badStr)) );
>> BOOST_CHECK_THAT( vec, Contains(42) );
>> 
>> Comparing them to current boost::test assertions, usually the 
>> two are directly equivalent, and it's just
>> a matter of style. At first, I was unconvinced of a need for this.
>> However, there are a few scenarios where
>> the hamcrest-style actually are really helpful:
>> 
>> BOOST_CHECK_THAT(vec, Any(Contains(42), Contains(-42)) );   
>> BOOST_CHECK_THAT(str, All(StartsWith("abc"), 
>>                  HasSubstring("lmnop"), EndsWith("xyz")) );
>> BOOST_CHECK_THAT(count, All(GE(0), LT(10)) );
>> BOOST_CHECK_THAT(vec, HasSubsequence(otherVec) );
>> BOOST_CHECK_THAT(vec, Sorted(IsEqualTo({1, 2, 3})) );
>> BOOST_CHECK_THAT(optionalStr, Any(IsEqualTo(boost::none), 
>> Deref(IsEqualTo("str")));
> 
> Hi Jared,
> 
> Let me start by saying that you might have something here, but I am not 
> convinced yet. I have number of comments here:
> 
> 1. Stylistic
> All these IsEqualTo, HasSubstring etc has a strong Java stench. I personally 
> do not see a place for something like this in proper C++.

Jeff Langr, the author of that book, is indeed a Java guy ... your instinct is right on. :) I'm with you on this. I wasnt convinced by them at first either; I personally really dislike Java style.

> 2. Why exactly you are giving preference to first argument over all other 
> predicate arguments? Will your predicate report them all? Why the asymmetry 
> then?

Yes, the diagnostics can take advantage of that; for example, "got 42, expected 43" instead of "42 != 43". 

> 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:

BOOST_CHECK_PREDICATE(vec, (Contains(42)));

> Moreover we have a framework for writing custom predicates. This should be a 
> function which returns test_tools::assertion_result. For example:
> 
> boost::test_tools::assertion_result
> compare_lists( std::list<int> const& l1, std::list<int> const& l2 )
> {
>    if( l1.size() != l2.size() ) {
>        boost::test_tools::predicate_result res( false );
> 
>        res.message() << "Different sizes [" << l1.size() << "!=" << 
> l2.size() << "]";
> 
>        return res;
>    }
> 
>    return true;
> }

Yes, these new predicates would be implemented like this.

> 4. The new Boost.Test drops most of the current macros in a favor of a 
> single "super" macro BOOST_TEST:
> 
> BOOST_TEST( a == b )
> BOOST_TEST( a != b )
> BOOST_TEST( a+1 <= b )
> 
> This one is based on brilliant idea from Kevlin Henney (pretty much once of 
> the few rare new ideas that came out recently in testing domain). This 
> handles many of your cases already with proper syntax.

I'm looking forward to these! I think BOOST_TEST covers 80-90% of the assertions I would use. Even if hamcrest was available, I would still expect that people use BOOST_TEST(a==b) over BOOST_CHECK_THAT(a, Eq(b)). But, the "Eq" check is still useful in composed tests ("None(Eq,Eq)"), which is the only reason I think it should be included. 

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())

Using a higher level concept, rather than raw STL algorithm, also lets the failure messages be a bit more coherent. Compare:
 * the error message that prints the inequality of "str.find("abc")" and "string::npos" (something like "10 != -1")
 * the error message "str does not contain "abc"; str was "xyz""

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 lets you pop values from a queue and run two checks without having to save the value first to a local variable; this can improve the readability of a unit test (and it can hurt too if misused).

> 5. There way to many custom user specific predicates out there. We can try 
> all we want, but all we end up doing is just covering needs of rather small 
> subset of users. There should be line we draw somewhere: what is expected to 
> be supplied by the library and what is specific to the user's domain. 
> 
> That being said...
> 
> If you believe you can present a set of some generic reusable predicates, 
> which can be used with BOOST_TEST_PREDICATE and implemented based on 
> assertion_result, I think we can discuss if there is a place for it inside 
> of UTF. Keep in mind "do not pay for what do not use" principle. You can't 
> have one big header with 100 dependencies.

Agreed. I think a massive header full of a hamcrest corresponding to each STL algorithm is overkill, but I think that a handful of them could make testing some (10%) assertions easier. The hamcrest header(s) would have to be explicitly included (not included in unit_test.hpp) and users would probably want a "using namespace" to bring these into global lookup.

Jared