llvm dev - Feb 2016 - Testing Best Practices/Goals (in the context of compiler-rt)

Recently had a bit of a digression in a review thread related to some tests
going in to compiler-rt (
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html
) and there seems to be some disconnect at least between my expectations
and reality. So I figured I'd have a bit of a discussion out here on the
dev lists where there's a bit more visibility.

My basic expectation is that the lit tests in any LLVM project except the
test-suite are targeted tests intended to test only the functionality in
the project. This seems like a pretty well accepted doctrine across most
LLVM projects - most visibly in Clang, where we make a concerted effort not
to have tests that execute LLVM optimizations, etc.

There are exceptions/middle ground to this - DIBuilder is in LLVM, but
essentially tested in Clang rather than writing LLVM unit tests. It's
somewhat unavoidable that any of the IR building code (IRBuilder,
DIBuilder, IR asm printing, etc) is 'tested' incidentally in Clang in
process of testing Clang's IR generation. But these are seen as incidental,
not intentionally trying to cover LLVM with Clang tests (we don't add a
Clang test if we add a new feature to IRBuilder just to test the IRBuilder).

Another case with some middle ground are things like linker tests and
objdump, dwarfdump, etc - in theory to isolate the test we would checkin
binaries (or the textual object representation lld had for a while, etc) to
test those tools. Some tests instead checkin assembly and assemble it with
llvm-mc. Again, not to cover llvm-mc, but on the assumption that llvm-mc is
tested, and just using it as a tool to make tests easier to maintain.


