Re: [boost] Review: Test library

From: Beman Dawes (bdawes_at_[hidden])
Date: 2001-02-17 11:21:39

At 01:47 PM 2/13/2001 -0600, Ed Brey wrote:

 >I think the Test library should be accepted into boost.  I think the
 >breakdown of execution, test, and unit test tools is excellent.  Comments
 >regarding areas for improvement:
 >
 >catch_exceptions.hpp:
 >
 >1. Why is BOOST_GENERAL_ERROR_RETURN made to look like a macro, and not 
in
 >a namespace?
 >Assuming there is a good reason, then there is the issue that the 
anonymous
 >namespace around BOOST_GENERAL_ERROR_RETURN is unnecessary; since it's a
 >constant it doesn't get external linkage.

Boost doesn't have any guidelines for constants, so I (probably 
unconsciously) fell back on some other guidelines used in unrelated work.

For Boost, I guess the constant should go in namespace boost to minimize 
namespace pollution.  For the name, how about "EXIT_EXCEPTION_FAILURE" to 
match EXIT_FAILURE.

Then the analogous case in test_main.cpp would be EXIT_TEST_FAILURE.

 >  If trying to be like EXIT_FAILURE, what is the difference?

Environments may want to distinguish between different classes of 
errors.  Also, lots of programs distinguish between types of errors by 
returning codes which are typically are integers < 100. OTOH, I had a 
comment from Jens that some environments use integers >=256 for other 
purposes.  Thus he suggested using 200 as the Boost code.  I thought we 
might want to distinguish between exceptions and other errors, so I was 
going to use 201 or 199 for the other value.

 >2. RTTI should be used on platforms where appropriate to provide display 
of
 >precise exception class names (as pre earlier post).  When RTTI is not
 >appropriate, all standard exceptions should be hard coded;

I'll try to work that in.  But I'm worried there may be unknown or 
unintended consequences.

 > currently missing is std::ios_base::failure.

Will add.

 >3. Function_object be checked against EXIT_SUCCESS instead of zero to
 >determine success, unless there is a good reason to do otherwise?  A
 >rationale should be documented.

I'll change it to EXIT_SUCCESS.

 >test_tools.hpp:
 >
 >1. (void)0 could become static_cast<void>(0).  The one benefit I can see 
is
 >this allows one to run a lint check looking for C-style casts on boost
 >(either by itself or as a component of a larger work, which is using
 >boost).

I was just recycling some old code.  Nowadays I never use an old-style 
cast.  I'll change it.

 >test_tools.cpp:
 >
 >1. It might be nice to allow the output to be any stream, instead of 
always
 >cout.

Hum... There isn't any test_tools.cpp.  I assume you mean test_main.cpp 
since it wouldn't make sense in test_tools.hpp (because it is used in 
functions which may not have enough context to know what stream should be 
used, plus shouldn't be coupled to any I/O headers.)

But, yes, test_main.cpp might be made independent of the particular 
ostreams.  Perhaps for use in a GUI.  I'm so clueless about GUI's, it is 
hard for me to know if that would be worthwhile.  Do any of the major GUI's 
have ostreams that could be used?  But, yes, if I can figure out how to do 
it cleanly, I'll add that.

 >test_main.cpp:
 >
 >1. test_tools_exception has superfluous destructor definition.  As I
 >recall,
 >a previous post asserted that a non-throwing destructor is generated by
 >default in a case such as this.

Removed.

 >2. Why does BOOST_TEST_ERROR_RETURN needed rather than reusing
 >BOOST_GENERAL_ERROR_RETURN?  The "why make it look like a macro" question
 >applies here as well.

See above.

 >execution_tools.htm:
 >
 >1. Hello world string literal has slash instead of backslash.

Fixed.

 >2. Hello world example should have argc and argv commented out to show
 >example of how avoid warnings when not using arguments.

Fixed.

 >test_tools.htm:
 >
 >1. (This one's my favorite.)  "Boost Unit Test Tolls may be more suitable
 >for complex test programs."  If boost doesn't find a good funding source
 >through a book or journal, maybe this is the right answer.  :-).

:-)

 >2. Talks about "std::EXIT_SUCCESS" and "std::EXIT_FAILURE".  These names
 >are
 >macros and are not in any namespace.  Also, it is implied that 
EXIT_SUCCESS
 >is always 0 and that EXIT_FAILURE is always non-zero.  The Standard 
doesn't
 >seem to guarantee such; rather it says that success is returned if and 
only
 >if exit is passed either 0 or EXIT_SUCCESS (but EXIT_SUCCESS is not
 >constrained to be either the same or different than 0). 18.3/8

I'll work on making it clearer.  Both the C and C++ standards seem to try 
to leave a lot of wiggle room in their wording.  That makes it hard to 
write docs users can rely on across all implementations.

 >General:
 >
 >1. Capitalization is inconsistent: Only the "Errors detected; see stdout
 >for
 >details" has a capital letter; nothing else does.  My preference would be 

 >to
 >always start with a capital letter and end with a period for canned 
summary
 >messages "Returning with error code #." and "Errors detected; see stdout
 >for
 >details.", but leave detail messages (exceptions, test failed) as is.

I'll fix that.

 >2. Conditional compilation blocks should list standard code first, then
 >compiler workarounds.

Yes.

 >3. Lost of extra spaces in docs.  When I use FrontPage, I too find it is
 >easy to have extra spaces sneak in.  The solution to find them is pretty
 >simple: grep for "nbsp".

I'll work on that.  Because of FrontPage, I expect it is a problem 
elsewhere, too.

 >4. I agree with a previous post that calls for giving several of the 
global
 >variables a home.

I'm planning to do that, but also note that variables in the unnamed 
namespace are not globals and it is a stylistic rather than code safety 
issue.

 >Pet feature (MSVC-specific):
 >
 >Test programs do well as console (as opposed to GUI) applications, since
 >there are many advantages to having them be completely automated; hence 
no
 >GUI is needed for user input.  The test tools suite is very fits in very
 >well in this regard.  For debugging, it is often very efficient to use 
the
 >exact same console application written for automated testing, as one can
 >often, by quickly commented out working sections of a test, reproduce 
bugs
 >quickly in a deterministic way, and get to breakpoints very quickly.
 >
 >The catch is that in the Windows environment, when using VC (and possibly
 >other vendors' tools), debugging a console test application can be 
annoying
 >and less productive that debugging an application that writes its output 
to
 >the Windows debug stream.  This avoids the console from popping up any 
time
 >code is running between breakpoints, and avoids having to switch to the
 >console window to view output (VC provides a good deal of control over
 >where the debug stream is displayed).
 >
 >The best of both worlds is a test executable that could run as a console
 >application by default (useful when running tests outside a debugger), 
but
 >run as a basic Win32, non-console application (useful when debugging) 
with
 >a command line switch.  Actually achieving run-time behavior such as this
 >isn't trivial (at least for me; I went in knee deep and got stuck).  If
 >anyone else is interested in this kind of functionality, contact me, and 
I
 >can share what I know.  In general, I would like to present this use case 

 >as something to think about (and balance against simplicity) in keeping
 >the test tools library flexible.

My first priority for platform specific improvements is to catch OS 
specific exceptions/signals on Win32 and Unix platforms.

But I'm open to some GUI-knowledgeable programmer supplying the kind of 
aids you are talking about.

Thanks for all the comments,

--Beman