[LLVMdev] [Patch] Adding unit tests to LLVM

[Misha Brukman]

On Dec 22, 7:34 pm, Talin <viri... at gmail.com> wrote:
> (Forwarding this to llvm-dev)
>
> This patch adds a unit test framework to LLVM, along with a sample unit test
> for DenseMap. I don't expect this patch to be accepted as-is, this is mainly
> a trial balloon and proof of concept.

I think this is a great idea!  As Keir already noted, I would also
agree with LLVM snapshotting a copy of googletest, but I think it
should live in llvm/test/googletest (rather than top-level), since
it's not going to be linked into anything outside of llvm/test.

On the assumption that we'll end up using googletest, here are some
comments on your patch:

* s/Insure/Ensure/

* LLVM uses "llvm/foo.h" for inclusion rather than <llvm/foo.h>
* You should use the same format for gtest headers

* If reverse iteration isn't supported, you should either have an
ASSERT_DEATH() on the decrement, or not have the code there (that's
commented out) at all.

* Instead of this:
EXPECT_TRUE(uintMap[0] == 1);
you should use EXPECT_EQ()

* Instead of this:
EXPECT_TRUE(uintMap.find(0u) == uintMap.begin());
is it possible to use EXPECT_EQ() as well?

* In this test:
TEST_F(DenseMapTest, IterationTest) {
you use the array "int numbers[100];" as an array of booleans; why not
make it "bool visited[100];" to make clear what it for, how it's used,
and maybe be slightly more efficient?

> 1) For the testing framework, I went with Google Test, since it's the one I
> have the most experience with. I fully expect an extended bikeshed
> discussion to result from this.
>
> 2) Both the test framework and the tests are optional build targets, they
> will not be built with the normal "make all". To build and run the unit
> tests, use "make check-unit".

I am with Keir on this one: "check-unit" isn't the most intuitive name
to me -- "unittest" sounds fine, but this isn't a big deal to me.

I'll also volunteer to help maintain the LLVM branch of googletest and
review the unittests being checked in, as I also have experience with
using googletest, and I think LLVM could derive a lot of value from
unittesting.

Misha