From d21346e6a1a9b2f91140073dec93e6a92c69e77a Mon Sep 17 00:00:00 2001 From: Alexander Nikforov Date: Mon, 23 May 2022 14:19:23 -0700 Subject: [PATCH 1/3] unique_ptr --- googlemock/src/gmock-internal-utils.cc | 3 +-- googletest/include/gtest/gtest.h | 3 ++- .../include/gtest/internal/gtest-internal.h | 3 +-- googletest/src/gtest.cc | 19 +++---------------- googletest/test/googletest-param-test-test.cc | 4 ++-- googletest/test/gtest-unittest-api_test.cc | 4 ++-- googletest/test/gtest_unittest.cc | 2 +- 7 files changed, 12 insertions(+), 26 deletions(-) diff --git a/googlemock/src/gmock-internal-utils.cc b/googlemock/src/gmock-internal-utils.cc index 0a74841f..9e2d9b8b 100644 --- a/googlemock/src/gmock-internal-utils.cc +++ b/googlemock/src/gmock-internal-utils.cc @@ -180,8 +180,7 @@ GTEST_API_ void Log(LogSeverity severity, const std::string& message, std::cout << "\n"; } std::cout << "Stack trace:\n" - << ::testing::internal::GetCurrentOsStackTraceExceptTop( - ::testing::UnitTest::GetInstance(), actual_to_skip); + << ::testing::internal::GetCurrentOsStackTraceExceptTop(actual_to_skip); } std::cout << ::std::flush; } diff --git a/googletest/include/gtest/gtest.h b/googletest/include/gtest/gtest.h index d19a587a..1926abb1 100644 --- a/googletest/include/gtest/gtest.h +++ b/googletest/include/gtest/gtest.h @@ -1092,10 +1092,11 @@ class GTEST_API_ TestEventListeners { // according to their specification. class GTEST_API_ UnitTest { public: + friend std::default_delete; // Gets the singleton UnitTest object. The first time this method // is called, a UnitTest object is constructed and returned. // Consecutive calls will return the same object. - static UnitTest* GetInstance(); + static std::unique_ptr& GetInstance(); // Runs all tests in this UnitTest object and prints the result. // Returns 0 if successful, or 1 otherwise. diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index 6d05f96c..2f6f6cf0 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -837,8 +837,7 @@ class TypeParameterizedTestSuite { // For example, if Foo() calls Bar(), which in turn calls // GetCurrentOsStackTraceExceptTop(..., 1), Foo() will be included in // the trace but Bar() and GetCurrentOsStackTraceExceptTop() won't. -GTEST_API_ std::string GetCurrentOsStackTraceExceptTop(UnitTest* unit_test, - int skip_count); +GTEST_API_ std::string GetCurrentOsStackTraceExceptTop(int skip_count); // Helpers for suppressing warnings on unreachable code or constant // condition. diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index 1acad59a..20a48568 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -5129,22 +5129,9 @@ void TestEventListeners::SuppressEventForwarding() { // Gets the singleton UnitTest object. The first time this method is // called, a UnitTest object is constructed and returned. Consecutive // calls will return the same object. -// -// We don't protect this under mutex_ as a user is not supposed to -// call this before main() starts, from which point on the return -// value will never change. -UnitTest* UnitTest::GetInstance() { - // CodeGear C++Builder insists on a public destructor for the - // default implementation. Use this implementation to keep good OO - // design with private destructor. - -#if defined(__BORLANDC__) - static UnitTest* const instance = new UnitTest; +std::unique_ptr& UnitTest::GetInstance() { + static std::unique_ptr instance{ new UnitTest }; return instance; -#else - static UnitTest instance; - return &instance; -#endif // defined(__BORLANDC__) } // Gets the number of successful test suites. @@ -6250,7 +6237,7 @@ void UnitTestImpl::UnshuffleTests() { // GetCurrentOsStackTraceExceptTop(..., 1), Foo() will be included in // the trace but Bar() and GetCurrentOsStackTraceExceptTop() won't. GTEST_NO_INLINE_ GTEST_NO_TAIL_CALL_ std::string -GetCurrentOsStackTraceExceptTop(UnitTest* /*unit_test*/, int skip_count) { +GetCurrentOsStackTraceExceptTop(int skip_count) { // We pass skip_count + 1 to skip this wrapper function in addition // to what the user really wants to skip. return GetUnitTestImpl()->CurrentOsStackTraceExceptTop(skip_count + 1); diff --git a/googletest/test/googletest-param-test-test.cc b/googletest/test/googletest-param-test-test.cc index 848ef975..01941913 100644 --- a/googletest/test/googletest-param-test-test.cc +++ b/googletest/test/googletest-param-test-test.cc @@ -821,7 +821,7 @@ TEST_F(PREFIX_WITH_MACRO(NamingTestNonParametrized), TEST(MacroNameing, LookupNames) { std::set know_suite_names, know_test_names; - auto ins = testing::UnitTest::GetInstance(); + auto ins = testing::UnitTest::GetInstance().get(); int ts = 0; while (const testing::TestSuite* suite = ins->GetTestSuite(ts++)) { know_suite_names.insert(suite->name()); @@ -897,7 +897,7 @@ INSTANTIATE_TEST_SUITE_P(CustomParamNameLambda, CustomLambdaNamingTest, }); TEST(CustomNamingTest, CheckNameRegistry) { - ::testing::UnitTest* unit_test = ::testing::UnitTest::GetInstance(); + ::testing::UnitTest* unit_test = ::testing::UnitTest::GetInstance().get(); std::set test_names; for (int suite_num = 0; suite_num < unit_test->total_test_suite_count(); ++suite_num) { diff --git a/googletest/test/gtest-unittest-api_test.cc b/googletest/test/gtest-unittest-api_test.cc index 2a13fa32..cc32f22b 100644 --- a/googletest/test/gtest-unittest-api_test.cc +++ b/googletest/test/gtest-unittest-api_test.cc @@ -106,7 +106,7 @@ const int kTypedTests = 1; // Since tests can be run in any order, the values the accessors that track // test execution (such as failed_test_count) can not be predicted. TEST(ApiTest, UnitTestImmutableAccessorsWork) { - UnitTest* unit_test = UnitTest::GetInstance(); + UnitTest* unit_test = UnitTest::GetInstance().get(); ASSERT_EQ(2 + kTypedTestSuites, unit_test->total_test_suite_count()); EXPECT_EQ(1 + kTypedTestSuites, unit_test->test_suite_to_run_count()); @@ -224,7 +224,7 @@ TEST(DISABLED_Test, Dummy2) {} class FinalSuccessChecker : public Environment { protected: void TearDown() override { - UnitTest* unit_test = UnitTest::GetInstance(); + UnitTest* unit_test = UnitTest::GetInstance().get(); EXPECT_EQ(1 + kTypedTestSuites, unit_test->successful_test_suite_count()); EXPECT_EQ(3 + kTypedTests, unit_test->successful_test_count()); diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index cdfdc6c4..d6e0a259 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -7703,7 +7703,7 @@ auto* dynamic_test = testing::RegisterTest( __LINE__, []() -> DynamicUnitTestFixture* { return new DynamicTest; }); TEST(RegisterTest, WasRegistered) { - auto* unittest = testing::UnitTest::GetInstance(); + auto* unittest = testing::UnitTest::GetInstance().get(); for (int i = 0; i < unittest->total_test_suite_count(); ++i) { auto* tests = unittest->GetTestSuite(i); if (tests->name() != std::string("DynamicUnitTestFixture")) continue; From 2c49f9cb0d6934535cf9480d47e39e13b700c048 Mon Sep 17 00:00:00 2001 From: Alexander Nikforov Date: Fri, 1 Jul 2022 13:36:51 -0700 Subject: [PATCH 2/3] clean up --- googletest/test/googletest-param-test-test.cc | 2 +- googletest/test/gtest-unittest-api_test.cc | 4 ++-- googletest/test/gtest_unittest.cc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/googletest/test/googletest-param-test-test.cc b/googletest/test/googletest-param-test-test.cc index 01941913..c9367d36 100644 --- a/googletest/test/googletest-param-test-test.cc +++ b/googletest/test/googletest-param-test-test.cc @@ -897,7 +897,7 @@ INSTANTIATE_TEST_SUITE_P(CustomParamNameLambda, CustomLambdaNamingTest, }); TEST(CustomNamingTest, CheckNameRegistry) { - ::testing::UnitTest* unit_test = ::testing::UnitTest::GetInstance().get(); + auto unit_test = ::testing::UnitTest::GetInstance().get(); std::set test_names; for (int suite_num = 0; suite_num < unit_test->total_test_suite_count(); ++suite_num) { diff --git a/googletest/test/gtest-unittest-api_test.cc b/googletest/test/gtest-unittest-api_test.cc index cc32f22b..1d7191c2 100644 --- a/googletest/test/gtest-unittest-api_test.cc +++ b/googletest/test/gtest-unittest-api_test.cc @@ -106,7 +106,7 @@ const int kTypedTests = 1; // Since tests can be run in any order, the values the accessors that track // test execution (such as failed_test_count) can not be predicted. TEST(ApiTest, UnitTestImmutableAccessorsWork) { - UnitTest* unit_test = UnitTest::GetInstance().get(); + auto unit_test = UnitTest::GetInstance().get(); ASSERT_EQ(2 + kTypedTestSuites, unit_test->total_test_suite_count()); EXPECT_EQ(1 + kTypedTestSuites, unit_test->test_suite_to_run_count()); @@ -224,7 +224,7 @@ TEST(DISABLED_Test, Dummy2) {} class FinalSuccessChecker : public Environment { protected: void TearDown() override { - UnitTest* unit_test = UnitTest::GetInstance().get(); + auto unit_test = UnitTest::GetInstance().get(); EXPECT_EQ(1 + kTypedTestSuites, unit_test->successful_test_suite_count()); EXPECT_EQ(3 + kTypedTests, unit_test->successful_test_count()); diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index d6e0a259..dc43c0b3 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -7703,7 +7703,7 @@ auto* dynamic_test = testing::RegisterTest( __LINE__, []() -> DynamicUnitTestFixture* { return new DynamicTest; }); TEST(RegisterTest, WasRegistered) { - auto* unittest = testing::UnitTest::GetInstance().get(); + auto unittest = testing::UnitTest::GetInstance().get(); for (int i = 0; i < unittest->total_test_suite_count(); ++i) { auto* tests = unittest->GetTestSuite(i); if (tests->name() != std::string("DynamicUnitTestFixture")) continue; From cd175a705418495a2fdfe12777c166e85908d904 Mon Sep 17 00:00:00 2001 From: Alexander Nikforov Date: Fri, 1 Jul 2022 13:58:35 -0700 Subject: [PATCH 3/3] make them const& so we can avoid changes in future --- googletest/test/googletest-param-test-test.cc | 4 ++-- googletest/test/gtest-unittest-api_test.cc | 4 ++-- googletest/test/gtest_unittest.cc | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/googletest/test/googletest-param-test-test.cc b/googletest/test/googletest-param-test-test.cc index c9367d36..e3090ae4 100644 --- a/googletest/test/googletest-param-test-test.cc +++ b/googletest/test/googletest-param-test-test.cc @@ -821,7 +821,7 @@ TEST_F(PREFIX_WITH_MACRO(NamingTestNonParametrized), TEST(MacroNameing, LookupNames) { std::set know_suite_names, know_test_names; - auto ins = testing::UnitTest::GetInstance().get(); + const auto& ins = testing::UnitTest::GetInstance(); int ts = 0; while (const testing::TestSuite* suite = ins->GetTestSuite(ts++)) { know_suite_names.insert(suite->name()); @@ -897,7 +897,7 @@ INSTANTIATE_TEST_SUITE_P(CustomParamNameLambda, CustomLambdaNamingTest, }); TEST(CustomNamingTest, CheckNameRegistry) { - auto unit_test = ::testing::UnitTest::GetInstance().get(); + const auto& unit_test = ::testing::UnitTest::GetInstance(); std::set test_names; for (int suite_num = 0; suite_num < unit_test->total_test_suite_count(); ++suite_num) { diff --git a/googletest/test/gtest-unittest-api_test.cc b/googletest/test/gtest-unittest-api_test.cc index 1d7191c2..2ea69273 100644 --- a/googletest/test/gtest-unittest-api_test.cc +++ b/googletest/test/gtest-unittest-api_test.cc @@ -106,7 +106,7 @@ const int kTypedTests = 1; // Since tests can be run in any order, the values the accessors that track // test execution (such as failed_test_count) can not be predicted. TEST(ApiTest, UnitTestImmutableAccessorsWork) { - auto unit_test = UnitTest::GetInstance().get(); + const auto& unit_test = UnitTest::GetInstance(); ASSERT_EQ(2 + kTypedTestSuites, unit_test->total_test_suite_count()); EXPECT_EQ(1 + kTypedTestSuites, unit_test->test_suite_to_run_count()); @@ -224,7 +224,7 @@ TEST(DISABLED_Test, Dummy2) {} class FinalSuccessChecker : public Environment { protected: void TearDown() override { - auto unit_test = UnitTest::GetInstance().get(); + const auto& unit_test = UnitTest::GetInstance(); EXPECT_EQ(1 + kTypedTestSuites, unit_test->successful_test_suite_count()); EXPECT_EQ(3 + kTypedTests, unit_test->successful_test_count()); diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index dc43c0b3..b579e81e 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -7703,7 +7703,7 @@ auto* dynamic_test = testing::RegisterTest( __LINE__, []() -> DynamicUnitTestFixture* { return new DynamicTest; }); TEST(RegisterTest, WasRegistered) { - auto unittest = testing::UnitTest::GetInstance().get(); + const auto& unittest = testing::UnitTest::GetInstance(); for (int i = 0; i < unittest->total_test_suite_count(); ++i) { auto* tests = unittest->GetTestSuite(i); if (tests->name() != std::string("DynamicUnitTestFixture")) continue;