RE: [boost] Re: Formal review: New Boost.Test Library

From: Fernando Cacciola (fcacciola_at_[hidden])
Date: 2001-12-06 12:45:04

----- Original Message -----
From: rogeeff <rogeeff_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Thursday, December 06, 2001 2:15 PM
Subject: [boost] Re: Formal review: New Boost.Test Library

> >
> > ) In "test_tools.hpp", there are macros (i.e: BOOST_CHECKPOINT)
> which appear
> > *before* the declaration of  the objects they use. To me, this is
> confusing,
> > I recommend moving those macros to the bottom.
>
> I disagree. Macros are the public interface fot Test Tools and should
> be on top of file.
>
Ah, I see. OK then.

> >
> > ) I don't like the names of the macros 'BOOST_WARN_MESSAGE',
> > 'BOOST_CHECK_MESSAGE', 'BOOST_REQUIRE_MESSAGE', they don't do what
> they are
> > sematically implying: That is, 'check message' implies: 'check *the*
> > message, not 'check *this* and eventually show a message'. I
> suggest....
> > something else :)  [just couldn't come up with something]
>
> I hope it would be clear enouph, gived their non _MESSAGE versions.
> Anyway I did not find better names.
>
How about:  BOOST_CHECK_msg()..
Are macros names usually required to be ALL upercase?

> >
> > ) Also, I think the the 'message' in 'BOOST_CHECK_MESSAGE' (etc...)
> should
> > also contain the predicate, as in 'BOOST_CHECK'.  In other words,
> the
> > MESSAGE should be *added* to the output.
> > This is useful, because instead of:
> >
> >    BOOST_CHECK_MESSAGE(a!=0,"A isn't zero. Enter a different
> number")
> >
> > you would write:
> >
> >    BOOST_CHECK_MESSAGE(a!=0,"Enter a different number")
>
> I disagree. The purpose of _MESSAGE is t oprovide a readable failure
> description instead of posibly cryptic predicate dump. If you do need
> to add the predicate you can always do. Even Better you can write
> like this
>    BOOST_CHECK_MESSAGE(a!=0,"Invalid input: " << a << " != 0" );
>
> So current version allows everything.
>
I understand your point that the predicate *could* be too cryptic.
However, notice that this way you are forced either to add the predicate
string yourself or to miss it from the output.
I think that the question is:  Which would be typically more anoying, having
a possibly too cryptic predicate string or not having it?
I prefer to *always* see the predicate that failed, cryptic or not.
Anyway, this is certainly not something that would prevent the library from
being accepted.

 > >
> > ) I *strongly* disagree with having floating point and collection
> equlity
> > tools as part of the test library. They are orthogonal to the unit
> test
> > framework.
>
> Thay are not a part of Unit Test Framework. Thay are part of Test
> Tools.
>
> > There is an unbounded set of predicates that can be tested and
> > they shouln't be embebbed in the test library.
>
> For example? And why not
>
> There several tools left over to be included as an extentions:
> BOOST_CHECK_NOT_EQUAL, BOOST_CHECK_LESS, BOOST_CHECK_LESS_EQUAL
>
> > I would agree to provide, in the proper place, a floating point
> comparator
> > and a sequence matcher, but they should be outside the test suite.
>
> What if you need to check equality of to collections? Or result of
> you function under test if floating-point value, so you need to check
> how close it is t oexpected result? I think it's a natural part of
> Test Tools component.
>
See my other post...

> >
> > ) Just a question:
> >
> > Why does wrapstrstream::operator << returns a 'const' reference?
> This
> > requires 'buf' to be mutable.
>
> Instances of wrapstrstream in most cases created inline and passed by
> const references.
OK.

> So all methods of class wrapstrstream should be const.
Why? Do you mean that inside the function that took

 wrapstrstream  const&

you need to use operator << ?

Then why should it be taken as 'const&' instead of '&'?

Fernando Cacciola
Sierra s.r.l.
fcacciola_at_[hidden]
www.gosierra.com