<div dir="ltr">Hey Zach,<br><br>Thanks for looking into/improving test coverage!<br><br>My knee-jerk/gut reaction to the idea of adding a tool binary for testing a fairly low-level API like this is that it doesn't /feel/ quite right, and I've looked over the patch a bit but not in quite enough detail to provide really actionable/specific feedback yet.<br><br>My broad/high-level question would be: Could the existing unit test approach be made more/sufficiently readable by implementing helper functions (essentially the kind of code that's in the new tool you're introducing - providing that as utility functions with parameters, rather than a program with command line parameters). It seems like that might be a simpler alternative, but I'm certainly not 100% sure of it.<br><br>- Dave<br><br><div class="gmail_quote"><div dir="ltr">On Thu, Jun 7, 2018 at 3:31 PM Zachary Turner 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">zturner created this revision.<br>
zturner added reviewers: echristo, dblaikie.<br>
Herald added a subscriber: mgorny.<br>
<br>
I was auditing our process launching API and looking at how well it was tested, and I noticed that none of the functionality related to I/O redirection was tested at all.  I also noticed that all testing was done via unit tests and they are all extremely hard to understand and follow.<br>
<br>
The approach here is more LLVMish, using a tool to dump some output and then FileCheck it.  Since I was specifically trying to test re-direction here, the only support I've added so far is for specifying command line arguments that describe how to redirect stdout and stderr, and I wrote some tests which I believe are significantly easier to understand than the existing unit tests.<br>
<br>
There are plenty of other interesting opportunities for exploration with a tool such as this.  For example, LLDB (or any kind of tracing utility) could use it as a kind of "strace" equivalent where it logs all of its events and we could FileCheck that.<br>
<br>
In a followup patch, I plan to use this mechanism to test that launching a process with a specific environment works.  I am going to do this by introducing meta-variable substitutions into the command language, so that you can write something like `write stdout $ENV{PATH}`<br>
<br>
<br>
<a href="https://reviews.llvm.org/D47913" rel="noreferrer" target="_blank">https://reviews.llvm.org/D47913</a><br>
<br>
Files:<br>
  llvm/CMakeLists.txt<br>
  llvm/test/Support/execute.test<br>
  llvm/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a><br>
  llvm/utils/trace/CMakeLists.txt<br>
  llvm/utils/trace/Trace.cpp<br>
<br>
</blockquote></div></div>