<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 21, 2014 at 12:27 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div class="">On Wed, May 21, 2014 at 7:22 AM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>On Tue, May 20, 2014 at 7:34 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>We use lit tests for diagnostic messages. Currently they use FileCheck, but it probably makes sense to implement an analog of Clang's -verify in clang-tidy.</div>


<div><br></div>As for the tests for fix-its, I consider unit tests to be more readable for this purpose.</div></blockquote><div><br></div></div><div>I don't understand this. What could be more readable than something like:</div>


<div><br></div><div><div>namespace {</div><div>} // namespace asdf</div><div>// CHECK:      namespace {</div><div>// CHECK-NEXT: } // namespace</div></div><div><br></div><div>?? I certainly prefer it hands-down to:</div>

<div><div>
<br></div><div><div>+  EXPECT_EQ("namespace {\n"</div><div>+            "} // namespace",</div><div>+            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"</div><div>+                                                  "} // namespace asdf"));</div>


</div><div><br></div></div><div>(actually I think FileCheck's default matching of allowing a match of a partial line could be an issue here, but that can be worked around with a regex. I think that even adding a CHECK-EXACT directive or similar to support this in a very readable way might be worth it for this sort of test)</div>


<div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">They are also more isolated and closer to the check itself.</div>


</blockquote><div><br></div></div><div>I don't understand this. The check is either 1) performing a particular source code transformation or 2) providing a diagnostic message about a questionable construct. In case 1) you want to verify that the output takes a particular plaintext form, i.e. exactly what a FileCheck does. In case 2) you can avoid rolling your own -verify and use the simpler but almost surely sufficient facility built into FileCheck <a href="http://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-expressions" target="_blank">http://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-expressions</a></div>

<div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"> In a lit test, if we verify the full text of the output, the FileCheck tests become quite bulky.</div>


</blockquote><div> </div></div></div><div>Could you give an example? It is generally possible to create minimal test cases so that the actual output (and the input, in fact) are quite compact. yaml2obj is probably a worst case in this respect but we've still found it easy to keep the tests small and readable. E.g.</div>


<div><br></div><div><a href="http://reviews.llvm.org/diffusion/L/browse/llvm/trunk/test/Object/yaml2obj-elf-symbol-LocalGlobalWeak.yaml" target="_blank">http://reviews.llvm.org/diffusion/L/browse/llvm/trunk/test/Object/yaml2obj-elf-symbol-LocalGlobalWeak.yaml</a><br>


</div><div><a href="http://reviews.llvm.org/diffusion/L/browse/llvm/trunk/test/Object/yaml2obj-elf-bits-endian.test" target="_blank">http://reviews.llvm.org/diffusion/L/browse/llvm/trunk/test/Object/yaml2obj-elf-bits-endian.test</a><br>

</div><div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Another reason to put these checks in the unit tests, is that each new lit test file has non-trivial run-time overhead.</div>


</blockquote><div><br></div></div><div>Personally I've never found myself waiting on an incremental lit test run (i.e. rerunning a particular failing test or relevant test directory for the code I'm working on). I *have* found myself waiting significant amounts of time for a unittest (gtest) compile and link in that same scenario.</div>

</div></div></div></blockquote><div><br></div></div><div>Interesting. I have pretty much the reverse experience. I think it's mainly because running a single gtest is very simple (--gtest_filter=*MyTest*) while running a single lit test is a nightmare.</div>
</div></div></div></blockquote><div><br></div><div>Interesting. Are you aware that you can do `$BUILD/bin/llvm-lit $SRC/test/Foo` or `$BUILD/bin/llvm-lit $SRC/test/Foo/foobar.test` and it will automatigically run all tests in the indicated directory, or just the selected test? Until I learned that, I found it a nightmare, but it's quite pleasant once that trick is known. My typical iteration with yaml2obj was something like</div>
<div><br></div><div>$ ninja ./bin/yaml2obj && ./bin/llvm-lit ../llvm/test/Object/yaml2obj*</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> Even worse, if you have a FileCheck test that tests many different things (take for example the static analyzer tests, which I happen to have cursed at in the past weeks), the only way I found to isolate a single cause for debugging is to copy-and-paste the code.</div>
</div></div></div></blockquote><div><br></div><div>Well, tests that test many different things are a pain independently of the testing methodology or setup ;)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> That's really ineffective. On top of that, the results of FileCheck tests are often really hard to figure out (you have to basically look at the command, figure out which file is actually checked, which is often a copy of a copy of a file somewhere, and then use regexps on that file to see what might have changed so that whatever was not found didn't appear).</div>

<div><br></div><div>I grant you that FileCheck tests are nicer to read if the tests are at the integration level anyway, though, which makes the balance often a tough call (for example, for clang-format we've found that we'd much rather have FileCheck tests for most tests nowadays, but haven't gotten around to converting them yet).</div>
</div></div></div></blockquote><div><br></div><div>Interesting. I thought that clang-format was a natural case for unittests due to the need for juggling the options and styles for different verifications, which would be a pain with FileCheck.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> So I'm actually with you in this specific case. I'd want to have people most probably maintaining the code to have some headway here, as insufficient tests will affect them the most.</div>
<div><br></div><div>And finally, you're probably not trying to run the lit stuff on windows, where starting a process has a significant overhead.</div></div></div></div></blockquote><div><br></div><div>I admit I'm not on windows, but I hear the overhead there is bad; hopefully lit can be improved to make things speedier there. It must be really bad if it negates the parallelism advantage from lit (IIRC our gtest configuration does not run tests in parallel).</div>
<div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">
<div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Cheers,</div><div>/Manuel</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><font color="#888888">
<div><br></div><div>-- Sean Silva</div></font></span><div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div dir="ltr">
<div><div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 20, 2014 at 7:32 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br>



<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Is there a particular reason that this is being tested with unittests instead of the usual lit tests? This kind of thing seems like it would be a lot more readable as a lit test.<span><font color="#888888"><div>



<br></div><div>-- Sean Silva</div></font></span></div></blockquote></div>
</div></div></div></div>
</blockquote></div></div><br></div></div>
<br></div><div class="">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></div></blockquote></div><br></div></div>
</blockquote></div><br></div></div>