diff --git a/docs/gmock_cheat_sheet.md b/docs/gmock_cheat_sheet.md index 7b863e9ee..39947039a 100644 --- a/docs/gmock_cheat_sheet.md +++ b/docs/gmock_cheat_sheet.md @@ -225,10 +225,6 @@ mock objects when running a larger test suite: ```cpp Mock::CheckLeakInstant(); ``` -You can use this for example by adding following to the end of your test cases: -```cpp -ASSERT_EQ(Mock::CheckLeakInstant(),true); -``` ## Mock Classes diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h index bf5f970b2..b9bd2a8ae 100644 --- a/googlemock/include/gmock/gmock-spec-builders.h +++ b/googlemock/include/gmock/gmock-spec-builders.h @@ -372,8 +372,7 @@ class GTEST_API_ Mock { // Tells Google Mock to instantly check for leftover mock objects and report // them if there are. - // Returns false if there are leftover mock objects and true otherwise. - static bool CheckLeakInstant(void) + static void CheckLeakInstant(void) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex); // Verifies and clears all expectations on the given mock object. diff --git a/googlemock/src/gmock-spec-builders.cc b/googlemock/src/gmock-spec-builders.cc index 8496a69eb..eaa6e4e8f 100644 --- a/googlemock/src/gmock-spec-builders.cc +++ b/googlemock/src/gmock-spec-builders.cc @@ -453,10 +453,12 @@ static CallReaction intToCallReaction(int mock_behavior) { } // namespace internal // Class Mock. + namespace { class MockObjectRegistry; -bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg); +bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg, + std::string& msg); typedef std::set FunctionMockers; @@ -487,23 +489,25 @@ class MockObjectRegistry { typedef std::map StateMap; // This destructor will be called when a program exits, after all - // tests in it have been run. By then, there should be no mock - // object alive. Therefore we report any living object as test + // tests in it have been run. By then, there should be no mock + // object alive. Therefore we report any living object as test // failure, unless the user explicitly asked us to ignore it. ~MockObjectRegistry() { internal::MutexLock l(&internal::g_gmock_mutex); - if (!ReportMockObjectRegistryLeaks(*this)) { + std::string msg{}; + if (!ReportMockObjectRegistryLeaks(*this, msg)) { // RUN_ALL_TESTS() has already returned when this destructor is // called. Therefore we cannot use the normal Google Test // failure reporting mechanism. + std::cout << msg; + std::cout.flush(); + ::std::cerr.flush(); #ifdef GTEST_OS_QURT qurt_exception_raise_fatal(); #else _Exit(1); // We cannot call exit() as it is not reentrant and // may already have been called. #endif - } else { - return; } } @@ -516,12 +520,13 @@ class MockObjectRegistry { // Checks the given MockObjectRegistry for leaks (i.e. MockObjectStates objects // which are still in the given MockObjectRegistry and are not marked as -// leakable) and reports them. Furthermore it returns false if leaks were -// determined and true otherwise. -// NOTE: -// It is the callers job to make calls on "reg" safe, e.g. locking its mutex (if -// there is any). -bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg) { +// leakable) and returns a report in msg. Furthermore it returns false if leaks +// were determined and true otherwise. +// NOTE: It is the callers job to make calls +// on "reg" safe, e.g. locking its mutex (if there is any). +bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg, + std::string& msg) { + // In case user specified to ignore leaks via a cl flag return true. if (!GMOCK_FLAG_GET(catch_leaked_mocks)) return true; typedef std::map StateMap; @@ -534,31 +539,33 @@ bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg) { // FIXME: Print the type of the leaked object. // This can help the user identify the leaked object. - std::cout << "\n"; + msg += "\n"; const MockObjectState& state = it->second; - std::cout << internal::FormatFileLocation(state.first_used_file, - state.first_used_line); - std::cout << " ERROR: this mock object"; + msg += internal::FormatFileLocation(state.first_used_file, + state.first_used_line); + msg += " ERROR: this mock object"; if (!state.first_used_test.empty()) { - std::cout << " (used in test " << state.first_used_test_suite << "." - << state.first_used_test << ")"; + msg += " (used in test " + state.first_used_test_suite + "." + + state.first_used_test + ")"; } - std::cout << " should be deleted but never is. Its address is @" - << it->first << "."; + msg += " should be deleted but never is. Its address is @"; + std::ostringstream oss; + oss << it->first; + std::string address = oss.str(); + msg += address + "."; + leaked_count++; } if (leaked_count > 0) { - std::cout << "\nERROR: " << leaked_count << " leaked mock " - << (leaked_count == 1 ? "object" : "objects") - << " found at program exit. Expectations on a mock object are " - "verified when the object is destructed. Leaking a mock " - "means that its expectations aren't verified, which is " - "usually a test bug. If you really intend to leak a mock, " - "you can suppress this error using " - "testing::Mock::AllowLeak(mock_object), or you may use a " - "fake or stub instead of a mock.\n"; - std::cout.flush(); - ::std::cerr.flush(); + msg += "\nERROR: " + std::to_string(leaked_count) + " leaked mock " + + (leaked_count == 1 ? "object" : "objects") + + " found at program exit. Expectations on a mock object are " + "verified when the object is destructed. Leaking a mock " + "means that its expectations aren't verified, which is " + "usually a test bug. If you really intend to leak a mock, " + "you can suppress this error using " + "testing::Mock::AllowLeak(mock_object), or you may use a " + "fake or stub instead of a mock.\n"; return false; } @@ -638,11 +645,21 @@ void Mock::AllowLeak(const void* mock_obj) g_mock_object_registry.states()[mock_obj].leakable = true; } -bool Mock::CheckLeakInstant(void) +// Tells Google Mock to instantly check for leftover mock objects and report +// them if there are. +void Mock::CheckLeakInstant(void) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { internal::MutexLock l(&internal::g_gmock_mutex); - return ReportMockObjectRegistryLeaks(g_mock_object_registry); + std::string msg{}; + // Check for leftover mock objects at this point. + if (!ReportMockObjectRegistryLeaks(g_mock_object_registry, msg)) { + // If there are leftover (leaked) mock objects, fail the current test and + // report all leaks. + auto const& first_state = g_mock_object_registry.states().begin(); + testing::internal::Expect(false, first_state->second.first_used_file, + first_state->second.first_used_line, msg); + } } // Verifies and clears all expectations on the given mock object. If diff --git a/googlemock/test/gmock_leak_test.py b/googlemock/test/gmock_leak_test.py index b786f29e4..2fd98fc15 100755 --- a/googlemock/test/gmock_leak_test.py +++ b/googlemock/test/gmock_leak_test.py @@ -37,8 +37,8 @@ PROGRAM_PATH = gmock_test_utils.GetTestExecutablePath('gmock_leak_test_') TEST_WITH_EXPECT_CALL = [PROGRAM_PATH, '--gtest_filter=*ExpectCall*'] TEST_WITH_ON_CALL = [PROGRAM_PATH, '--gtest_filter=*OnCall*'] TEST_MULTIPLE_LEAKS = [PROGRAM_PATH, '--gtest_filter=*MultipleLeaked*'] -TEST_INSTANT_LEAKS = [PROGRAM_PATH, '--gtest_filter=*InstantLeak*'] -TEST_INSTANT_LEAKS_ENV = [PROGRAM_PATH, '--gtest_filter=*InstantAllowedByEnvironment*'] +TEST_INSTANT_LEAK = [PROGRAM_PATH, '--gtest_filter=*InstantLeak*'] +TEST_INSTANT_NO_LEAK = [PROGRAM_PATH, '--gtest_filter=*InstantNoLeak*'] environ = gmock_test_utils.environ SetEnvVar = gmock_test_utils.SetEnvVar @@ -111,18 +111,24 @@ class GMockLeakTest(gmock_test_utils.TestCase): ) def testInstantLeakCheck(self): + self.assertNotEqual( + 0, + gmock_test_utils.Subprocess( + TEST_INSTANT_LEAK + ['--gmock_catch_leaked_mocks=1'], env=environ + ).exit_code, + ) self.assertEqual( 0, gmock_test_utils.Subprocess( - TEST_INSTANT_LEAKS + ['--gmock_catch_leaked_mocks=1'], env=environ + TEST_INSTANT_LEAK + ['--gmock_catch_leaked_mocks=0'], env=environ ).exit_code, ) - def testInstantLeakCheckEnv(self): + def testInstantNoLeak(self): self.assertEqual( 0, gmock_test_utils.Subprocess( - TEST_INSTANT_LEAKS_ENV + ['--gmock_catch_leaked_mocks=0'], env=environ + TEST_INSTANT_NO_LEAK + ['--gmock_catch_leaked_mocks=1'], env=environ ).exit_code, ) diff --git a/googlemock/test/gmock_leak_test_.cc b/googlemock/test/gmock_leak_test_.cc index 6f6ff45e1..d7f633983 100644 --- a/googlemock/test/gmock_leak_test_.cc +++ b/googlemock/test/gmock_leak_test_.cc @@ -93,11 +93,6 @@ TEST(LeakTest, CatchesMultipleLeakedMockObjects) { // Makes sure Google Mock's leak detector can change the exit code // to 1 even when the code is already exiting with 0. - // Additionally the instant leak check should: - // a) return false since mock objects are leaked - // b) not influence the outcome of the on program end mock object leak check. - ASSERT_EQ(testing::Mock::CheckLeakInstant(), false); - exit(0); } @@ -109,8 +104,8 @@ TEST(LeakTest, InstantNoLeak) { delete foo; // Since foo is properly deleted instant leak check should not see a leaked - // mock object and therefore return true. - ASSERT_EQ(testing::Mock::CheckLeakInstant(), true); + // mock object and therefore not fail the test. + testing::Mock::CheckLeakInstant(); } TEST(LeakTest, InstantLeak) { @@ -120,14 +115,14 @@ TEST(LeakTest, InstantLeak) { foo->DoThis(); // At this point foo is still allocated. Calling the instant leak check should - // detect it and return false. - ASSERT_EQ(testing::Mock::CheckLeakInstant(), false); + // detect it and fail the test. + testing::Mock::CheckLeakInstant(); // Free foo in order to not fail the end of program leak check. delete foo; } -TEST(LeakTest, InstantLeakAllowed) { +TEST(LeakTest, InstantNoLeakAllowed) { MockFoo* foo = new MockFoo; testing::Mock::AllowLeak(foo); @@ -136,25 +131,11 @@ TEST(LeakTest, InstantLeakAllowed) { // At this point foo is still allocated However since we made foo a leakable // mock object with AllowLeak() the instant leak check should ignore it and - // return true. - ASSERT_EQ(testing::Mock::CheckLeakInstant(), true); + // pass the test. + testing::Mock::CheckLeakInstant(); // Free foo in order to not fail the end of program leak check. delete foo; } -TEST(LeakTest, InstantAllowedByEnvironment) { - MockFoo* foo = new MockFoo; - - EXPECT_CALL(*foo, DoThis()); - foo->DoThis(); - - // At this point foo is still allocated. However since we made foo a leakable - // via setting environment variable --gmock_catch_leaked_mocks=0 therefore - // true should be returned. - ASSERT_EQ(testing::Mock::CheckLeakInstant(), true); - - // Free foo in order to not fail the end of program leak check. - delete foo; -} } // namespace diff --git a/googlemock/test/gmock_output_test_.cc b/googlemock/test/gmock_output_test_.cc index 5a867429c..a58a55eb1 100644 --- a/googlemock/test/gmock_output_test_.cc +++ b/googlemock/test/gmock_output_test_.cc @@ -252,7 +252,7 @@ TEST_F(GMockOutputTest, CatchesLeakedMocks) { // Both foo1 and foo2 are deliberately leaked. // Call the instant leak check in order to validate it's output. - ASSERT_EQ(testing::Mock::CheckLeakInstant(),false); + testing::Mock::CheckLeakInstant(); } MATCHER_P2(IsPair, first, second, "") { diff --git a/googlemock/test/gmock_output_test_golden.txt b/googlemock/test/gmock_output_test_golden.txt index 7d3c95a3b..2c9c94090 100644 --- a/googlemock/test/gmock_output_test_golden.txt +++ b/googlemock/test/gmock_output_test_golden.txt @@ -304,12 +304,15 @@ FILE:#: Stack trace: [ OK ] GMockOutputTest.ExplicitActionsRunOutWithDefaultAction [ RUN ] GMockOutputTest.CatchesLeakedMocks +FILE:#: Failure FILE:#: ERROR: this mock object should be deleted but never is. Its address is @0x#. FILE:#: ERROR: this mock object should be deleted but never is. Its address is @0x#. FILE:#: ERROR: this mock object should be deleted but never is. Its address is @0x#. ERROR: 3 leaked mock objects found at program exit. Expectations on a mock object are verified when the object is destructed. Leaking a mock means that its expectations aren't verified, which is usually a test bug. If you really intend to leak a mock, you can suppress this error using testing::Mock::AllowLeak(mock_object), or you may use a fake or stub instead of a mock. -[ OK ] GMockOutputTest.CatchesLeakedMocks + + +[ FAILED ] GMockOutputTest.CatchesLeakedMocks [ RUN ] GMockOutputTest.PrintsMatcher FILE:#: Failure Value of: (std::pair(42, true)) @@ -331,6 +334,7 @@ Expected: is pair (first: is >= 48, second: true) [ FAILED ] GMockOutputTest.MismatchArgumentsAndWith [ FAILED ] GMockOutputTest.UnexpectedCallWithDefaultAction [ FAILED ] GMockOutputTest.ExcessiveCallWithDefaultAction +[ FAILED ] GMockOutputTest.CatchesLeakedMocks [ FAILED ] GMockOutputTest.PrintsMatcher