[Lldb-commits] [lldb] r349401 - [lit] Detect unexpected passes in lldbtest.

[Pavel Labath via lldb-commits]

On 17/12/2018 22:40, Jonas Devlieghere via lldb-commits wrote:
> Author: jdevlieghere
> Date: Mon Dec 17 13:40:37 2018
> New Revision: 349401
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=349401&view=rev
> Log:
> [lit] Detect unexpected passes in lldbtest.
> 
> This patch will have lit report unexpected passes when dotest reports at
> least one XPASS and no failures.
> 
> Modified:
>      lldb/trunk/lit/Suite/lldbtest.py
> 
> Modified: lldb/trunk/lit/Suite/lldbtest.py
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=349401&r1=349400&r2=349401&view=diff
> ==============================================================================
> --- lldb/trunk/lit/Suite/lldbtest.py (original)
> +++ lldb/trunk/lit/Suite/lldbtest.py Mon Dec 17 13:40:37 2018
> @@ -96,6 +96,10 @@ class LLDBTest(TestFormat):
>           if exitCode:
>               return lit.Test.FAIL, out + err
>   
> +        unexpected_test_line = 'XPASS'
> +        if unexpected_test_line in out or unexpected_test_line in err:
> +            return lit.Test.XPASS, ''
> +
>           passing_test_line = 'RESULT: PASSED'
>           if passing_test_line not in out and passing_test_line not in err:
>               msg = ('Unable to find %r in dotest output:\n\n%s%s' %
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

It would be nice to have some notice before changes like this are 
implemented. I mean, I know there was some talk of this in the past, but 
that was months ago, so it wasn't really clear if/when is that going to 
happen. It didn't take me too long to clean up the remaining unexpected 
passes on my test configuration (and I found some pretty interesting 
things while doing that, for which I am grateful), but I am not sure 
this will be so simple for everyone.

The other issue I have with this patch is that it creates a rift between 
how lit evaluates test results and how dotest does it (I don't know how 
you guys do this, but I still run dotest manually when I want to zero in 
on a single test failure). Now it can happen that someone runs 
"check-lldb", it reports a failure (unexpected success), and when the 
person runs the single test via dotest, it happily reports that 
everything is alright.

I think it would be better to first teach dotest to treat "unexpected 
successes" as a "bad" result (i.e., to exit with non-zero exit code). 
Then, we can play around with how to convert that result into lit test 
states

cheers,
pavel