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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a subscriber: zturner.
dblaikie added a comment.

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


https://reviews.llvm.org/D47913