search

tr00per / googletest

Automatically exported from code.google.com/p/googletest
0 stars 0 forks source link

EXPECT_FATAL_FAILURE needs more comments and tests #97

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
On Fri, Jan 2, 2009 at 9:08 AM, Josh Kelley <joshkel@gmail.com> wrote:
>
> What is the purpose of the GTestExpectFatalFailureHelper classes in
> gtest-spi.h?  Will it break anything if I remove them and invoke
> statement directly?

There are two kinds of assertions in Google Test: EXPECT_* can be used
in *any* function, while ASSERT_* can only be used in functions
returning void:

http://code.google.com/p/googletest/wiki/GoogleTestAdvancedGuide#Assertion_Place
ment

GTestExpectFatalFailureHelper is necessary for allowing the following
code in functions that return a value:

  EXPECT_FATAL_FAILURE(ASSERT_TRUE(false), "");

I will update the comments and add a test to verify this.

Original issue reported on code.google.com by shiq...@gmail.com on 5 Jan 2009 at 6:50

GoogleCodeExporter commented 9 years ago 
2009/1/6 Josh Kelley <joshkel@gmail.com>:
> It looks like the problem is that, under C++Builder, EXPECT_FATAL_FAILURE
> cannot even refer to static members or member types.  This is causing
> ExpectFailureTest in gtest_unittest.cc to fail to compile.  For now, to work

Uh, that would be a bug in C++ Builder.

> around this, I removed the GTestExpectFatalFailureHelper class from the
> EXPECT_FATAL_FAILURE... macros and invoke statement directly.
>
> I've been working on a full C++Builder port which I'll submit once I'm

Cool.  I'm looking forward to it.

> done.  How would you like me to handle this shortcoming of C++Builder?  I
> guess changing ASSERT... to use exceptions would be one approach, but it
> would be very invasive.

Correct.  Google Test needs to work where exceptions are disabled, so
this is not a good option.

> Since EXPECT_FATAL_FAILURE's design adds
> limitations on using members of the current object and since ASSERT...
> already requires that it only be used from within a void function, would it
> be better to remove GTestExpectFatalFailureHelper entirely?

There's another important reason why the helper class is necessary,
which I forgot to mention.  Without the helper class,

 EXPECT_FATAL_FAILURE(ASSERT_TRUE(false), "");
 ... the rest of the test ...

will cause the test to abort before the rest of the test gets a chance
to be executed.  This could cause important validations to be skipped
and hide bugs.  We can tell the user about this got-cha, but the macro
will be too easy to misuse.

Which version of C++ Builder are you using?  Is it possible that a
newer version has fixed the bug?  Have you contacted CodeGear about a
possible fix?

Unless the bug is fixed in C++ Builder, my suggestion to you is to
disable this macro when compiled with C++ Builder.  Normal users won't
need it.  Only those who extend Google Test or build things on top of
it do.  Therefore this should not be a big limitation.

Original comment by shiq...@gmail.com on 6 Jan 2009 at 7:08

GoogleCodeExporter commented 9 years ago 
Tests added.

Original comment by zhanyong...@gmail.com on 22 Feb 2009 at 5:41

GoogleCodeExporter commented 9 years ago 
I have just downloaded the latest version of googletest (1.6.0) and I am 
getting the ICE when compiling the project on line number 4807:

  EXPECT_FATAL_FAILURE(ASSERT_LE(UnprintableChar('y'), UnprintableChar('x')),
                       "1-byte object <79>");

I'm guessing this is a typo, because just above with the check for #ifndef 
__BORLANDC__, there are two lines:

  EXPECT_FATAL_FAILURE(ASSERT_NE(UnprintableChar('x'), UnprintableChar('x')),
                       "1-byte object <78>");
  EXPECT_FATAL_FAILURE(ASSERT_LE(UnprintableChar('y'), UnprintableChar('x')),
                       "1-byte object <78>");

which has the ASSERT_LE.

I am running C++ Builder XE2 Update 4.

Please fix, or advise, or was this tested with C++ Builder XE4?

Thanks :)

Original comment by SideFFec...@gmail.com on 21 May 2013 at 4:28