From a13a0626188b4e7d22d63a4c9fcfe9f441c81e4a Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 19 Dec 2019 12:36:47 -0500 Subject: Googletest export Add option (default to disabled) to make C++ type parameterized tests (TYPED_TEST_P) fail when they're not instantiated. When an un-instantiated TYPED_TEST_P is found, a new test will be inserted that emits a suitable message. For now, that is just a notice, but the hope it to flip the bit to make it fail by default. PiperOrigin-RevId: 286408038 --- googletest/include/gtest/gtest-typed-test.h | 2 +- googletest/include/gtest/internal/gtest-internal.h | 11 +++- .../include/gtest/internal/gtest-param-util.h | 28 ++++++++++ googletest/src/gtest-internal-inl.h | 9 ++++ googletest/src/gtest-typed-test.cc | 5 +- googletest/src/gtest.cc | 59 ++++++++++++++++++++++ .../test/googletest-output-test-golden-lin.txt | 13 +++-- googletest/test/googletest-output-test_.cc | 15 ++++++ googletest/test/googletest-param-test-test.cc | 5 ++ googletest/test/gtest-typed-test_test.cc | 12 ++--- 10 files changed, 145 insertions(+), 14 deletions(-) diff --git a/googletest/include/gtest/gtest-typed-test.h b/googletest/include/gtest/gtest-typed-test.h index 6f635c8..3ffa50b 100644 --- a/googletest/include/gtest/gtest-typed-test.h +++ b/googletest/include/gtest/gtest-typed-test.h @@ -297,7 +297,7 @@ INSTANTIATE_TYPED_TEST_SUITE_P(My, FooTest, MyTypes); static const char* const GTEST_REGISTERED_TEST_NAMES_( \ SuiteName) GTEST_ATTRIBUTE_UNUSED_ = \ GTEST_TYPED_TEST_SUITE_P_STATE_(SuiteName).VerifyRegisteredTestNames( \ - __FILE__, __LINE__, #__VA_ARGS__) + GTEST_STRINGIFY_(SuiteName), __FILE__, __LINE__, #__VA_ARGS__) // Legacy API is deprecated but still available #ifndef GTEST_REMOVE_LEGACY_TEST_CASEAPI_ diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index 59d2a83..57ebbda 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -617,8 +617,9 @@ class GTEST_API_ TypedTestSuitePState { // Verifies that registered_tests match the test names in // defined_test_names_; returns registered_tests if successful, or // aborts the program otherwise. - const char* VerifyRegisteredTestNames( - const char* file, int line, const char* registered_tests); + const char* VerifyRegisteredTestNames(const char* test_suite_name, + const char* file, int line, + const char* registered_tests); private: typedef ::std::map RegisteredTestsMap; @@ -750,6 +751,11 @@ class TypeParameterizedTest { } }; +GTEST_API_ void RegisterTypeParameterizedTestSuite(const char* test_suite_name, + CodeLocation code_location); +GTEST_API_ void RegisterTypeParameterizedTestSuiteInstantiation( + const char* case_name); + // TypeParameterizedTestSuite::Register() // registers *all combinations* of 'Tests' and 'Types' with Google // Test. The return value is insignificant - we just need to return @@ -762,6 +768,7 @@ class TypeParameterizedTestSuite { const char* test_names, const std::vector& type_names = GenerateNames()) { + RegisterTypeParameterizedTestSuiteInstantiation(case_name); std::string test_name = StripTrailingSpaces( GetPrefixUntilComma(test_names)); if (!state->TestExists(test_name)) { diff --git a/googletest/include/gtest/internal/gtest-param-util.h b/googletest/include/gtest/internal/gtest-param-util.h index 3f2495b..ffa7d72 100644 --- a/googletest/include/gtest/internal/gtest-param-util.h +++ b/googletest/include/gtest/internal/gtest-param-util.h @@ -731,6 +731,34 @@ class ParameterizedTestSuiteRegistry { GTEST_DISALLOW_COPY_AND_ASSIGN_(ParameterizedTestSuiteRegistry); }; +// Keep track of what type-parameterized test suite are defined and +// where as well as which are intatiated. This allows susequently +// identifying suits that are defined but never used. +class TypeParameterizedTestSuiteRegistry { + public: + // Add a suite definition + void RegisterTestSuite(const char* test_suite_name, + CodeLocation code_location); + + // Add an instantiation of a suit. + void RegisterInstantiation(const char* test_suite_name); + + // For each suit repored as defined but not reported as instantiation, + // emit a test that reports that fact (configurably, as an error). + void CheckForInstantiations(); + + private: + struct TypeParameterizedTestSuiteInfo { + explicit TypeParameterizedTestSuiteInfo(CodeLocation c) + : code_location(c), instantiated(false) {} + + CodeLocation code_location; + bool instantiated; + }; + + std::map suites_; +}; + } // namespace internal // Forward declarations of ValuesIn(), which is implemented in diff --git a/googletest/src/gtest-internal-inl.h b/googletest/src/gtest-internal-inl.h index c3575ee..d0ebe0c 100644 --- a/googletest/src/gtest-internal-inl.h +++ b/googletest/src/gtest-internal-inl.h @@ -698,6 +698,13 @@ class GTEST_API_ UnitTestImpl { return parameterized_test_registry_; } + // Returns TypeParameterizedTestSuiteRegistry object used to keep track of + // type-parameterized tests and instantiations of them. + internal::TypeParameterizedTestSuiteRegistry& + type_parameterized_test_registry() { + return type_parameterized_test_registry_; + } + // Sets the TestSuite object for the test that's currently running. void set_current_test_suite(TestSuite* a_current_test_suite) { current_test_suite_ = a_current_test_suite; @@ -874,6 +881,8 @@ class GTEST_API_ UnitTestImpl { // ParameterizedTestRegistry object used to register value-parameterized // tests. internal::ParameterizedTestSuiteRegistry parameterized_test_registry_; + internal::TypeParameterizedTestSuiteRegistry + type_parameterized_test_registry_; // Indicates whether RegisterParameterizedTests() has been called already. bool parameterized_tests_registered_; diff --git a/googletest/src/gtest-typed-test.cc b/googletest/src/gtest-typed-test.cc index 8677caf..1b1cfb0 100644 --- a/googletest/src/gtest-typed-test.cc +++ b/googletest/src/gtest-typed-test.cc @@ -58,7 +58,10 @@ static std::vector SplitIntoTestNames(const char* src) { // registered_tests_; returns registered_tests if successful, or // aborts the program otherwise. const char* TypedTestSuitePState::VerifyRegisteredTestNames( - const char* file, int line, const char* registered_tests) { + const char* test_suite_name, const char* file, int line, + const char* registered_tests) { + RegisterTypeParameterizedTestSuite(test_suite_name, CodeLocation(file, line)); + typedef RegisteredTestsMap::const_iterator RegisteredTestIter; registered_ = true; diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index f6466b9..07015cb 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -415,6 +415,7 @@ namespace { // // This configuration bit will likely be removed at some point. constexpr bool kErrorOnUninstantiatedParameterizedTest = false; +constexpr bool kErrorOnUninstantiatedTypeParameterizedTest = false; // A test that fails at a given file/line location with a given message. class FailureTest : public Test { @@ -467,6 +468,63 @@ void InsertSyntheticTestCase(const std::string &name, CodeLocation location) { }); } +void RegisterTypeParameterizedTestSuite(const char* test_suite_name, + CodeLocation code_location) { + GetUnitTestImpl()->type_parameterized_test_registry().RegisterTestSuite( + test_suite_name, code_location); +} + +void RegisterTypeParameterizedTestSuiteInstantiation(const char* case_name) { + GetUnitTestImpl() + ->type_parameterized_test_registry() + .RegisterInstantiation(case_name); +} + +void TypeParameterizedTestSuiteRegistry::RegisterTestSuite( + const char* test_suite_name, CodeLocation code_location) { + suites_.emplace(std::string(test_suite_name), + TypeParameterizedTestSuiteInfo(code_location)); +} + +void TypeParameterizedTestSuiteRegistry::RegisterInstantiation( + const char* test_suite_name) { + auto it = suites_.find(std::string(test_suite_name)); + if (it != suites_.end()) { + it->second.instantiated = true; + } else { + GTEST_LOG_(ERROR) << "Unknown type parameterized test suit '" + << test_suite_name << "'"; + } +} + +void TypeParameterizedTestSuiteRegistry::CheckForInstantiations() { + for (const auto& testcase : suites_) { + if (testcase.second.instantiated) continue; + + std::string message = + "Type paramaterized test suite " + testcase.first + + " is defined via REGISTER_TYPED_TEST_SUITE_P, but never instantiated " + "via INSTANTIATE_TYPED_TEST_SUITE_P. None of the test cases will run." + "\n\n" + "Ideally, TYPED_TEST_P definitions should only ever be included as " + "part of binaries that intend to use them. (As opposed to, for " + "example, being placed in a library that may be linked in to get other " + "utilities.)"; + + std::string full_name = + "UninstantiatedTypeParamaterizedTestSuite<" + testcase.first + ">"; + RegisterTest( // + "GoogleTestVerification", full_name.c_str(), + nullptr, // No type parameter. + nullptr, // No value parameter. + testcase.second.code_location.file.c_str(), + testcase.second.code_location.line, [message, testcase] { + return new FailureTest(testcase.second.code_location, message, + kErrorOnUninstantiatedTypeParameterizedTest); + }); + } +} + // A copy of all command line arguments. Set by InitGoogleTest(). static ::std::vector g_argvs; @@ -2710,6 +2768,7 @@ namespace internal { void UnitTestImpl::RegisterParameterizedTests() { if (!parameterized_tests_registered_) { parameterized_test_registry_.RegisterTests(); + type_parameterized_test_registry_.CheckForInstantiations(); parameterized_tests_registered_ = true; } } diff --git a/googletest/test/googletest-output-test-golden-lin.txt b/googletest/test/googletest-output-test-golden-lin.txt index a08140e..c1db004 100644 --- a/googletest/test/googletest-output-test-golden-lin.txt +++ b/googletest/test/googletest-output-test-golden-lin.txt @@ -12,7 +12,7 @@ Expected equality of these values: 3 Stack trace: (omitted) -[==========] Running 86 tests from 41 test suites. +[==========] Running 87 tests from 41 test suites. [----------] Global test environment set-up. FooEnvironment::SetUp() called. BarEnvironment::SetUp() called. @@ -982,12 +982,17 @@ Expected failure Stack trace: (omitted) [ FAILED ] PrintingStrings/ParamTest.Failure/a, where GetParam() = "a" -[----------] 1 test from GoogleTestVerification +[----------] 2 tests from GoogleTestVerification [ RUN ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite Paramaterized test suite DetectNotInstantiatedTest is defined via TEST_P, but never instantiated. None of the test cases will run. Either no INSTANTIATE_TEST_SUITE_P is provided or the only ones provided expand to nothing. Ideally, TEST_P definitions should only ever be included as part of binaries that intend to use them. (As opposed to, for example, being placed in a library that may be linked in to get other utilities.) [ OK ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite +[ RUN ] GoogleTestVerification.UninstantiatedTypeParamaterizedTestSuite +Type paramaterized test suite DetectNotInstantiatedTypesTest is defined via REGISTER_TYPED_TEST_SUITE_P, but never instantiated via INSTANTIATE_TYPED_TEST_SUITE_P. None of the test cases will run. + +Ideally, TYPED_TEST_P definitions should only ever be included as part of binaries that intend to use them. (As opposed to, for example, being placed in a library that may be linked in to get other utilities.) +[ OK ] GoogleTestVerification.UninstantiatedTypeParamaterizedTestSuite [----------] Global test environment tear-down BarEnvironment::TearDown() called. googletest-output-test_.cc:#: Failure @@ -1001,8 +1006,8 @@ Failed Expected fatal failure. Stack trace: (omitted) -[==========] 86 tests from 41 test suites ran. -[ PASSED ] 32 tests. +[==========] 87 tests from 41 test suites ran. +[ PASSED ] 33 tests. [ FAILED ] 54 tests, listed below: [ FAILED ] NonfatalFailureTest.EscapesStringOperands [ FAILED ] NonfatalFailureTest.DiffForLongStrings diff --git a/googletest/test/googletest-output-test_.cc b/googletest/test/googletest-output-test_.cc index 1c7943c..b1d66f9 100644 --- a/googletest/test/googletest-output-test_.cc +++ b/googletest/test/googletest-output-test_.cc @@ -876,6 +876,21 @@ class TypedTestPNames { INSTANTIATE_TYPED_TEST_SUITE_P(UnsignedCustomName, TypedTestP, UnsignedTypes, TypedTestPNames); +template +class DetectNotInstantiatedTypesTest : public testing::Test {}; +TYPED_TEST_SUITE_P(DetectNotInstantiatedTypesTest); +TYPED_TEST_P(DetectNotInstantiatedTypesTest, Used) { + TypeParam instantiate; + (void)instantiate; +} +REGISTER_TYPED_TEST_SUITE_P(DetectNotInstantiatedTypesTest, Used); + +// kErrorOnUninstantiatedTypeParameterizedTest=true would make the above fail. +// Adding the following would make that test failure go away. +// +// typedef ::testing::Types MyTypes; +// INSTANTIATE_TYPED_TEST_SUITE_P(All, DetectNotInstantiatedTypesTest, MyTypes); + #endif // GTEST_HAS_TYPED_TEST_P #if GTEST_HAS_DEATH_TEST diff --git a/googletest/test/googletest-param-test-test.cc b/googletest/test/googletest-param-test-test.cc index 85c76c3..f92eb31 100644 --- a/googletest/test/googletest-param-test-test.cc +++ b/googletest/test/googletest-param-test-test.cc @@ -1072,6 +1072,11 @@ namespace works_here { // Never used not instantiated, this should work. class NotUsedTest : public testing::TestWithParam {}; +/////// +// Never used not instantiated, this should work. +template +class NotUsedTypeTest : public testing::Test {}; +TYPED_TEST_SUITE_P(NotUsedTypeTest); } // namespace works_here int main(int argc, char **argv) { diff --git a/googletest/test/gtest-typed-test_test.cc b/googletest/test/gtest-typed-test_test.cc index 5411832..0c1f660 100644 --- a/googletest/test/gtest-typed-test_test.cc +++ b/googletest/test/gtest-typed-test_test.cc @@ -228,7 +228,7 @@ class TypedTestSuitePStateTest : public Test { TEST_F(TypedTestSuitePStateTest, SucceedsForMatchingList) { const char* tests = "A, B, C"; EXPECT_EQ(tests, - state_.VerifyRegisteredTestNames("foo.cc", 1, tests)); + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, tests)); } // Makes sure that the order of the tests and spaces around the names @@ -236,33 +236,33 @@ TEST_F(TypedTestSuitePStateTest, SucceedsForMatchingList) { TEST_F(TypedTestSuitePStateTest, IgnoresOrderAndSpaces) { const char* tests = "A,C, B"; EXPECT_EQ(tests, - state_.VerifyRegisteredTestNames("foo.cc", 1, tests)); + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, tests)); } using TypedTestSuitePStateDeathTest = TypedTestSuitePStateTest; TEST_F(TypedTestSuitePStateDeathTest, DetectsDuplicates) { EXPECT_DEATH_IF_SUPPORTED( - state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, A, C"), + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, A, C"), "foo\\.cc.1.?: Test A is listed more than once\\."); } TEST_F(TypedTestSuitePStateDeathTest, DetectsExtraTest) { EXPECT_DEATH_IF_SUPPORTED( - state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, C, D"), + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, C, D"), "foo\\.cc.1.?: No test named D can be found in this test suite\\."); } TEST_F(TypedTestSuitePStateDeathTest, DetectsMissedTest) { EXPECT_DEATH_IF_SUPPORTED( - state_.VerifyRegisteredTestNames("foo.cc", 1, "A, C"), + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, C"), "foo\\.cc.1.?: You forgot to list test B\\."); } // Tests that defining a test for a parameterized test case generates // a run-time error if the test case has been registered. TEST_F(TypedTestSuitePStateDeathTest, DetectsTestAfterRegistration) { - state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, C"); + state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, C"); EXPECT_DEATH_IF_SUPPORTED( state_.AddTestName("foo.cc", 2, "FooTest", "D"), "foo\\.cc.2.?: Test D must be defined before REGISTER_TYPED_TEST_SUITE_P" -- cgit v0.12