<div dir="ltr"><div dir="ltr">I understand that it's annoying to fix these tests but I strongly believe it will pay off in the long run. The motivation for this change is that we fixed tests that were XFAILED, but forgot to enable them again. Later they started failing and we didn't notice. This was especially a problem with lit where this information was not at all available. </div><div dir="ltr"><br></div><div dir="ltr">I created a patch that changes the dotest behavior to match what lit does.</div><div dir="ltr"><br></div><div dir="ltr"><a href="https://reviews.llvm.org/D55835">https://reviews.llvm.org/D55835</a><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 18, 2018 at 7:54 AM Pavel Labath <<a href="mailto:pavel@labath.sk">pavel@labath.sk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On 17/12/2018 22:40, Jonas Devlieghere via lldb-commits wrote:<br>
> Author: jdevlieghere<br>
> Date: Mon Dec 17 13:40:37 2018<br>
> New Revision: 349401<br>
> <br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=349401&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=349401&view=rev</a><br>
> Log:<br>
> [lit] Detect unexpected passes in lldbtest.<br>
> <br>
> This patch will have lit report unexpected passes when dotest reports at<br>
> least one XPASS and no failures.<br>
> <br>
> Modified:<br>
> lldb/trunk/lit/Suite/lldbtest.py<br>
> <br>
> Modified: lldb/trunk/lit/Suite/lldbtest.py<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=349401&r1=349400&r2=349401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=349401&r1=349400&r2=349401&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/lit/Suite/lldbtest.py (original)<br>
> +++ lldb/trunk/lit/Suite/lldbtest.py Mon Dec 17 13:40:37 2018<br>
> @@ -96,6 +96,10 @@ class LLDBTest(TestFormat):<br>
> if exitCode:<br>
> return lit.Test.FAIL, out + err<br>
> <br>
> + unexpected_test_line = 'XPASS'<br>
> + if unexpected_test_line in out or unexpected_test_line in err:<br>
> + return lit.Test.XPASS, ''<br>
> +<br>
> passing_test_line = 'RESULT: PASSED'<br>
> if passing_test_line not in out and passing_test_line not in err:<br>
> msg = ('Unable to find %r in dotest output:\n\n%s%s' %<br>
> <br>
> <br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
> <br>
<br>
It would be nice to have some notice before changes like this are <br>
implemented. I mean, I know there was some talk of this in the past, but <br>
that was months ago, so it wasn't really clear if/when is that going to <br>
happen. It didn't take me too long to clean up the remaining unexpected <br>
passes on my test configuration (and I found some pretty interesting <br>
things while doing that, for which I am grateful), but I am not sure <br>
this will be so simple for everyone.<br>
<br>
The other issue I have with this patch is that it creates a rift between <br>
how lit evaluates test results and how dotest does it (I don't know how <br>
you guys do this, but I still run dotest manually when I want to zero in <br>
on a single test failure). Now it can happen that someone runs <br>
"check-lldb", it reports a failure (unexpected success), and when the <br>
person runs the single test via dotest, it happily reports that <br>
everything is alright.<br>
<br>
I think it would be better to first teach dotest to treat "unexpected <br>
successes" as a "bad" result (i.e., to exit with non-zero exit code). <br>
Then, we can play around with how to convert that result into lit test <br>
states<br>
<br>
cheers,<br>
pavel<br>
</blockquote></div>