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

[Chandler Carruth]

On Tue, Jan 3, 2012 at 1:28 PM, Chris Lattner <clattner at apple.com> wrote:

>
> On Jan 2, 2012, at 1:19 AM, Chandler Carruth wrote:
>
> > Author: chandlerc
> > Date: Mon Jan  2 03:19:48 2012
> > New Revision: 147431
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=147431&view=rev
> > Log:
> > Undo the hack in r147427 and move this unittest to a better home. This
> > is testing the bitcode reader's functionality, not VMCore's. Add the
> > what is a hope sufficient build system mojo to build and run a new
> > unittest.
> >
> > 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...

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.

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...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120103/c9728f50/attachment.html>