<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sun, Jul 30, 2017 at 10:07 PM Dean Michael Berris via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberris marked an inline comment as done.<br>
dberris added inline comments.<br>
<br>
<br>
================<br>
Comment at: test/xray/TestCases/Linux/quiet-start.cc:5-8<br>
+// RUN: XRAY_OPTIONS="patch_premain=true verbosity=1" %run %t 2>&1 | \<br>
+// RUN:    FileCheck %s --check-prefix NOISY<br>
+// RUN: XRAY_OPTIONS="patch_premain=true verbosity=0" %run %t 2>&1 | \<br>
+// RUN:    FileCheck %s --check-prefix QUIET<br>
----------------<br>
dblaikie wrote:<br>
> Does this test the default (the comment says it checks what happens when there are no XRay instrumentation sleds - but the code seems to explicitly specify verbosity, rather than relying on any default based on the presence/absence of sleds? (unless that would override the XRAY_OPTIONS?))<br>
><br>
> Seems like it might be better to phrase this whole change, etc in terms of tying it into the verbosity level & mention as an aside that this means it defaults to off. (& I don't think it's necessary to test that default if the compiler-rt verbosity work already handles the default, tests & documents it, etc))<br>
Added a test that will use an empty XRAY_OPTIONS environment variable, so that we're testing all the defaults.<br></blockquote><div><br>If the default verbosity is already tested elsewhere, I wouldn't bother testing them here. Good to keep things focused. (alternatively I wouldn't bother testing the explicit handling if that's tested elsewhere & might use the default in this test, etc)<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D35789" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35789</a><br>
<br>
<br>
<br>
</blockquote></div></div>