[PATCH] D47913: [Support] Introduce a new utility for testing child process execution

[David Blaikie via llvm-commits]

Hey Zach,

Thanks for looking into/improving test coverage!

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.

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.

- Dave

On Thu, Jun 7, 2018 at 3:31 PM Zachary Turner via Phabricator <
reviews at reviews.llvm.org> wrote:

> zturner created this revision.
> zturner added reviewers: echristo, dblaikie.
> Herald added a subscriber: mgorny.
>
> 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.
>
> 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.
>
> 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.
>
> 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}`
>
>
> https://reviews.llvm.org/D47913
>
> Files:
>   llvm/CMakeLists.txt
>   llvm/test/Support/execute.test
>   llvm/test/lit.cfg.py
>   llvm/utils/trace/CMakeLists.txt
>   llvm/utils/trace/Trace.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180608/b7ca9452/attachment.html>