[llvm-dev] [RFC] Require PRs for XFAILing tests
Chris Bieneman via llvm-dev
llvm-dev at lists.llvm.org
Wed Sep 28 10:05:31 PDT 2016
I think one of the good things that would come out of this policy change is that we would have to audit the existing XFAILs. The most common XFAIL directive from my grep is "XFAIL: *”, which is the universal failure. I cannot think of any situation where a universally failing test should be in-tree unless it is a bug that someone is expecting to fix.
The only situation that I’ve come up with where filing a PR for an XFAIL isn’t reasonable would be for out-of-tree tests, although it is likely I’m not thinking of some edge case. To support out-of-tree code I think that we could adapt my patch to support multiple prefixes that denote different bug trackers. That would allow downstream users to annotate their tests for their bug trackers.
> On Sep 28, 2016, at 9:48 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>> On Sep 28, 2016, at 9:39 AM, Chris Bieneman via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> Hello LLVM-Dev,
>> The other day as I was digging through lldb’s test suite I noticed they support something kinda neat. In their python test harness, the attribute they use to denote expected failures supports a parameter for specifying the bug number. This got me thinking.
>> I believe that any test that is marked XFAIL is a bug, and we can use LIT to enforce that. So I wrote a patch (https://reviews.llvm.org/D25035 <https://reviews.llvm.org/D25035>) to add a feature to LIT which would support mapping XFAILs to PRs, and a flag to turn XFAILS without PRs into failures.
>> My proposal is to add this feature to LIT (after proper code review, and I still need to write tests for it), and then to annotate all our XFAILS with PRs. Once all the PRs are annotated I think we should enable this behavior by default and require PRs tracking all XFAILs.
> I think that’s a great idea!
> I wonder if there won’t be annoying cases that won’t fit well though, and a fake PR may be inserted just to please the system.
> Have you survey the existing XFAIL test to see if it would be the case? (I can’t imagine why but…)
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-dev