Re: Review: Boost Test Library

From: williamkempf_at_[hidden]
Date: 2001-02-09 16:34:50

--- In boost_at_y..., Beman Dawes <bdawes_at_a...> wrote:
> At 07:21 PM 2/9/2001 +0000, williamkempf_at_h... wrote:
>  >Also, since catch_exceptions takes a function object as the first
>  >parameter the use of a function pointer and two global variables
>  >doesn't feel right.  I'd suggest using a true function object
>  >constructed with the argc and argv parameters instead.  I know 
that
>  >this is a personal preference kind of thing, but usage of global
>  >variables for this sort of thing seems very questionable.
> 
> They actually aren't globals; they are in the unnamed namespace and 
thus 
> safe to use AFAIK.

Even though they are in the unnamed namespace, they are still 
globals.  Just globals of a different stripe.  Since they are in the 
unnamed namespace they are safe (or is that just safer?) from ODR 
violations, but many of the dangers of "regular" global variables 
still exist.  In this case I can't see any usage that would actually 
create a problem, but it still just leaves me feeling like this 
should be addressed.  You know, one of those general "bad feelings 
with no concrete reason" circumstances that comes from "following 
safe idioms".

>  But none-the-less, I had exactly the same thought about 
> using a true function object.  My reasoning went something 
like "why bother 
> since it doesn't actually buy you anything."  But I may reconsider 
in light 
> of your comment.

If you decide not to you might still consider documenting any reason
(s) that might exist, just so others don't bring this up again in the 
future.

>  >In catch_exceptions.hpp I wonder about the usefulness of capturing
>  >every exception type in the std::exception heirarchy.  If it's
>  >beneficial to have the exact type reported then it seems that 
there
>  >needs to be a way to specify types that will be caught.  Otherwise
>  >libraries that extend this heirarchy with their own exception 
types
>  >will be reported as generic exception types while the standard
>  >exception types that extend the heirarchy are more precise.  I'd 
be
>  >in favor of just catching std::exception types instead.
> 
> I don't agree.  I think we should be as explicit as possible.  
Furthermore, 
> as soon as I get a chance, my plan was to further extend the types 
caught 
> to cover platforms like Windows where it is possible to turn OS 
exceptions 
> into C++ exceptions with some platform dependent code.

My concern was that it wasn't really explicit enough.  The optimal 
solution would be to somehow allow the user to specify the exception 
types that can be caught, but that's beyond the scope at this point I 
think.  However, since it was pointed out that not all 
implimentations give reasonable what() responses, I guess what you've 
done is the best solution.

>  >In test_tools.hpp I'd suggest adding two more macros:
>  >BOOST_TEST_FAILURE and BOOST_TEST_CRITICAL_FAILURE.  These would 
take
>  >string descriptions instead of boolean test results for reporting
>  >errors that are hard or even impossible to capture in boolean
>  >conditions.  For example, if we expect a call to return an 
exception
>  >when can write test code like this:
>  >
>  >try
>  >{
>  >   foo();
>  >   BOOST_TEST_FAILURE("foo() did not generate a bar() exception");
>  >}
>  >catch (bar&)
>  >{
>  >// ok
>  >}
> 
> I like it!  I also like your use of "CRITICAL" to identify a non-
resumable 
> error. (Assuming that is what you meant!).

Yes, that's what I meant.

>  I'm a bit nervous over the name 
> BOOST_TEST_ASSERT because I'm afraid readers will think it is just 
a 
> half-baked wrapper.
> 
> Maybe the names should be:
> 
>     BOOST_TEST_VERIFY
>     BOOST_TEST_CRITICAL_VERIFY
>     BOOST_TEST_FAILURE
>     BOOST_TEST_CRITICAL_FAILURE
> 
> or
> 
>     BOOST_TEST_VERIFY
>     BOOST_TEST_VERIFY_CRITICAL
>     BOOST_TEST_FAILURE
>     BOOST_TEST_FAILURE_CRITICAL
> 
> Comments?

Sounds good to me.  Though I like BOOST_TEST_CRITICAL_FAILURE over 
BOOST_TEST_FAILURE_CRITICAL, when applied to ..VERIFY.. it only makes 
sense to put the ..CRITICAL.. at the end, so I'd go with the latter 
set.

Bill Kempf