Make instant leak checks fail the current running test (#4215)

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
 

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 void CheckLeakInstant(void)
-  static bool CheckLeakInstant(void)
       GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex);
 
   // Verifies and clears all expectations on the given mock object.

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<internal::UntypedFunctionMockerBase*> FunctionMockers;
 
@@ -487,23 +489,25 @@ class MockObjectRegistry {
   typedef std::map<const void*, MockObjectState> 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
+  // tests in it have been run. By then, there should be no mock
-  // object alive.  Therefore we report any living object as test
+  // 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
+// leakable) and returns a report in msg. Furthermore it returns false if leaks
-// determined and true otherwise.
+// were determined and true otherwise.
-// NOTE:
+// NOTE: It is the callers job to make calls
-// It is the callers job to make calls on "reg" safe, e.g. locking its mutex (if
+// on "reg" safe, e.g. locking its mutex (if there is any).
-// there is any).
+bool ReportMockObjectRegistryLeaks(MockObjectRegistry const& reg,
-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<const void*, MockObjectState> 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,
+    msg += internal::FormatFileLocation(state.first_used_file,
-                                              state.first_used_line);
+                                        state.first_used_line);
-    std::cout << " ERROR: this mock object";
+    msg += " ERROR: this mock object";
     if (!state.first_used_test.empty()) {
-      std::cout << " (used in test " << state.first_used_test_suite << "."
+      msg += " (used in test " + state.first_used_test_suite + "." +
-                << state.first_used_test << ")";
+             state.first_used_test + ")";
     }
-    std::cout << " should be deleted but never is. Its address is @"
+    msg += " should be deleted but never is. Its address is @";
-              << it->first << ".";
+    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 "
+    msg += "\nERROR: " + std::to_string(leaked_count) + " leaked mock " +
-              << (leaked_count == 1 ? "object" : "objects")
+           (leaked_count == 1 ? "object" : "objects") +
-              << " found at program exit. Expectations on a mock object are "
+           " found at program exit. Expectations on a mock object are "
-                 "verified when the object is destructed. Leaking a mock "
+           "verified when the object is destructed. Leaking a mock "
-                 "means that its expectations aren't verified, which is "
+           "means that its expectations aren't verified, which is "
-                 "usually a test bug. If you really intend to leak a mock, "
+           "usually a test bug. If you really intend to leak a mock, "
-                 "you can suppress this error using "
+           "you can suppress this error using "
-                 "testing::Mock::AllowLeak(mock_object), or you may use a "
+           "testing::Mock::AllowLeak(mock_object), or you may use a "
-                 "fake or stub instead of a mock.\n";
+           "fake or stub instead of a mock.\n";
-    std::cout.flush();
-    ::std::cerr.flush();
 
     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

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_LEAK = [PROGRAM_PATH, '--gtest_filter=*InstantLeak*']
-TEST_INSTANT_LEAKS_ENV = [PROGRAM_PATH, '--gtest_filter=*InstantAllowedByEnvironment*']
+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,
     )
 

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.
+  // mock object and therefore not fail the test.
-  ASSERT_EQ(testing::Mock::CheckLeakInstant(), true);
+  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.
+  // detect it and fail the test.
-  ASSERT_EQ(testing::Mock::CheckLeakInstant(), false);
+  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.
+  // pass the test.
-  ASSERT_EQ(testing::Mock::CheckLeakInstant(), true);
+  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

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, "") {

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<int, bool>(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