Fixing Windows client unit tests. They were broken in r1034 due to gMock and gTest upgrade.

Fixing Windows client unit tests.  They were broken in r1034 due to gMock and
gTest upgrade.  While fixing the broken tests I also used the opportunity to add
a few more tests that cover filter and callback execution, and nesting of
exception handlers.

[Ivan Penkov]

1. Windows client exception handler test fixes.
2. A few more tests that cover filter and callback execution, and exception
handler nesting.

Thanks,
-Ivan

[Ted Mielczarek]

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_nesting_test.cc (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:36: #include
"exception_handler_test.h"
Fix these includes to all be relative to src/.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:50: const char
kCallbackReturnsFalse[] = "callback_returns_false";
If you're going to use these all as std::string down below, maybe you should
just define them as const strings instead?

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:155:
&CrashHandlerFilter<false>) :
You can't just say &CrashHandlerFilter<filter_return_value> here, because it
needs to be statically decidable, right? Bummer.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_test.cc (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_test.cc:47: namespace testing {
Seems a little weird to put this in the testing namespace since that's Google
Test's namespace, but it is a gtest helper, so I'm not completely opposed.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_test.h (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_test.h:40: // code we need to disable
the exception handling done by GTest.
Don't use "we" in comments.

[Ivan Penkov]

Fixed.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_nesting_test.cc (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:36: #include
"exception_handler_test.h"
On 2012/10/18 11:36:30, Ted Mielczarek wrote:
> Fix these includes to all be relative to src/.

Done.  I also fixed the #includes in the rest of the files in that folder.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:50: const char
kCallbackReturnsFalse[] = "callback_returns_false";
On 2012/10/18 11:36:30, Ted Mielczarek wrote:
> If you're going to use these all as std::string down below, maybe you should
> just define them as const strings instead?

Unfortunately, the Google C++ style guide disallows it.  Only POD (Plain Old
Data) can be used as global static variables.  Here is the exact quote from the
C++ style guide:

"Static or global variables of class type are forbidden: they cause hard-to-find
bugs due to indeterminate order of construction and destruction.
 ...
 This rule completely disallows vector (use C arrays instead), or string (use
const char [])."

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:155:
&CrashHandlerFilter<false>) :
On 2012/10/18 11:36:30, Ted Mielczarek wrote:
> You can't just say &CrashHandlerFilter<filter_return_value> here, because it
> needs to be statically decidable, right? Bummer.

Correct.  The template functions referenced here must be instantiated and their
addresses known at compile time.  I could have used
&CrashHandlerFilter<filter_return_value> if 'filter_return_value' was a template
parameter of InstallExceptionHandlerAndCrash, however I have slight preference
for the current version, where InstallExceptionHandlerAndCrash is a regular
(non-template) function.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_test.cc (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_test.cc:47: namespace testing {
On 2012/10/18 11:36:30, Ted Mielczarek wrote:
> Seems a little weird to put this in the testing namespace since that's Google
> Test's namespace, but it is a gtest helper, so I'm not completely opposed.

I'm fine with changing this if you prefer something else.  Let me know.

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
File client/windows/unittests/exception_handler_test.h (right):

https://breakpad.appspot.com/489002/diff/1006/client/windows/unittests/except...
client/windows/unittests/exception_handler_test.h:40: // code we need to disable
the exception handling done by GTest.
On 2012/10/18 11:36:30, Ted Mielczarek wrote:
> Don't use "we" in comments.

Done.

[Ted Mielczarek]

LGTM

https://breakpad.appspot.com/489002/diff/6007/client/windows/unittests/except...
File client/windows/unittests/exception_handler_nesting_test.cc (right):

https://breakpad.appspot.com/489002/diff/6007/client/windows/unittests/except...
client/windows/unittests/exception_handler_nesting_test.cc:179:
std::string(kEndOfLine));
I guess the only other thing you could do to make this a little cleaner would be
to use a test fixture and assign these constants in the constructor. Doesn't buy
you much though.

https://breakpad.appspot.com/489002/diff/6007/client/windows/unittests/except...
File client/windows/unittests/exception_handler_test.h (right):

https://breakpad.appspot.com/489002/diff/6007/client/windows/unittests/except...
client/windows/unittests/exception_handler_test.h:33: namespace testing {
I don't really have a better suggestion here, and it's just bikeshedding, so
leave it.