[cfe-dev] [llvm-dev] Testing Best Practices/Goals (in the context of compiler-rt)

[Alexey Samsonov via cfe-dev]

On Thu, Feb 11, 2016 at 1:50 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, Feb 10, 2016 at 3:55 PM, Alexey Samsonov via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> I mostly agree with what Richard and Justin said. Adding a few notes
>> about the general strategy we use:
>>
>> (1) lit tests which look "end-to-end" proved to be way more convenient
>> for testing runtime libraries than unit tests.
>>
> We do have
>> the latter, and use them to provide test coverage for utility functions,
>> but we quite often accompany fix to the runtime library with
>> "end-to-end" small reproducer extracted from the real-world code that
>> exposed the issue.
>> Incidentally, this tests a whole lot of other functionality: Clang
>> driver, frontend, LLVM passes, etc, but it's not the intent of the test.
>>
>
> Indeed - this is analogous to the tests for, say, LLD that use llvm-mc to
> produce the inputs rather than checking in object files. That area is open
> to some discussion as to just how many tools we should rope in/how isolated
> we should make tests (eg: maybe building the json object file format was
> going too far towards isolation? Not clear - opinions differ). But the
> point of the test is to test the compiler-rt functionality that was
> added/removed/modified.
>
> I think most people are in agreement with that, while acknowledging the
> fuzzy line about how isolated we might be.
>

Yes.


>
>
>> These tests are sometimes platform-specific and poorly portable, but they
>> are more reliable (we make the same steps as the
>> user of the compiler), and serve the purpose of documentation.
>>
>> (2) If we change LLVM instrumentation - we add a test to LLVM. If we
>> change Clang code generation or driver behavior - we add
>> a test to Clang. No excuses here.
>>
>> (3) Sometimes we still add a compiler-rt test for the change in LLVM or
>> Clang: e.g. if we enhance Clang frontend to teach UBSan
>> to detecting yet another kind of overflow, it makes sense to add a test
>> to UBSan test-suite that demonstrates it, in addition to
>> Clang test verifying that we emit a call to UBSan runtime. Also,
>> compiler-rt test would allow us to verify that the actual error report
>> we present to the user is sane.
>>
>
> This bit ^ is a bit unclear to me. If there was no change to the UBSan
> runtime, and the code generated by Clang is equivalent/similar to an
> existing use of the UBSan runtime - what is it that the new compiler-rt
> test is providing? (perhaps you could give a concrete example you had in
> mind to look at?)
>

See r235568 (change to Clang) followed by r235569 (change to compiler-rt
test). Now, it's a cheat because I'm fixing test, not adding it. However, I
would have definitely added it, if it was missing. In this case, a change
to Clang
instrumentation (arguments passed to UBSan runtime callbacks) improved the
user-facing part of the tool, and compiler-rt test suite is a good place to
verify that.

You may argue that Clang test would have been enough (I disagree with
that), or that it qualifies as "adding coverage" (maybe).


>
>
>> (4) True, we're intimidated by test-suite :) I feel that current use of
>> compiler-rt test suite to check compiler-rt libs better follows
>> the doctrine described by David.
>>
>
> Which David? ;) (I guess David Li, not me)
>

Nope, paragraph 2 from your original email.


> I think maybe what could be worth doing would be separating out the
> broader/intentionally "end to end" sort of tests from the ones intended to
> test compiler-rt in relative isolation.
>

It's really hard to draw the line here, even some of compiler-rt unit tests
require instrumentation, therefore depend on new features of Clang/LLVM.
Unlike builtins, which are
trivial to test in isolation, testing sanitizer runtimes in isolation (w/o
compiler) is often hard to implement (we tried to do so for TSan, but found
unit tests extremely hard to write),
and is barely useful - compiler-rt runtimes don't consist of modules (like
LLVMCodeGen and LLVMMC for instance), and are never used w/o the compiler
anyway.


>
> Most importantly, I'd expect only the latter to run in a "make check-all"
> run, as we do for Clang/LLVM, etc.
>

And now we're getting to the goals :) Why would such a change be good? Do
you worry about the time it takes to execute the test suite?


>
>
>> Also, there's significant complexity in compiler-rt test suite that
>> narrows the tests executed
>> to those supported by current host.
>>
>
>
>
>
>>
>> On Wed, Feb 10, 2016 at 2:33 PM, Xinliang David Li via cfe-dev <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>>
>>>
>>> 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
>>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160212/3c476e84/attachment.html>