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

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In https://reviews.llvm.org/D47913#1126958, @dblaikie wrote:

> 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


For simple cases, maybe.  But I expect to gradually test more and more complicated things and it's going to get quite awkward testing those kinds of things with unit tests.

Right now the way the unit tests test that spawning a process with a specific environment works is that the particular unit test checks for the presence of an environment variable and returns a magic value if it finds it.  Then the unit test spawns the top-level gtest executable again, filtered to this specific test.  It should detect the environment variable and return the magic value.

That's about on the borderline of how gross I would want a unit test to be.

Imagine you implemented a ptrace loop on top of our `sys::Execute` functions.  Then you might want to test that events that come through are as you would expect for a variety of different test programs, so now the unit tests start having inputs that you have to compile first.

I was hoping this utility could be useful in these types of situations as well.

Think of it sort of like a little miniature interpreted DSL that allow a parent and child process to communicate with each other.  Maybe this just further cements your idea that this is too high level for testing low level process launching though :), I was just viewing it as a special case of more complicated scenarios.

It does feel a little weird that this is the only 2 tests that are not unit tests, but idk.  If you don't think it's a good fit for a lit style test I can go back to the unit test model, but I do think the current Program unit tests are kind of pushing the limits of what unit tests should be doing.


https://reviews.llvm.org/D47913