uncaught exceptions thrown by a unit test should be reported as failing tests

[GoogleCodeExporter]

What steps will reproduce the problem?
1. write a test that throws an exception
2. build and run the test on mac and other platforms (mingw/cygwin on windows)
3. exception crashes the test app so no other tests are run or reported

What is the expected output? 
  [ RUN      ] ExceptionsAsErrors.TestThrowsInt
../test/test_exceptions.cc:12: Failure. Exception of type 'int' 
thrown by test.
              (Ideally, print name of exception type, 
               the "what()" string if it is a 
               subclass of std::exception, and a stack crawl.)
  [   FAILED ]  ExceptionsAsErrors.TestThrowsInt

What do you see instead?
  [ RUN      ] ExceptionsAsErrors.TestThrowsInt
  terminate called after throwing an instance of 'int'
  make[1]: *** [run-tests] Abort trap
  make: *** [all] Error 2

Please use labels and text to provide additional information.

this is release 1.1 of gTest

Original issue reported on code.google.com by keith....@gmail.com on 3 Oct 2008 at 9:03

• Blocking: #313

[GoogleCodeExporter]

Original comment by shiq...@gmail.com on 12 Oct 2008 at 3:20

• Added labels: OpSys-All, Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Use --gtest_catch_exceptions on windows. I think this option is not supported on
linux and mac however.

Original comment by dalius.d...@gmail.com on 23 Oct 2008 at 11:08

[GoogleCodeExporter]

I've created a small patch to handle this on linux / mac,
maybe it's useful to someone else. The patch is for gtest 1.3.0.
For me this is way better than aborting the test runner.

Original comment by thomas.j...@intra2net.com on 1 Jun 2009 at 11:54

Attachments:

• gtest-handle-uncaught-exceptions-on-linux.patch

[GoogleCodeExporter]

Thomas,

Thanks for the contribution! There are couple of things I Need to mention. 
First,
before we can accept code contributions from you we have to ask you to sign a 
CLA.
Would you mind to do that? You can find details in
http://code.google.com/p/googletest/wiki/GoogleTestDevGuide#Contributor_License_
Agreements.

Second, implementing this feature correctly is a bit complicated. We have the
requirements that an implementation must conform to:

* On platforms that support SEH, gtest has to intercept SEH correctly
* On all platforms gtest has to intercept C++ exceptions correctly.
* The interception of C++ exceptions must be controlled by the
GTEST_FLAG(catch_exceptions). If the flag is not set, gtest should not try to
intercept them.
* gtest must not intercept debug breakpoints (initiated by the DebugBreak() 
API) if
GTEST_FLAG(break_on_failure) is set. If it is not set, gtest must report debug
breakpoints as SEH exceptions.
* SEH exceptions should not be reported as C++ exceptions and vice versa.

If you are willing to take on the implementation, you are most welcome and we 
will be
happy to advise you.

Original comment by vladlosev on 2 Jun 2009 at 6:35

[GoogleCodeExporter]

Thanks, I've handed those documents down to your legal department for review.
I've missed your reply to my patch as I didn't knew I needed to "vote" this 
issue to
get email change notifications. Fixed it.

As I've never encountered SEH before, it really looks Windows specific to me and
seems to be supported well already. Also there is no such thing as a 
DebugBreak()
API in the linux world, atleast not know to me. 

If I understood all the requirements correctly, the only thing missing in the 
patch
is the "GTEST_FLAG(catch_exceptions)" handling, right?

Hopefully we can sort out the legalese part ;-)

Original comment by thomas.j...@intra2net.com on 9 Jun 2009 at 7:00

[GoogleCodeExporter]

