RFC: running tests in a separate process

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.

[wan, 2010-09-09 18:32:46]

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 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,

On Tue, Sep 7, 2010 at 6:08 PM,  <joeyoravec@gmail.com> wrote:
> Reviewers: ,
>
> 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/5690c...
>
> 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

[joeyoravec, 2010-09-10 03:44:23]

On Sep 9, 2010, at 2:32 PM, Zhanyong Wan (λx.x x) wrote:

> 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