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

Wed Feb 17 08:45:47 PST 2016

On Fri, Feb 12, 2016 at 5:43 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:

>
>
> 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.
>

Right, I think the difference here is "if it was missing" - the test case
itself seems like it could be a reasonable one (are there other tests of
the same compiler-rt functionality? (I assume the compiler-rt functionality
is the implementation of sadd/ssub?))

> 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.
>

This seems like the problematic part - changes to LLVM improve the
user-facing part of Clang, but we don't add end-to-end tests of that, as a
general rule. I'm trying to understand why the difference between that and
compiler-rt

>
>

> 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/20160217/dafc4a89/attachment.html>