[llvm-commits] [llvm] r97726 - in /llvm/trunk: test/lit.cfg utils/lit/lit/TestRunner.py utils/lit/lit/TestingConfig.py

[Daniel Dunbar]

On Thu, Mar 4, 2010 at 5:43 AM, John McCall <rjmccall at apple.com> wrote:
>
> On Mar 4, 2010, at 5:13 AM, Daniel Dunbar wrote:
>> I'm not sure I like this change. Do we really need this level of complexity?
>>
>> It would be more in keeping with the current style to just have something like
>>   // REQUIRES: keyword1, keyword2, ...
>
> I think there are three classes of tests here:
> 1)  Tests with a single RUN line.  REQUIRES would be marginally better than IF for this.
> 2)  Tests with a lot of inter-dependent RUN lines.  On the one hand, IF is pretty ugly for these tests;  on the other hand, I think the consensus is that we're trying to kill these off and replace them with FileCheck.
> 3)  Tests with a lot of independent RUN lines.  IF allows these to degrade gracefully.  For example, there are some nice Clang IR-gen tests that pass the same file through FileCheck with multiple targets;  we'd either have to split these arbitrarily into separate tests (guaranteeing that improvements to one won't get made to the other) or turn the entire test off when any of the targets are missing.

This is a *very* small class of tests, because that should only apply
to tests that actually REQUIRE some specific target, and those tests
should have been testing for target specific things anyway.

>> There are also stylistic arguments against allowing conditional
>> functionality in tests. It is nice to say "foo passed" or "foo
>> failed", but if you allow parts of a test to be conditional then you
>> need to say "foo passed with options bla"...
>
> Well, but those aren't the options.  The options are "foo passed but some parts of it were suppressed" and "foo couldn't be run at all".
>
> There are also stylistic advantages to having all the RUN lines in a test be independent, which this encourages.

Perhaps, but thats not really a style we follow.

The LLVM tests have been using the simple style for a long time and it
seems to work fine there, I really don't like having a test be
conditional, it just makes it more confusing to run tests and
understand their output. Given that we generally should be avoiding
having tests which require particular things (except for large swaths
of functionality, like a backend), I think REQUIRES is more
reasonable.

I propose starting with adding REQUIRES, which is consistent with the
other test options (XFAIL and XTARGET), and see how it works out.

 - Daniel

> John.