RFC: running tests in a separate process (issue2107044)
joeyo...@gmail.com
Description:
This code modifies gtest to run each test in a separate process. This
forces the OS to cleanup (release memory, release handles) after every
test. It also protects the main process against a test crashing. Please
see my original mailing list post for more information:
http://groups.google.com/group/googletestframework/browse_thread/thread/5690c5cc91065ce0#
This implementation:
- Changes class Test to pure-virtual
- Adds class NormalTest with the original Test::Run() behavior
- Adds class SeparateProcessTest with a new Run() that executes
TestBody() in a child process.
- Adds class ChildProcessPerThreadTestPartResultReportHelper which is a
listener (in the child) that sends each assertion results across the
pipe to the parent.
This effort started as a copy of Death Tests, so many of the same
restrictions apply. Some known issues:
- Has only been tested on my PC with VS2010
- Assumes SeparateProcessTest, doesn't have a way to choose between
SeparateProcessTest and NormalTest
- Doesn't compile using gtest-all.cc due to namespace collisions. For
now just gtest-separateprocess-test.cc to the *.vcproj
- In case of crash, the AssertHelper output will have a bogus file and
line number. There's no way to know what was excuting at the time of
crash.
- Run() doesn't work with SEH since the child's result forwarder is on
the stack. Uses the non-SEH version now.
- RecordProperty doesn't seem to work, I suppose it doesn't go through
the listener api?
- SetUp and TearDown run in both parent and child, they should probably
run only once (in the child)
- Some conditions (sometimes ctrl-c in parent) will hang the child
process. Use task manager to terminate it manually.
- TestWithParam is hardcoded because it needs to derive from either
NormalTest or SeparateProcessTest. This is not very clean.
- Debugging can be quite difficult, since it's attached to the parent
process. Need to run as a NormalTest if you expect to hit breakpoints
within the test.
- Didn't understand the factory, so I just tried my best to imitate how
the private bits get hidden.
Please take a look and comment.
Please review this at http://codereview.appspot.com/2107044/
Affected files:
include/gtest/gtest-separateprocess-test.h
include/gtest/gtest-test-part.h
include/gtest/gtest.h
include/gtest/internal/gtest-internal.h
include/gtest/internal/gtest-port.h
include/gtest/internal/gtest-separateprocess-test-internal.h
src/gtest-all.cc
src/gtest-death-test.cc
src/gtest-internal-inl.h
src/gtest-separateprocess-test.cc
src/gtest-test-part.cc
src/gtest.cc
Zhanyong Wan (λx.x x)
Thanks for contributing to gtest!
This is a big, complex change. I understand that it can be very
useful to those who need it. OTOH, I worry about the extra complexity
it brings to the framework. It can quickly get out of hand when this
feature interacts with other gtest features. My head is spinning
already. ;-)
Have you considered writing your test like this:
TEST(Foo, Bar) {
EXPECT_EXIT({
DoTest();
if (HasFailure()) {
PrintAllTestFailuresToStderr();
exit(1);
}
fprintf(stderr, "Separate-process test succeeded!\n");
exit(0);
}, ExitedWithCode(0), "Separate-process test succeeded!");
}
DoTest() does the real testing logic. Since it's in EXPECT_EXIT(), it
will be executed in a child process.
PrintAllTestFailuresToStderr() will print all failure messages for the
current TEST to stderr. It doesn't exist yet but should be easy to
implement.
If DoTest() finishes without crashing and without test failures, the
EXPECT_EXIT() will succeed.
If DoTest() finishes without crashing but with test failures, the
EXPECT_EXIT() will fail, and the failure messages from DoTest() will
be printed by EXPECT_EXIT().
If DoTest() crashes, the EXPECT_EXIT() will fail and print the crash
message, but the main test program can go on.
And we can define a macro to hide the boilerplate, such that you only
need to write
TEST(Foo, Bar) {
TEST_IN_NEW_PROCESS(DoTest());
}
I think this can be implemented with a fraction of the complexity of
your change, and is easy to learn for a new user. What do you think?
Thanks,
--
Zhanyong
Joey Oravec
> This is a big, complex change. I understand that it can be very
> useful to those who need it. OTOH, I worry about the extra complexity
> it brings to the framework.
Ya... it took quite a while for me to catch-up and write a first draft. I don't expect people to jump on this quickly. :)
> Have you considered writing your test like this:
>
> TEST(Foo, Bar) {
> EXPECT_EXIT({
> DoTest();
> if (HasFailure()) {
> PrintAllTestFailuresToStderr();
> exit(1);
> }
> fprintf(stderr, "Separate-process test succeeded!\n");
> exit(0);
> }, ExitedWithCode(0), "Separate-process test succeeded!");
> }
>
> DoTest() does the real testing logic. Since it's in EXPECT_EXIT(), it
> will be executed in a child process.
Yes I actually started that way. Sure a macro could do the job. It's not a clear decision, so I'll throw out a few thoughts about why I moved so far in the direction of new code:
- The RFC is describing a test which "hopefully works, might crash". Death tests describe a statement which "must terminate the process". It's subjective but I saw them quite differently.
- Assertion results normally get reported through the event listener interface which pinpoints the file, line number, and message for each failure. Routing error messages to stderr may result in a large block of text that the operator needs to read (and understand).
- To emphasize, with the boilerplate macro approach the parent wouldn't know the difference between a crash and an assertion failure. In both cases it only sees that the child exited with a code representing failure.
- The RFC seems to have less impact on writing ordinary TEST() code. I leaned away from macro decoration, assuming you might specify same-process versus separate-process with a command line argument, or a prefix/suffix on the name of the test.
- Even though RecordProperty() is broken in my first draft, it's fixable because the child is able to move data across the pipe to the parent. Would require similar changes to achieve the same with death tests.
- Same for things like test fixture SetUp() and TearDown(). Right now those are in Run() for the testcase and test, which seems like a good place for an abstraction. Would be nice to control if they run on the parent, child, or both without adding conditionals to the main path.
- Using things like stdin/stdout/stderr adds an assumption that the code under test won't touch them right? Likely safe, but I think the pipe and event listener interface are more immune against anything the code under test could do.
- Expecting exit means any exit(0) inside DoTest() results in a pass. If you're testing unknown code that's some risk of false positive.
- The boilerplate ends up as a fairly complex, deep set of macros. That's ok but there are some natural followups like "test should complete within X time, or kill the child" that we should consider. They might be easier with the proposal than with increasingly deep macros.
- Unless DoTest is a function it'll need something like ReturnSentinel in today's death tests. This is solvable, but also scares me away from macros.
- I don't need death tests inside a separate process test, but that seems like a restriction for what you could put in DoTest if it's implemented with an expect exit. No matter what, there will be some subtle restrictions and assumptions.
So... thats what motivated me. That's a lot so I'll let you digest for a few days.
-joey
Zhanyong Wan (λx.x x)
This is an auto-generated response.
I have moved to another project and am no longer working on Google
Test and Google Mock. Please send your Google Test questions to
googletes...@googlegroups.com, and your Google Mock questions
to googl...@googlegroups.com. Thanks,
--
Zhanyong
On Thu, Nov 11, 2010 at 9:55 PM, Matt Frantz <matthew....@gmail.com> wrote:
> Hi, Zhanyong,
> I'm trying to do exactly what you are describing (in order to isolate
> my tests from one another when they touch global/static state). You
> say that PrintAllTestFaluresToStderr() "doesn't exist yet but should
> be easy to implement." Does that mean easy for you or easy for
> me? :) I'm scanning through the parts of gtest.h that I've not had
> to look at before. Things like TestPartResultReporterInterface. I'm
> getting that feeling that I'm in the weeds.
>
> Let me just stop to praise the usefulness of EXPECT_EXIT for testing
> [legacy] code that makes use of irreversible global state! If I could
> see which of my EXPECT's are failing, I'd be home free.
>
> Thanks,
> Matt
>
> On Sep 9, 10:32 am, Zhanyong Wan (λx.x x) <w...@google.com> wrote:
>> Hi, Joey,
>>
>> Thanks for contributing to gtest!
>>
>> This is a big, complex change. I understand that it can be very
>> useful to those who need it. OTOH, I worry about the extra complexity
>> it brings to the framework. It can quickly get out of hand when this
>> feature interacts with other gtest features. My head is spinning
>> already. ;-)
>>
>> Have you considered writing yourtestlike this:
>>
>> TEST(Foo, Bar) {
>> EXPECT_EXIT({
>> DoTest();
>> if (HasFailure()) {
>> PrintAllTestFailuresToStderr();
>> exit(1);
>> }
>> fprintf(stderr, "Separate-processtestsucceeded!\n");
>> exit(0);
>> }, ExitedWithCode(0), "Separate-processtestsucceeded!");
>>
>> }
>>
>> DoTest() does the real testing logic. Since it's in EXPECT_EXIT(), it
>> will be executed in a child process.
>>
>> PrintAllTestFailuresToStderr() will print all failure messages for the
>> currentTESTto stderr. It doesn't exist yet but should be easy to
>> implement.
>>
>> If DoTest() finishes without crashing and withouttestfailures, the
>> EXPECT_EXIT() will succeed.
>>
>> If DoTest() finishes without crashing but withtestfailures, the
>> EXPECT_EXIT() will fail, and the failure messages from DoTest() will
>> be printed by EXPECT_EXIT().
>>
>> If DoTest() crashes, the EXPECT_EXIT() will fail and print the crash
>> message, but the maintestprogram can go on.
>>
>> And we can define a macro to hide the boilerplate, such that you only
>> need to write
>>
>> TEST(Foo, Bar) {
>> TEST_IN_NEW_PROCESS(DoTest());
>>
>> }
>>
>> I think this can be implemented with a fraction of the complexity of
>> your change, and is easy to learn for a new user. What do you think?
>> Thanks,
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Sep 7, 2010 at 6:08 PM, <joeyora...@gmail.com> wrote:
>> > Reviewers: ,
>>
>> > Description:
>> > This code modifies gtest to run eachtestin a separate process. This
>> > forces the OS to cleanup (release memory, release handles) after every
>> >test. It also protects the main process against atestcrashing. Please
>> > see my original mailing list post for more information:
>> >http://groups.google.com/group/googletestframework/browse_thread/thre...
>>
>> > This implementation:
>>
>> > - Changes classTestto pure-virtual
>> > - Adds class NormalTest with the originalTest::Run() behavior
>> > - Adds class SeparateProcessTest with a new Run() that executes
>> > TestBody() in a child process.
>> > - Adds class ChildProcessPerThreadTestPartResultReportHelper which is a
>> > listener (in the child) that sends each assertion results across the
>> > pipe to the parent.
>>
>> > This effort started as a copy ofDeathTests, so many of the same
>> > Please review this athttp://codereview.appspot.com/2107044/