[llvm-commits] [llvm] r147431 - in /llvm/trunk/unittests: Bitcode/ Bitcode/BitReaderTest.cpp Bitcode/Makefile CMakeLists.txt Makefile VMCore/Makefile VMCore/pr11677.cpp

[Chris Lattner]

On Jan 3, 2012, at 1:55 PM, Chandler Carruth wrote:
> > Also clean up some of the test's naming. The goal for the file should be
> > to unittest the Bitcode Reader, and this is just one particular test
> > among potentially many in the future. Also, reverse my position and
> > relegate the PR# to a comment, but stash the comment on the same line as
> > the test name so it doesn't get lost. This makes the code more
> > self-documenting hopefully w/o losing track of the PR number.
> 
> Ugh.  Why is this worth having a unit test for?  Is the pain/benefit ratio here actually in favor of having the test?
> 
> What pain are you feeling from this unit test? That the code to implement the unit test is ugly? I certainly don't like that part of it…

The fact that a 2 line patch has a testcase that is an order of magnitude larger than it, requires another unit test to be linked (slowing down builds), the fact that Rafael had to waste his (presumably valuable ;-) time writing it, etc.

This seems like a complete waste of time.

> I think this is still worth having because this wasn't just a new unit test, this was a regression test for a bug we actually hit during LTO. It seems reasonable to want to test that we don't re-introduce the bug.

Why not write this as an LTO test in llvm/test/Linker then?

> However, I also think that writing tests like this could be *much* cleaner and simpler in order to reduce the pain of adding and maintaining them. I was hoping to wait for test #2 or #3 before investing in that infrastructure, but I can do it sooner if you'd just like to see what it would look like. I've been thinking about it a while and have some ideas of what I would really like tests of this nature to look like...

Why not just use the proper infrastructure we already have for this, instead of pushing unit tests?

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120103/967c1f1c/attachment.html>