Re: [PATCH 00/16] Unit tests framework (v3)

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2015-10-28 at 12:38 +0100, Bernd Schmidt wrote:
> On 10/27/2015 08:48 PM, David Malcolm wrote:
> > The following patch kit adds a unit tests framework for gcc,
> > as a new subdirectory below gcc/testsuite.
> 
> So, as a general comment I think this would be a very good thing to 
> have, and from a quick look through the tests they look pretty sensible.

Thanks.

> > Like previous versions of the patch kit it uses the Google Test
> > framework, since I've had good experiences with it:
> >    http://code.google.com/p/googletest/
> > and like v2 of the kit it embeds a two-file copy of v1.7 of
> > the kit, to avoid adding extra dependencies (one .h file and one
> > .c file).
> 
> How much of this are you actually using? How much effort would be 
> involved in removing the extra prerequisite? Is it just the EXPECT_TRUE 
> etc. macros?

As well as the EXPECT_FOO macros, there's test registration, sometimes
done via TEST, sometimes via TEST_F to support fixtures.  I used
TYPED_TEST for type-parameterizing the wide-int tests (though, to be
fair, I didn't manage to get this properly working; see the comment in
test-wide-int.c).

There may be useful things in gtest for us that I'm not using yet.  For
example, the support for death tests may be useful for testing that e.g.
our checking macros kill the program sanely in the presence of malformed
internal data.

I wanted to reuse the gtest code on the grounds that:

  * the code exists

  * it's maintained (https://github.com/google/googletest
    shows the last commit was 10 days ago)

  * it has documentation:
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md
(there's much more that just that)

  * it will be familiar to some people from other projects


That said, if it's a blocker, I can attempt a minimal reimplementation
of the subset of the API that I'm using.


One wart in the v3 patchkit was that I used the default configuration
for gtest.  For example, this implicitly required threading support.
This could be suppressed by defining:

   #define GTEST_HAS_PTHREAD 0

before including gtest.h.

> > v1 of the kit was structured as a frontend, v2 of the kit as
> > a plugin that was built as if it were a frontend.  Both of
> > these approaches were problematic, so this version
> > of the patch kit simply builds as a test case within the
> > plugin.exp suites.
> 
> This is the part I'm least certain about, for two reasons:
>   * They are more like source files than tests, and I think I'd prefer
>     to have them alongside the source, in gcc/ rather than in the
>     testsuite. This way people are invariable going to fail to notice
>     them when they grep for something.

So something like:

  gcc/test-vec.c

or even:

  gcc/vec-tests.c

so that it sorts next to gcc/vec.h in a listing?

i.e.   gcc/FOO-tests.c  for gcc/FOO.c

Maybe the unittests-plugin.c should live within the "gcc" directory?

>   * This uses a plugin into whatever compiler was built, but sometimes
>     you can't really set up unit tests that way because what you want
>     to test depends on target specifics. What I've often wanted is a
>     special test target that gets built with a special machine
>     description that has whatever patterns are needed to replicate
>     tricky situations in reload or other optimization passes.
> 
> The tests you have so far are focused mostly on high-level gimple/tree 
> tests where this limitation is probably not showing up very much, but I 
> think it would be better to have something that allows us to have more 
> in-depth tests.

I think our long-term goal should be at least 5 styles of test:

(A) end-to-end tests of the compiler: running it on some source and
verifying properties of the diagnostics and/or generated code
(potentially running it).  This is what we have today.

(B) unit tests of individual subsystems: tests that exercise specific
internal APIs e.g. containers like vec<>, hash_map<> etc, also things
like gengtype.  This is a gap in our current test coverage, and what
this patch kit aims to start filling.

(C) and (D): tests of specific passes: tests that accept IR (either
gimple or RTL), and run just one pass, and verify properties of the IR
emitted by the pass, or messages emitted by the pass.  LLVM has a "-R"
remark option, so that a pass can issue remarks about what it's doing (a
new kind of diagnostic):

http://llvm.org/releases/3.5.0/tools/clang/docs/UsersManual.html#opt-rpass

We could implement this kind of testing by implementing gimple and RTL
frontends, and a -R option, which could integrate nicely with DejaGnu,
with
  /* { dg-remark "inlined here" } */
and the like.

This would require finishing the gimple FE, and writing an RTL FE
(independent goals).  Hopefully that would give us test coverage for
dealing with e.g. tricky reload situations.

(E) performance testing


The purpose of this patch kit is (B).  I'm interested in tackling (C)
and (D), but I think those are going to have to wait until next stage 1
at this point.


Dave