Re: [boost] [Polygon] unit tests

Subject: Re: [boost] [Polygon] unit tests
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2009-09-02 15:35:22

Thomas Klimpel wrote:
> Simonson, Lucanus J wrote:
>> Thomas Klimpel wrote:
>>> - The unit-tests should be split into smaller files and use a
>>> nonzero exit value to indicate failure. They should be placed in
>>> libs/polygon/test.
>> 
>> What granularity would you like to see the tests broken into?
> 
> It's OK from my point of view to have many tests in a single source
> file, as long as regression testing is still possible. My basic
> assumption is that any unit-test can potentially fail, so it's not
> good if a single failing unit-test will mask many other unrelated
> test results. The typical reaction in such a situation would be to
> comment out the failing unit-test, which bears its own risks...     
> 
> One point to consider when writing unit-tests for a template library
> is that the compiler will not even find simple typos as long as a
> template function is not instantiated and called with a concrete type
> (Try this yourself: Change line 207 of interval_concept.hpp from
> "high(interval, (newHigh));" to "hhigh(interval, (newHigh));", and
> see if you can find a compiler that will detect this typo when
> compiling your current unit-tests.). So I probably would put pure
> "compile" tests into different source files than real functional
> tests. Having pure "compile" tests that just exercise every template
> function in the interface of a concept might be a good idea,
> especially if the test function is itself a template that is
> instantiated and called both for a class provided by the library and
> a custom class which registers as that concept. I also wouldn't
> object to have "compile" tests for more than one concept in a single
> source file.              
> 
> Don't forget the "use a nonzero exit value to indicate failure"
> advice for the functional tests. Even if you don't want to abort at
> the first failing test, ensure that the exit value is non-zero as
> soon as at least one test failed.   

Interval concept high() is called in the internal implementation of many algorithms, so even if there is no unit test for it the unit tests do cover it.  I do understand about template functions not being compiled if not used.  My methodology is to add a line in each that produces a compiler warning and remove that line when I see the warning produced by my tests.

My unit test does return non-zero error code.  I consider any failure to be critically important, so if I had multiple failures I imagine I would fix them one at a time until all were fixed and the unit test passes.  Having multiple files would help in the circumstance where multiple developers are breaking and fixing different pieces of the code simultaneously.  Since I was the sole author I didn't need to provide a testing methodology suitable for team development.  Also, it is possible for different developers to make changes on a branch and have unit test passing be a requirement for checking into the trunk.

>>> - Failing unit-tests should not be commented out just because they
>>> fail to compile.
>> 
>> I uncommented the handful of tests that I had commented out in the
>> unit test source and these tests do compile and pass in MSVC9 and
>> gcc4.2.2. The only tests commented out now are one which fails
>> because the functionality isn't implemented (planed but not
>> implemented functinality) and one that has long runtime and performs
>> no functional testing. 
> 
> After the custom_xxx.cpp examples failed to compile, I searched for
> "geometry_concept" in "gtl_boost_unit_test.cpp" and all occurrences
> of it were in code that was currently commented out. I searched again
> for "geometry_concept" after your modifications, but the
> corresponding code is still commented out.    

Ah, this is code that was lifted out of the unit test into the library itself and made part of the library interfaces.  It should be deleted.  I commented it out because it conflicted with the code that does the same thing which is now part of the library itself.

Thanks, I think reviewing the unit tests is an important and easily overlooked area and I'm grateful that you took the time.
Luke