You are right, SEH is Windows specific. Since we support Windows as well, we 
have to
take care of SEH. It is supported now but if we introduce C++ exception 
handling, the
C++ exception handling on Windows must not interfere with SEH handling. MS
implemented C++ exceptions via SEH in a way that intercepting a C++ exception 
can
also eat SEH exceptions passing through the same function. And on Windows, 
segfaults
are reported via SEH. :-(

The DebugBreak() on Windows simply executes the 'int 3' assembler command 
command.
The resulting interrupt is converted into a special SEH exception which the 
debugger
treats as a debug breakpoint.

The code for Windows when both SEH and C++ exceptions are enabled should look
something like this:

void Test::Run() {
  ...
  bool debug_break_intercepted = false;
  try {
    SehEnabledSetUp(&debug_break_intercepted);
  }
  catch (std::exception e) {
    // std exception processing
  }
  catch (...) {
    if (debug_break_intercepted) {
      throw;  // pass it on
    } else {
      // General exception processing.
    }
  }
  ...
}

void SehEnabledSetUp(bool* debug_break_intercepted) {
  __try {
    SetUp();
  } __except(ShouldProcessSEH(GetExceptionCode(), &debug_break_intercepted)) {
    AddExceptionThrownFailure(GetExceptionCode(), "SetUp()");
  }
}

bool ShouldProcessSEH(DWORD exception_code, bool* debug_break_detected) {
  if (GTEST_FLAG(catch_exceptions)) {
    switch (exception_code) {
      case EXCEPTION_BREAKPOINT:
        *debug_break_detected = true;
        return EXCEPTION_CONTINUE_SEARCH;
      case EXCEPTION_CPP:
        return EXCEPTION_CONTINUE_SEARCH;
      default:
        // This is neither C++ exception nor a break point. It's ours!
        EXCEPTION_EXECUTE_HANDLER;
  } else {
    return EXCEPTION_CONTINUE_SEARCH;
  }
}

Hopefully the entire sequence can be abstracted into a separate function so the
resulting code will not have to repeat it multiple times (SetUp,test body, 
TearDown,
TestCaseSetUp, TestCaseTearDown). Yep, this is a bit complicated, isn't it? :-(

Original comment by vladlosev on 15 Jun 2009 at 6:40

[GoogleCodeExporter]

Increased priority due to user requests.

Original comment by w...@google.com on 23 Dec 2009 at 7:00

• Added labels: Priority-High, Usability
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Vlad, do you want to close this now?  We can track the exception-death-test 
interaction in a separate issue, as it's different.

Original comment by w...@google.com on 8 Sep 2010 at 6:06

[GoogleCodeExporter]

I consider this issue unfinished. Leaving aside the problem of how to treat 
exceptions in death tests conceptually, there are some cases where this 
implementation fails, while the previous (lack of) implementation succeeded. 
E.g., EXPECT_DEATH(throw 1, "");  // Fails with the 'illegal return' message.

Original comment by vladlosev on 8 Sep 2010 at 2:34

[GoogleCodeExporter]

All apologies if this is off topic or inappropriate, but my interest in this 
issue to be able to use --gtest_catch_exceptions on linux. Is that issue 
tracked separately? This open request is the only hit I get if I search the 
issues for catch_exceptions.

I tried applying the patch above to gtest as shipped with gmock 1.5.0 but at 
this point it does not apply cleanly. 

Original comment by nate.ha...@gmail.com on 8 Sep 2010 at 3:02

[GoogleCodeExporter]

Vlad, I agree there is more work to be done.  I'm just saying that the 
*original* issue, which is "uncaught exceptions thrown by a unit test should be 
reported as failing tests", should be closed now, as gtest now does report 
uncaught exceptions as test failures.  By leaving it open, we lead people to 
think that gtest still cannot report uncaught exceptions as failures.

The example you gave (EXPECT_DEATH(throw 1, "");) is about the interaction 
between death tests and exceptions.  Even though it's a regression, it's better 
tracked by a separate issue.  I'll create a issue for that.

Original comment by w...@google.com on 8 Sep 2010 at 4:56

[GoogleCodeExporter]

Nate, it's not off-topic at all.  This is the right issue for your use case.  
--gtest_catch_exceptions now works on Linux (in the trunk head revision of 
gtest), thanks to Vlad's recent patch.  To try it, please check out the head 
revision using an SVN client (freely available).  Thanks.

Original comment by w...@google.com on 8 Sep 2010 at 4:59

[GoogleCodeExporter]

Created http://code.google.com/p/googletest/issues/detail?id=312 to track the 
death test issue.

Original comment by w...@google.com on 8 Sep 2010 at 5:02

[GoogleCodeExporter]

Sweet, thank you! Our team is really pleased with gtest, easily the best C++ 
unit testing framework we have found.

Original comment by nate.ha...@gmail.com on 8 Sep 2010 at 5:06

[GoogleCodeExporter]

Glad it works for you, Nate!

Original comment by w...@google.com on 8 Sep 2010 at 5:18

[GoogleCodeExporter]

Vlad has fixed the original issue in the trunk.  There are some remaining 
issues tracked by 312 and 313.

Original comment by w...@google.com on 8 Sep 2010 at 5:36

• Changed state: Fixed

[GoogleCodeExporter]

Vlad: did you forget to commit?
It did not work when I tried on my linux machine:

$ svn co http://googletest.googlecode.com/svn/trunk googletest
$ cd googletest/
$ echo "TEST(FailureTest, Nullpointer) { int* pi = NULL; *pi = 2;}" >> 
samples/sample1_unittest.cc && cd make/ && make && ./sample1_unittest 
--gtest_catch_exceptions
g++ -I../include -g -Wall -Wextra -c ../samples/sample1.cc
g++ -I../include -g -Wall -Wextra -c ../samples/sample1_unittest.cc
g++ -I../include -I.. -g -Wall -Wextra -c \
            ../src/gtest-all.cc
g++ -I../include -I.. -g -Wall -Wextra -c \
            ../src/gtest_main.cc
ar rv gtest_main.a gtest-all.o gtest_main.o
ar: creating gtest_main.a
a - gtest-all.o
a - gtest_main.o
g++ -I../include -g -Wall -Wextra -lpthread sample1.o sample1_unittest.o 
gtest_main.a -o sample1_unittest
Running main() from gtest_main.cc
[==========] Running 7 tests from 3 test cases.
[----------] Global test environment set-up.
[----------] 3 tests from FactorialTest
[ RUN      ] FactorialTest.Negative
[       OK ] FactorialTest.Negative (0 ms)
[ RUN      ] FactorialTest.Zero
[       OK ] FactorialTest.Zero (0 ms)
[ RUN      ] FactorialTest.Positive
[       OK ] FactorialTest.Positive (0 ms)
[----------] 3 tests from FactorialTest (0 ms total)

[----------] 3 tests from IsPrimeTest
[ RUN      ] IsPrimeTest.Negative
[       OK ] IsPrimeTest.Negative (0 ms)
[ RUN      ] IsPrimeTest.Trivial
[       OK ] IsPrimeTest.Trivial (0 ms)
[ RUN      ] IsPrimeTest.Positive
[       OK ] IsPrimeTest.Positive (0 ms)
[----------] 3 tests from IsPrimeTest (0 ms total)

[----------] 1 test from FailureTest
[ RUN      ] FailureTest.Nullpointer
Segmentation fault
$

Original comment by nils.mar...@gmail.com on 14 Sep 2010 at 7:05

[GoogleCodeExporter]

Don't forget to use --gtest_catch_exceptions. This option is not set by default 
right now.

Original comment by vladlosev on 14 Sep 2010 at 8:45

[GoogleCodeExporter]

Yes, I added that switch also: "... make && ./sample1_unittest 
--gtest_catch_exceptions
"
Looking through the svn log I can't see any commit relevant to this issue. You 
haven't switched repository or something like that?

Original comment by nils.mar...@gmail.com on 14 Sep 2010 at 8:54

[GoogleCodeExporter]

It's on SVN trunk in revision 480.

Original comment by vladlosev on 14 Sep 2010 at 9:33

[GoogleCodeExporter]

Hi Nils,

--gtest_catch_exceptions is for catching exceptions.  Your test program 
dereferences a null pointer.  That's undefined behavior.  Most likely it will 
crash the program via SIGSEGV, but it will unlikely throw any exception.  
That's why it's not caught by gtest.  Thanks.

Original comment by w...@google.com on 14 Sep 2010 at 4:05

[GoogleCodeExporter]

Re-opening this as I realized there are still important cases unhandled:

1. exceptions thrown in a test fixture ctor.
2. exceptions thrown in a test fixture dtor (probably not much we can do here).
3. exceptions thrown in SetUpTestCase() and TearDownTestCase(),
4. exceptions thrown in set-up/tear-down of test environments.
5. basically any place where user-written code could run -- did I miss any?

Original comment by w...@google.com on 28 Dec 2010 at 9:38

• Changed state: Accepted

[GoogleCodeExporter]

What makes you think these cases are not covered? 
http://code.google.com/p/googletest/source/browse/trunk/test/gtest_catch_excepti
ons_test.py?r=528 seems to indicate the opposite.

Original comment by vladlosev on 29 Dec 2010 at 6:36

[GoogleCodeExporter]

My bad.  I got the wrong impression from reading Joey's email, but I should've 
double-checked.  Sorry for the noise.

Original comment by w...@google.com on 29 Dec 2010 at 6:38

• Changed state: Fixed