From 8764c62eed3fd7fa271a4c8dfe663abc48bde052 Mon Sep 17 00:00:00 2001 From: Matthias Donaubauer Date: Mon, 22 May 2023 17:35:35 +0200 Subject: [PATCH] Add instant leak check capabilities to gmock (#4215) --- docs/gmock_cheat_sheet.md | 11 ++ .../include/gmock/gmock-spec-builders.h | 6 ++ googlemock/src/gmock-spec-builders.cc | 102 +++++++++++------- googlemock/test/gmock_leak_test.py | 18 ++++ googlemock/test/gmock_leak_test_.cc | 61 +++++++++++ googlemock/test/gmock_output_test.py | 6 +- googlemock/test/gmock_output_test_.cc | 2 + googlemock/test/gmock_output_test_golden.txt | 5 + 8 files changed, 174 insertions(+), 37 deletions(-) diff --git a/docs/gmock_cheat_sheet.md b/docs/gmock_cheat_sheet.md index ddafaaa22..7b863e9ee 100644 --- a/docs/gmock_cheat_sheet.md +++ b/docs/gmock_cheat_sheet.md @@ -219,6 +219,17 @@ verified: Mock::AllowLeak(&mock_obj); ``` +Furthermore you can perform instant leak checks. This may help tracing down leaking +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 gMock defines a convenient mock class template diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h index c4c42b7c5..bf5f970b2 100644 --- a/googlemock/include/gmock/gmock-spec-builders.h +++ b/googlemock/include/gmock/gmock-spec-builders.h @@ -370,6 +370,12 @@ class GTEST_API_ Mock { static void AllowLeak(const void* mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex); + // 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) + GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex); + // Verifies and clears all expectations on the given mock object. // If the expectations aren't satisfied, generates one or more // Google Test non-fatal failures and returns false. diff --git a/googlemock/src/gmock-spec-builders.cc b/googlemock/src/gmock-spec-builders.cc index ffdf03dd4..8496a69eb 100644 --- a/googlemock/src/gmock-spec-builders.cc +++ b/googlemock/src/gmock-spec-builders.cc @@ -453,9 +453,11 @@ static CallReaction intToCallReaction(int mock_behavior) { } // namespace internal // Class Mock. - namespace { +class MockObjectRegistry; +bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg); + typedef std::set FunctionMockers; // The current state of a mock object. Such information is needed for @@ -489,42 +491,8 @@ class MockObjectRegistry { // object alive. Therefore we report any living object as test // failure, unless the user explicitly asked us to ignore it. ~MockObjectRegistry() { - if (!GMOCK_FLAG_GET(catch_leaked_mocks)) return; internal::MutexLock l(&internal::g_gmock_mutex); - - int leaked_count = 0; - for (StateMap::const_iterator it = states_.begin(); it != states_.end(); - ++it) { - if (it->second.leakable) // The user said it's fine to leak this object. - continue; - - // FIXME: Print the type of the leaked object. - // This can help the user identify the leaked object. - std::cout << "\n"; - const MockObjectState& state = it->second; - std::cout << internal::FormatFileLocation(state.first_used_file, - state.first_used_line); - std::cout << " ERROR: this mock object"; - if (!state.first_used_test.empty()) { - std::cout << " (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 << "."; - 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(); + if (!ReportMockObjectRegistryLeaks(*this)) { // RUN_ALL_TESTS() has already returned when this destructor is // called. Therefore we cannot use the normal Google Test // failure reporting mechanism. @@ -534,15 +502,70 @@ class MockObjectRegistry { _Exit(1); // We cannot call exit() as it is not reentrant and // may already have been called. #endif + } else { + return; } } + StateMap const& states() const { return states_; } StateMap& states() { return states_; } private: StateMap states_; }; +// 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) { + if (!GMOCK_FLAG_GET(catch_leaked_mocks)) return true; + + typedef std::map StateMap; + + int leaked_count = 0; + for (StateMap::const_iterator it = reg.states().begin(); + it != reg.states().end(); ++it) { + if (it->second.leakable) // The user said it's fine to leak this object. + continue; + + // FIXME: Print the type of the leaked object. + // This can help the user identify the leaked object. + std::cout << "\n"; + const MockObjectState& state = it->second; + std::cout << internal::FormatFileLocation(state.first_used_file, + state.first_used_line); + std::cout << " ERROR: this mock object"; + if (!state.first_used_test.empty()) { + std::cout << " (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 << "."; + 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(); + + return false; + } + + return true; +} + // Protected by g_gmock_mutex. MockObjectRegistry g_mock_object_registry; @@ -615,6 +638,13 @@ void Mock::AllowLeak(const void* mock_obj) g_mock_object_registry.states()[mock_obj].leakable = true; } +bool Mock::CheckLeakInstant(void) + GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { + internal::MutexLock l(&internal::g_gmock_mutex); + + return ReportMockObjectRegistryLeaks(g_mock_object_registry); +} + // Verifies and clears all expectations on the given mock object. If // the expectations aren't satisfied, generates one or more Google // Test non-fatal failures and returns false. diff --git a/googlemock/test/gmock_leak_test.py b/googlemock/test/gmock_leak_test.py index 8b02bc465..b786f29e4 100755 --- a/googlemock/test/gmock_leak_test.py +++ b/googlemock/test/gmock_leak_test.py @@ -37,6 +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*'] environ = gmock_test_utils.environ SetEnvVar = gmock_test_utils.SetEnvVar @@ -108,6 +110,22 @@ class GMockLeakTest(gmock_test_utils.TestCase): ).exit_code, ) + def testInstantLeakCheck(self): + self.assertEqual( + 0, + gmock_test_utils.Subprocess( + TEST_INSTANT_LEAKS + ['--gmock_catch_leaked_mocks=1'], env=environ + ).exit_code, + ) + + def testInstantLeakCheckEnv(self): + self.assertEqual( + 0, + gmock_test_utils.Subprocess( + TEST_INSTANT_LEAKS_ENV + ['--gmock_catch_leaked_mocks=0'], env=environ + ).exit_code, + ) + if __name__ == '__main__': gmock_test_utils.Main() diff --git a/googlemock/test/gmock_leak_test_.cc b/googlemock/test/gmock_leak_test_.cc index a6bb33921..6f6ff45e1 100644 --- a/googlemock/test/gmock_leak_test_.cc +++ b/googlemock/test/gmock_leak_test_.cc @@ -93,7 +93,68 @@ 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); } +TEST(LeakTest, InstantNoLeak) { + MockFoo* foo = new MockFoo; + + EXPECT_CALL(*foo, DoThis()); + foo->DoThis(); + + 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); +} + +TEST(LeakTest, InstantLeak) { + MockFoo* foo = new MockFoo; + + EXPECT_CALL(*foo, DoThis()); + 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); + + // Free foo in order to not fail the end of program leak check. + delete foo; +} + +TEST(LeakTest, InstantLeakAllowed) { + MockFoo* foo = new MockFoo; + testing::Mock::AllowLeak(foo); + + EXPECT_CALL(*foo, DoThis()); + foo->DoThis(); + + // 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); + + // 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.py b/googlemock/test/gmock_output_test.py index 7c24d6832..d26bc231e 100755 --- a/googlemock/test/gmock_output_test.py +++ b/googlemock/test/gmock_output_test.py @@ -166,12 +166,16 @@ class GMockOutputTest(gmock_test_utils.TestCase): # The normalized output should match the golden file. self.assertEqual(golden, output) - # The raw output should contain 2 leaked mock object errors for + # The raw output should contain 4 leaked mock object errors for # test GMockOutputTest.CatchesLeakedMocks. + # Two from the at end of program check and two from the instant leak check + # within GMockOutputTest.CatchesLeakedMocks. self.assertEqual( [ 'GMockOutputTest.CatchesLeakedMocks', 'GMockOutputTest.CatchesLeakedMocks', + 'GMockOutputTest.CatchesLeakedMocks', + 'GMockOutputTest.CatchesLeakedMocks', ], leaky_tests, ) diff --git a/googlemock/test/gmock_output_test_.cc b/googlemock/test/gmock_output_test_.cc index 03d842139..5a867429c 100644 --- a/googlemock/test/gmock_output_test_.cc +++ b/googlemock/test/gmock_output_test_.cc @@ -251,6 +251,8 @@ TEST_F(GMockOutputTest, CatchesLeakedMocks) { foo2->Bar2(1, 1); // 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); } MATCHER_P2(IsPair, first, second, "") { diff --git a/googlemock/test/gmock_output_test_golden.txt b/googlemock/test/gmock_output_test_golden.txt index ca88af029..7d3c95a3b 100644 --- a/googlemock/test/gmock_output_test_golden.txt +++ b/googlemock/test/gmock_output_test_golden.txt @@ -304,6 +304,11 @@ FILE:#: Stack trace: [ OK ] GMockOutputTest.ExplicitActionsRunOutWithDefaultAction [ RUN ] GMockOutputTest.CatchesLeakedMocks + +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 [ RUN ] GMockOutputTest.PrintsMatcher FILE:#: Failure