So I was surprised to find that the compiler-rt lit tests seem to diverge
from this philosophy & contain more intentional end-to-end tests (eg:
adding a test there when making a fix to Clang to add a counter to a
function that was otherwise missing a counter - I'd expect that to be
tested in Clang and that there would already be coverage in compiler-rt for
"if a function has a counter, does compiler-rt do the right thing with that
counter" (testing whatever code in compiler-rt needs to be tested)).

Am I off base here? Are compiler-rt's tests fundamentally different to the
rest of the LLVM project? Why? Should they continue to be?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160210/bb67d5e0/attachment.html>

On Wed, Feb 10, 2016 at 11:06 AM, David Blaikie via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> Recently had a bit of a digression in a review thread related to some tests
> going in to compiler-rt (
>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html
> ) and there seems to be some disconnect at least between my expectations
and
> reality. So I figured I'd have a bit of a discussion out here on the
dev
> lists where there's a bit more visibility.
>
> My basic expectation is that the lit tests in any LLVM project except the
> test-suite are targeted tests intended to test only the functionality in
the
> project. This seems like a pretty well accepted doctrine across most LLVM
> projects - most visibly in Clang, where we make a concerted effort not to
> have tests that execute LLVM optimizations, etc.
>
> There are exceptions/middle ground to this - DIBuilder is in LLVM, but
> essentially tested in Clang rather than writing LLVM unit tests. It's
> somewhat unavoidable that any of the IR building code (IRBuilder,
DIBuilder,
> IR asm printing, etc) is 'tested' incidentally in Clang in process
of
> testing Clang's IR generation. But these are seen as incidental, not
> intentionally trying to cover LLVM with Clang tests (we don't add a
Clang
> test if we add a new feature to IRBuilder just to test the IRBuilder).
>
> Another case with some middle ground are things like linker tests and
> objdump, dwarfdump, etc - in theory to isolate the test we would checkin
> binaries (or the textual object representation lld had for a while, etc) to
> test those tools. Some tests instead checkin assembly and assemble it with
> llvm-mc. Again, not to cover llvm-mc, but on the assumption that llvm-mc is
> tested, and just using it as a tool to make tests easier to maintain.
>
>
> So I was surprised to find that the compiler-rt lit tests seem to diverge
> from this philosophy & contain more intentional end-to-end tests (eg:
adding
> a test there when making a fix to Clang to add a counter to a function that
> was otherwise missing a counter - I'd expect that to be tested in Clang
and
> that there would already be coverage in compiler-rt for "if a function
has a
> counter, does compiler-rt do the right thing with that counter"
(testing
> whatever code in compiler-rt needs to be tested)).
>
> Am I off base here? Are compiler-rt's tests fundamentally different to
the
> rest of the LLVM project? Why? Should they continue to be?
If a change is made to Clang to make it emit different IR, there
should be a test for that in Clang's test suite. Adding a test to
compiler-rt should generally not be considered sufficient, especially
as plenty of people hack on Clang without compiler-rt checked out /
without running the compiler-rt tests.

On the compiler-rt side, taking the sanitizer tests as an example: I
would view testing that the compiler produces the right machine code
as being an incidental part of the unit test coverage in the same way
that an objdump test covering llvm-mc is incidental coverage -- it
should not be the point of the test, but it may be the most direct way
to demonstrate that the runtime library behaves in the right way in
the presence of the expected set of calls to it.

If the compiler-rt maintainers want to go beyond that and include
end-to-end tests alongside their unit tests, and doing so doesn't make
their tests fragile, I think that's their call, but it might make
organizational sense to separate those tests out into a different
directory at least.

David Blaikie via cfe-dev <cfe-dev at lists.llvm.org>
writes:
> Recently had a bit of a digression in a review thread related to some tests
> going in to compiler-rt (
>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html
> ) and there seems to be some disconnect at least between my expectations
> and reality. So I figured I'd have a bit of a discussion out here on
the
> dev lists where there's a bit more visibility.
>
> My basic expectation is that the lit tests in any LLVM project except the
> test-suite are targeted tests intended to test only the functionality in
> the project. This seems like a pretty well accepted doctrine across most
> LLVM projects - most visibly in Clang, where we make a concerted effort not
> to have tests that execute LLVM optimizations, etc.
>
> There are exceptions/middle ground to this - DIBuilder is in LLVM, but
> essentially tested in Clang rather than writing LLVM unit tests. It's
> somewhat unavoidable that any of the IR building code (IRBuilder,
> DIBuilder, IR asm printing, etc) is 'tested' incidentally in Clang
in
> process of testing Clang's IR generation. But these are seen as
incidental,
> not intentionally trying to cover LLVM with Clang tests (we don't add a
> Clang test if we add a new feature to IRBuilder just to test the
IRBuilder).
>
> Another case with some middle ground are things like linker tests and
> objdump, dwarfdump, etc - in theory to isolate the test we would checkin
> binaries (or the textual object representation lld had for a while, etc) to
> test those tools. Some tests instead checkin assembly and assemble it with
> llvm-mc. Again, not to cover llvm-mc, but on the assumption that llvm-mc is
> tested, and just using it as a tool to make tests easier to maintain.
>
> So I was surprised to find that the compiler-rt lit tests seem to diverge
> from this philosophy & contain more intentional end-to-end tests (eg:
> adding a test there when making a fix to Clang to add a counter to a
> function that was otherwise missing a counter - I'd expect that to be
> tested in Clang and that there would already be coverage in compiler-rt for
> "if a function has a counter, does compiler-rt do the right thing with
that
> counter" (testing whatever code in compiler-rt needs to be tested)).
>
> Am I off base here? Are compiler-rt's tests fundamentally different to
the
> rest of the LLVM project? Why? Should they continue to be?
I think there's a bit of grey area in terms testing the runtime -
generally it's pretty hard to use the runtime without a fairly
end-to-end test, so tests of the runtime often end up looking pretty
close to an end-to-end test.

That said, I don't think that should be used as an excuse to sneak
arbitrary end-to-end tests into compiler-rt. We should absolutely write
tests in clang and llvm that we're inputting what we expect to the
runtime and try to keep the tests in compiler-rt as focused on just
exercising the runtime code as possible.

IIUC, the correct place for integration tests in general is somewhere
like test-suite, but I think it's a bit intimidating to some people to
add new tests there (Are there docs on this?). I suspect some of the
profiling related tests in compiler-rt are doing a bit much and should
graduate to a spot in the test-suite (but I don't have time to volunteer
to do the work, unfortunately).

On Wed, Feb 10, 2016 at 2:11 PM, Justin Bogner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> David Blaikie via cfe-dev <cfe-dev at lists.llvm.org> writes:
> > Recently had a bit of a digression in a review thread related to some
> tests
> > going in to compiler-rt (
> >
>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html
> > ) and there seems to be some disconnect at least between my
expectations
> > and reality. So I figured I'd have a bit of a discussion out here
on the
> > dev lists where there's a bit more visibility.
> >
> > My basic expectation is that the lit tests in any LLVM project except
the
> > test-suite are targeted tests intended to test only the functionality
in
> > the project. This seems like a pretty well accepted doctrine across
most
> > LLVM projects - most visibly in Clang, where we make a concerted
effort
> not
> > to have tests that execute LLVM optimizations, etc.
> >
> > There are exceptions/middle ground to this - DIBuilder is in LLVM, but
> > essentially tested in Clang rather than writing LLVM unit tests.
It's
> > somewhat unavoidable that any of the IR building code (IRBuilder,
> > DIBuilder, IR asm printing, etc) is 'tested' incidentally in
Clang in
> > process of testing Clang's IR generation. But these are seen as
> incidental,
> > not intentionally trying to cover LLVM with Clang tests (we don't
add a
> > Clang test if we add a new feature to IRBuilder just to test the
> IRBuilder).
> >
> > Another case with some middle ground are things like linker tests and
> > objdump, dwarfdump, etc - in theory to isolate the test we would
checkin
> > binaries (or the textual object representation lld had for a while,
etc)
> to
> > test those tools. Some tests instead checkin assembly and assemble it
> with
> > llvm-mc. Again, not to cover llvm-mc, but on the assumption that
llvm-mc
> is
> > tested, and just using it as a tool to make tests easier to maintain.
> >
> > So I was surprised to find that the compiler-rt lit tests seem to
diverge
> > from this philosophy & contain more intentional end-to-end tests
(eg:
> > adding a test there when making a fix to Clang to add a counter to a
> > function that was otherwise missing a counter - I'd expect that to
be
> > tested in Clang and that there would already be coverage in
compiler-rt
> for
> > "if a function has a counter, does compiler-rt do the right thing
with
> that
> > counter" (testing whatever code in compiler-rt needs to be
tested)).
> >
> > Am I off base here? Are compiler-rt's tests fundamentally
different to
> the
> > rest of the LLVM project? Why? Should they continue to be?
>
> I think there's a bit of grey area in terms testing the runtime -
> generally it's pretty hard to use the runtime without a fairly
> end-to-end test, so tests of the runtime often end up looking pretty
> close to an end-to-end test.
>
> That said, I don't think that should be used as an excuse to sneak
> arbitrary end-to-end tests into compiler-rt. We should absolutely write
> tests in clang and llvm that we're inputting what we expect to the
> runtime and try to keep the tests in compiler-rt as focused on just
> exercising the runtime code as possible.
>
Yes, we should not use compiler-rt tests as an excuse of not adding
clang/LLVM test. The latter should always be added if possible -- they are
platform independent and is the first level of defense.  runtime test's
focus is also more on the runtime lib itself and interaction between
 runtime, compiler, binutils and other tools.

David
>
> IIUC, the correct place for integration tests in general is somewhere
> like test-suite, but I think it's a bit intimidating to some people to
> add new tests there (Are there docs on this?). I suspect some of the
> profiling related tests in compiler-rt are doing a bit much and should
> graduate to a spot in the test-suite (but I don't have time to
volunteer
> to do the work, unfortunately).
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160210/7c4524dc/attachment.html>