[LLVMdev] Unit test patch, updated

[Talin]

Hooray!

I'm thinking that getting unit tests for the classes in ADT should be an 
early goal.

I also wanted to mention a point about the general philosophy of unit 
testing, which is that the presence of such tests alters the calculation 
of risk when making changes to a code base. Programmers have various 
rules of thumb for estimating risk - for example, a change which affects 
a large number of lines of code is generally perceived to be riskier 
than a change which affects a small number of lines. But at the same 
time, some very large changes can carry almost no risk - such as 
changing the name of a symbol which occurs in thousands of places in the 
code - because the compiler will tell you if you made a mistake 
anywhere. What unit tests do is allow that same kind of assurance at the 
semantic level that the compiler's syntax checking gives you at the 
syntactical level. And since reduction of risk in one area allows taking 
greater risks elsewhere, it can act as an enabler for global 
refactorings and other radical improvements that would have been 
perceived as too risky otherwise.

This ability of unit tests to reduce perceived risk works best if the 
tests are comprehensive, testing even trivial aspects of the target 
class (such as brand new container correctly reporting empty()).

-- Talin

Misha Brukman wrote:
> 2008/12/31 Chris Lattner <clattner at apple.com>
>
>   
>> On Dec 30, 2008, at 2:54 PM, Talin wrote:
>>
>> OK changes made and new patch attached.
>>
>> +++ utils/unittest/Makefile (revision 0)
>>
>> ...
>> +# This has to come after Makefile.common, since it doesn't allow us to
>> +# override the VPATH value unless we set PROJECT_NAME, which we don't want
>> +# to do.
>> +VPATH = $(LLVM_SRC_ROOT)/utils/unittest/googletest/src/
>>
>>
>> Why play with VPATH here?  What is this doing?  Can this be handled a
>> different way?  Similarly in makefile.unittest.
>>
>>     
>
> Removed VPATH in this file by moving GTest up one directory, and added
> VPATH-less Makefiles:
> http://llvm.org/viewvc/llvm-project?view=rev&revision=61539
> http://llvm.org/viewvc/llvm-project?view=rev&revision=61540 (minor revision)
>
> Also removed VPATH from Makefile.unittest (see link below).
>
> +++ unittests/ADT/DenseMapTest.cpp (revision 0)
>   
>> +namespace {
>> +
>> +using namespace llvm;
>>
>>
>> Please put the 'using' right after the #includes.
>>
>>     
>
> Done.
>
>
>   
>> Otherwise, the patch looks good to me.  Please verify that it works with
>> both a SRCDIR=OBJDIR and SRCDIR!=OBJDIR build.
>>
>>     
>
> Done for both; passes both with flying colors.
> Submitted: http://llvm.org/viewvc/llvm-project?view=rev&revision=61541
>
> Thanks to Talin for the original patch.
>
> Now everyone can go write some unittests!
>
> Misha
>
> P.S.  Why are you still reading?  Go!  Write!  Test!
>
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>