Re: [lmi] Relaxing BOOST_TEST_THROW() message test to do substring match only

[Greg Chicares]

On 2016-02-18 13:03, Vadim Zeitlin wrote:
> On Thu, 18 Feb 2016 02:26:39 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-02-18 01:37, Vadim Zeitlin wrote:
> GC> 
> GC> >  IMO checking for substring, instead of exact, match, would be a better
> GC> > solution because it wouldn't really weaken the existing tests and would
> GC> > make using "" much less appealing thus strengthening the future ones. 
> And
> GC> > as a nice side effect, this would also make the macro implementation
> GC> > simpler as it wouldn't need to worry about "\n[file" part.
> GC> > 
> GC> >  What do you think?
> GC> 
> GC> Could we make this optional?
> 
>  Yes, of course. I can see 2 reasonable ways to do it: the simplest one is
> to just add another macro working as I'd like it to, and the most difficult
> problem with this approach is choosing the name of this macro: we could use
> a direct BOOST_TEST_THROW_SUBSTRING or more descriptive (but less precise)
> BOOST_TEST_THROW_RELAXED or something in the middle but very long like
> BOOST_TEST_THROW_INCLUDING_MESSAGE.

Of those, I like "BOOST_TEST_THROW_RELAXED" best. However...

>  Another one would be to make the third argument of the existing macro an
> object of some exception_message_matcher type. Such object would be
> implicitly constructible from string and would match exactly in this case,
> for compatibility, but it could also be constructed using some
> match_substring() factory function which would create an object matching as
> I'd like it to.
> 
>  The second approach is slightly more complicated but more flexible (e.g.
> we could easily extend it to support regex matching in the future) and
> avoids the need to agonize over the macro naming decision, so I prefer it.
> The main problem with it from your point of view is probably that it's
> intrusive, i.e. it requires changes to the existing code instead of being a
> pure addition as the first one.

How intrusive? Would the second approach require changing 'test_tools.hpp'
only, where the macro is defined, or would it require changes wherever the
macro is used? If the change is confined to that one header, then I'm okay
with the second approach (which you prefer).

> GC> Then I can have the rigid tests I like, and you can have greater
> GC> flexibility. And who knows, I might someday come round to your point of
> GC> view.
> 
>  Just as an example of why I think it's an overkill to test for the exact
> match: if I want to check that the SOA table creation code doesn't accept
> tables with the wrong hash, it's quite enough to check that an exception
> with "hash value" somewhere in its message is thrown, it really doesn't
> matter if the message says "hash value is incorrect" or "hash value
> mismatch" or "expected hash value XXX but got hash value YYY" which could
> all be reasonable formulations for this exception. And it definitely
> doesn't matter if the full message is actually "Invalid data for table
> NNN: incorrect hash value XXX specified at the line LLL", the fact that it
> includes "hash value" is quite enough.

If you want to write tests that way, that's fine with me. I might still
prefer to specify the full message in tests I write. If I were to check
only for "hash value" in order to ascertain whether an expected exception
like std::runtime_error("The hash value is incorrect.") was thrown, then
I'd want to consider whether any code I'm not focusing on might throw
  foo_error("Failed to compute hash value: libfoo not be loaded.")
If I specify the full message, then I don't have to think about that.
But I don't mind accommodating the relaxed style you prefer.