Add Test::DisableThisTest() member function

Add the functionality to disable a test at runtime.

unit test included.

[Zhanyong, 2008-07-25 04:48:48]

Hi, M-A,

Please see my comments below.  Thanks!

http://codereview.appspot.com/2611/diff/5/7
File include/gtest/gtest.h (right):

http://codereview.appspot.com/2611/diff/5/7#newcode229
Line 229: // Returns true if the current test has been dynamically disabled.
Why do you emphasize "dynamically" here?  Won't the function return true too
when the test is statically disabled?

http://codereview.appspot.com/2611/diff/5/7#newcode275
Line 275: void DisableThisTest();
This function needs to be static such that it can be called in subroutines that
are not members of the test fixture.

http://codereview.appspot.com/2611/diff/5/8
File src/gtest.cc (right):

http://codereview.appspot.com/2611/diff/5/8#newcode1687
Line 1687: void Test::DisableThisTest() {
Style nit: add a comment header on what this does.

Also, group this with IsDisabled()?

http://codereview.appspot.com/2611/diff/5/8#newcode1689
Line 1689: set_is_disabled(true);
We need to make this thread-safe, as the user may call this from multiple
threads concurrently.

You can use the synchronization primitives in
include/gtest/internal/gtest-port.h.  They have a dummy implementation for now
but will have a real implementation later.

http://codereview.appspot.com/2611/diff/5/8#newcode1810
Line 1810: // We will run the test only if SetUp() had no fatal failure.
Update this comment.

http://codereview.appspot.com/2611/diff/5/8#newcode1811
Line 1811: if (!HasFatalFailure() && !Test::IsDisabled()) {
The two calls should have the same style: use Test:: consistently.

http://codereview.appspot.com/2611/diff/5/8#newcode1836
Line 1836: // We will run the test only if SetUp() was successful.
The same.

http://codereview.appspot.com/2611/diff/5/8#newcode1837
Line 1837: if (!HasFatalFailure() && !Test::IsDisabled()) {
The same.

http://codereview.appspot.com/2611/diff/5/8#newcode1856
Line 1856: bool Test::IsDisabled() {
Add a comment header.

http://codereview.appspot.com/2611/diff/5/8#newcode1858
Line 1858: is_disabled();
We should make reading is_disabled() thread-safe.

http://codereview.appspot.com/2611/diff/5/8#newcode2006
Line 2006: // Runs the test only if the constructor of the test fixture didn't
Update this.

http://codereview.appspot.com/2611/diff/5/6
File test/gtest_unittest.cc (right):

http://codereview.appspot.com/2611/diff/5/6#newcode108
Line 108: TEST_F(TestDisabledInConstructor, DoesntRun) {
We should make sure the test's failures (if any) are ignored if the test is
disabled.

Therefore this won't tell us that the test body isn't run.

http://codereview.appspot.com/2611/diff/5/6#newcode126
Line 126: }
Need to test:

- calling DisableThisTest() and IsDisabled() from a function that's not a member
of the test fixture (nor the TEST itself).
- calling DisableThisTest() in the test ctor prevents SetUp(), the test body,
and TearDown() to be run.
- calling DisableThisTest() in SetUp() prevents the test body to be run, but
TearDown() should still be run.
- both statically disabled tests and dynamically disabled tests are included in
the count printed at the end of the test program.
- test failures before calling DisableThisTest() are ignored.
- test failures after calling DisableThisTest() are ignored.
- Google Test should print the names of the dynamically disabled tests in the
end.
- calling DisableThisTest() and IsDisabled() from multiple threads concurrently
is OK.  For this you need to modify gtest_stress_test.cc.

[DO NOT USE, 2008-07-25 15:49:27]

Will send a new patch with the changes done. Some unit tests are still missing
but the rest has been fixed.

http://codereview.appspot.com/2611/diff/5/6
File test/gtest_unittest.cc (right):

http://codereview.appspot.com/2611/diff/5/6#newcode108
Line 108: TEST_F(TestDisabledInConstructor, DoesntRun) {
On 2008/07/25 04:48:48, Zhanyong wrote:
> We should make sure the test's failures (if any) are ignored if the test is
> disabled.
> 
> Therefore this won't tell us that the test body isn't run.

I clarified the expectations in DisableThisTest(), so testing this is
unnecessary, i.e. the test will fail if the test body is run.

http://codereview.appspot.com/2611/diff/5/6#newcode126
Line 126: }
On 2008/07/25 04:48:48, Zhanyong wrote:
> Need to test:
> 
> - calling DisableThisTest() and IsDisabled() from a function that's not a
member
> of the test fixture (nor the TEST itself).

Not done. I need to find out how to do that.

> - calling DisableThisTest() in the test ctor prevents SetUp(), the test body,
> and TearDown() to be run.

Done.

> - calling DisableThisTest() in SetUp() prevents the test body to be run, but
> TearDown() should still be run.

Done.

> - both statically disabled tests and dynamically disabled tests are included
in
> the count printed at the end of the test program.

Not done. I need to find out how to do that.

> - test failures before calling DisableThisTest() are ignored.

N/A

> - test failures after calling DisableThisTest() are ignored.

N/A

> - Google Test should print the names of the dynamically disabled tests in the
> end.

Not done. I don't think this is in the scope of this change.

> - calling DisableThisTest() and IsDisabled() from multiple threads
concurrently
> is OK.  For this you need to modify gtest_stress_test.cc.

Not done. There is no vcproj that includes this source file.