Skip to content

Commit a13a062

Browse files
Abseil TeamAndy Soffer
Abseil Team
authored and
Andy Soffer
committed
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
1 parent 008629a commit a13a062

10 files changed

+145
-14
lines changed

‎googletest/include/gtest/gtest-typed-test.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ INSTANTIATE_TYPED_TEST_SUITE_P(My, FooTest, MyTypes);
297297
static const char* const GTEST_REGISTERED_TEST_NAMES_( \
298298
SuiteName) GTEST_ATTRIBUTE_UNUSED_ = \
299299
GTEST_TYPED_TEST_SUITE_P_STATE_(SuiteName).VerifyRegisteredTestNames( \
300-
__FILE__, __LINE__, #__VA_ARGS__)
300+
GTEST_STRINGIFY_(SuiteName), __FILE__, __LINE__, #__VA_ARGS__)
301301

302302
// Legacy API is deprecated but still available
303303
#ifndef GTEST_REMOVE_LEGACY_TEST_CASEAPI_

‎googletest/include/gtest/internal/gtest-internal.h

+9-2
Original file line numberDiff line numberDiff line change
@@ -617,8 +617,9 @@ class GTEST_API_ TypedTestSuitePState {
617617
// Verifies that registered_tests match the test names in
618618
// defined_test_names_; returns registered_tests if successful, or
619619
// aborts the program otherwise.
620-
const char* VerifyRegisteredTestNames(
621-
const char* file, int line, const char* registered_tests);
620+
const char* VerifyRegisteredTestNames(const char* test_suite_name,
621+
const char* file, int line,
622+
const char* registered_tests);
622623

623624
private:
624625
typedef ::std::map<std::string, CodeLocation> RegisteredTestsMap;
@@ -750,6 +751,11 @@ class TypeParameterizedTest<Fixture, TestSel, internal::None> {
750751
}
751752
};
752753

754+
GTEST_API_ void RegisterTypeParameterizedTestSuite(const char* test_suite_name,
755+
CodeLocation code_location);
756+
GTEST_API_ void RegisterTypeParameterizedTestSuiteInstantiation(
757+
const char* case_name);
758+
753759
// TypeParameterizedTestSuite<Fixture, Tests, Types>::Register()
754760
// registers *all combinations* of 'Tests' and 'Types' with Google
755761
// Test. The return value is insignificant - we just need to return
@@ -762,6 +768,7 @@ class TypeParameterizedTestSuite {
762768
const char* test_names,
763769
const std::vector<std::string>& type_names =
764770
GenerateNames<DefaultNameGenerator, Types>()) {
771+
RegisterTypeParameterizedTestSuiteInstantiation(case_name);
765772
std::string test_name = StripTrailingSpaces(
766773
GetPrefixUntilComma(test_names));
767774
if (!state->TestExists(test_name)) {

‎googletest/include/gtest/internal/gtest-param-util.h

+28
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,34 @@ class ParameterizedTestSuiteRegistry {
731731
GTEST_DISALLOW_COPY_AND_ASSIGN_(ParameterizedTestSuiteRegistry);
732732
};
733733

734+
// Keep track of what type-parameterized test suite are defined and
735+
// where as well as which are intatiated. This allows susequently
736+
// identifying suits that are defined but never used.
737+
class TypeParameterizedTestSuiteRegistry {
738+
public:
739+
// Add a suite definition
740+
void RegisterTestSuite(const char* test_suite_name,
741+
CodeLocation code_location);
742+
743+
// Add an instantiation of a suit.
744+
void RegisterInstantiation(const char* test_suite_name);
745+
746+
// For each suit repored as defined but not reported as instantiation,
747+
// emit a test that reports that fact (configurably, as an error).
748+
void CheckForInstantiations();
749+
750+
private:
751+
struct TypeParameterizedTestSuiteInfo {
752+
explicit TypeParameterizedTestSuiteInfo(CodeLocation c)
753+
: code_location(c), instantiated(false) {}
754+
755+
CodeLocation code_location;
756+
bool instantiated;
757+
};
758+
759+
std::map<std::string, TypeParameterizedTestSuiteInfo> suites_;
760+
};
761+
734762
} // namespace internal
735763

736764
// Forward declarations of ValuesIn(), which is implemented in

‎googletest/src/gtest-internal-inl.h

+9
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,13 @@ class GTEST_API_ UnitTestImpl {
698698
return parameterized_test_registry_;
699699
}
700700

701+
// Returns TypeParameterizedTestSuiteRegistry object used to keep track of
702+
// type-parameterized tests and instantiations of them.
703+
internal::TypeParameterizedTestSuiteRegistry&
704+
type_parameterized_test_registry() {
705+
return type_parameterized_test_registry_;
706+
}
707+
701708
// Sets the TestSuite object for the test that's currently running.
702709
void set_current_test_suite(TestSuite* a_current_test_suite) {
703710
current_test_suite_ = a_current_test_suite;
@@ -874,6 +881,8 @@ class GTEST_API_ UnitTestImpl {
874881
// ParameterizedTestRegistry object used to register value-parameterized
875882
// tests.
876883
internal::ParameterizedTestSuiteRegistry parameterized_test_registry_;
884+
internal::TypeParameterizedTestSuiteRegistry
885+
type_parameterized_test_registry_;
877886

878887
// Indicates whether RegisterParameterizedTests() has been called already.
879888
bool parameterized_tests_registered_;

‎googletest/src/gtest-typed-test.cc

+4-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ static std::vector<std::string> SplitIntoTestNames(const char* src) {
5858
// registered_tests_; returns registered_tests if successful, or
5959
// aborts the program otherwise.
6060
const char* TypedTestSuitePState::VerifyRegisteredTestNames(
61-
const char* file, int line, const char* registered_tests) {
61+
const char* test_suite_name, const char* file, int line,
62+
const char* registered_tests) {
63+
RegisterTypeParameterizedTestSuite(test_suite_name, CodeLocation(file, line));
64+
6265
typedef RegisteredTestsMap::const_iterator RegisteredTestIter;
6366
registered_ = true;
6467

‎googletest/src/gtest.cc

+59
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ namespace {
415415
//
416416
// This configuration bit will likely be removed at some point.
417417
constexpr bool kErrorOnUninstantiatedParameterizedTest = false;
418+
constexpr bool kErrorOnUninstantiatedTypeParameterizedTest = false;
418419

419420
// A test that fails at a given file/line location with a given message.
420421
class FailureTest : public Test {
@@ -467,6 +468,63 @@ void InsertSyntheticTestCase(const std::string &name, CodeLocation location) {
467468
});
468469
}
469470

471+
void RegisterTypeParameterizedTestSuite(const char* test_suite_name,
472+
CodeLocation code_location) {
473+
GetUnitTestImpl()->type_parameterized_test_registry().RegisterTestSuite(
474+
test_suite_name, code_location);
475+
}
476+
477+
void RegisterTypeParameterizedTestSuiteInstantiation(const char* case_name) {
478+
GetUnitTestImpl()
479+
->type_parameterized_test_registry()
480+
.RegisterInstantiation(case_name);
481+
}
482+
483+
void TypeParameterizedTestSuiteRegistry::RegisterTestSuite(
484+
const char* test_suite_name, CodeLocation code_location) {
485+
suites_.emplace(std::string(test_suite_name),
486+
TypeParameterizedTestSuiteInfo(code_location));
487+
}
488+
489+
void TypeParameterizedTestSuiteRegistry::RegisterInstantiation(
490+
const char* test_suite_name) {
491+
auto it = suites_.find(std::string(test_suite_name));
492+
if (it != suites_.end()) {
493+
it->second.instantiated = true;
494+
} else {
495+
GTEST_LOG_(ERROR) << "Unknown type parameterized test suit '"
496+
<< test_suite_name << "'";
497+
}
498+
}
499+
500+
void TypeParameterizedTestSuiteRegistry::CheckForInstantiations() {
501+
for (const auto& testcase : suites_) {
502+
if (testcase.second.instantiated) continue;
503+
504+
std::string message =
505+
"Type paramaterized test suite " + testcase.first +
506+
" is defined via REGISTER_TYPED_TEST_SUITE_P, but never instantiated "
507+
"via INSTANTIATE_TYPED_TEST_SUITE_P. None of the test cases will run."
508+
"\n\n"
509+
"Ideally, TYPED_TEST_P definitions should only ever be included as "
510+
"part of binaries that intend to use them. (As opposed to, for "
511+
"example, being placed in a library that may be linked in to get other "
512+
"utilities.)";
513+
514+
std::string full_name =
515+
"UninstantiatedTypeParamaterizedTestSuite<" + testcase.first + ">";
516+
RegisterTest( //
517+
"GoogleTestVerification", full_name.c_str(),
518+
nullptr, // No type parameter.
519+
nullptr, // No value parameter.
520+
testcase.second.code_location.file.c_str(),
521+
testcase.second.code_location.line, [message, testcase] {
522+
return new FailureTest(testcase.second.code_location, message,
523+
kErrorOnUninstantiatedTypeParameterizedTest);
524+
});
525+
}
526+
}
527+
470528
// A copy of all command line arguments. Set by InitGoogleTest().
471529
static ::std::vector<std::string> g_argvs;
472530

@@ -2710,6 +2768,7 @@ namespace internal {
27102768
void UnitTestImpl::RegisterParameterizedTests() {
27112769
if (!parameterized_tests_registered_) {
27122770
parameterized_test_registry_.RegisterTests();
2771+
type_parameterized_test_registry_.CheckForInstantiations();
27132772
parameterized_tests_registered_ = true;
27142773
}
27152774
}

‎googletest/test/googletest-output-test-golden-lin.txt

+9-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Expected equality of these values:
1212
3
1313
Stack trace: (omitted)
1414

15-
[==========] Running 86 tests from 41 test suites.
15+
[==========] Running 87 tests from 41 test suites.
1616
[----------] Global test environment set-up.
1717
FooEnvironment::SetUp() called.
1818
BarEnvironment::SetUp() called.
@@ -982,12 +982,17 @@ Expected failure
982982
Stack trace: (omitted)
983983

984984
[ FAILED ] PrintingStrings/ParamTest.Failure/a, where GetParam() = "a"
985-
[----------] 1 test from GoogleTestVerification
985+
[----------] 2 tests from GoogleTestVerification
986986
[ RUN ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<DetectNotInstantiatedTest>
987987
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.
988988

989989
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.)
990990
[ OK ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<DetectNotInstantiatedTest>
991+
[ RUN ] GoogleTestVerification.UninstantiatedTypeParamaterizedTestSuite<DetectNotInstantiatedTypesTest>
992+
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.
993+
994+
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.)
995+
[ OK ] GoogleTestVerification.UninstantiatedTypeParamaterizedTestSuite<DetectNotInstantiatedTypesTest>
991996
[----------] Global test environment tear-down
992997
BarEnvironment::TearDown() called.
993998
googletest-output-test_.cc:#: Failure
@@ -1001,8 +1006,8 @@ Failed
10011006
Expected fatal failure.
10021007
Stack trace: (omitted)
10031008

1004-
[==========] 86 tests from 41 test suites ran.
1005-
[ PASSED ] 32 tests.
1009+
[==========] 87 tests from 41 test suites ran.
1010+
[ PASSED ] 33 tests.
10061011
[ FAILED ] 54 tests, listed below:
10071012
[ FAILED ] NonfatalFailureTest.EscapesStringOperands
10081013
[ FAILED ] NonfatalFailureTest.DiffForLongStrings

‎googletest/test/googletest-output-test_.cc

+15
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,21 @@ class TypedTestPNames {
876876
INSTANTIATE_TYPED_TEST_SUITE_P(UnsignedCustomName, TypedTestP, UnsignedTypes,
877877
TypedTestPNames);
878878

879+
template <typename T>
880+
class DetectNotInstantiatedTypesTest : public testing::Test {};
881+
TYPED_TEST_SUITE_P(DetectNotInstantiatedTypesTest);
882+
TYPED_TEST_P(DetectNotInstantiatedTypesTest, Used) {
883+
TypeParam instantiate;
884+
(void)instantiate;
885+
}
886+
REGISTER_TYPED_TEST_SUITE_P(DetectNotInstantiatedTypesTest, Used);
887+
888+
// kErrorOnUninstantiatedTypeParameterizedTest=true would make the above fail.
889+
// Adding the following would make that test failure go away.
890+
//
891+
// typedef ::testing::Types<char, int, unsigned int> MyTypes;
892+
// INSTANTIATE_TYPED_TEST_SUITE_P(All, DetectNotInstantiatedTypesTest, MyTypes);
893+
879894
#endif // GTEST_HAS_TYPED_TEST_P
880895

881896
#if GTEST_HAS_DEATH_TEST

‎googletest/test/googletest-param-test-test.cc

+5
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,11 @@ namespace works_here {
10721072
// Never used not instantiated, this should work.
10731073
class NotUsedTest : public testing::TestWithParam<int> {};
10741074

1075+
///////
1076+
// Never used not instantiated, this should work.
1077+
template <typename T>
1078+
class NotUsedTypeTest : public testing::Test {};
1079+
TYPED_TEST_SUITE_P(NotUsedTypeTest);
10751080
} // namespace works_here
10761081

10771082
int main(int argc, char **argv) {

‎googletest/test/gtest-typed-test_test.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -228,41 +228,41 @@ class TypedTestSuitePStateTest : public Test {
228228
TEST_F(TypedTestSuitePStateTest, SucceedsForMatchingList) {
229229
const char* tests = "A, B, C";
230230
EXPECT_EQ(tests,
231-
state_.VerifyRegisteredTestNames("foo.cc", 1, tests));
231+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, tests));
232232
}
233233

234234
// Makes sure that the order of the tests and spaces around the names
235235
// don't matter.
236236
TEST_F(TypedTestSuitePStateTest, IgnoresOrderAndSpaces) {
237237
const char* tests = "A,C, B";
238238
EXPECT_EQ(tests,
239-
state_.VerifyRegisteredTestNames("foo.cc", 1, tests));
239+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, tests));
240240
}
241241

242242
using TypedTestSuitePStateDeathTest = TypedTestSuitePStateTest;
243243

244244
TEST_F(TypedTestSuitePStateDeathTest, DetectsDuplicates) {
245245
EXPECT_DEATH_IF_SUPPORTED(
246-
state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, A, C"),
246+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, A, C"),
247247
"foo\\.cc.1.?: Test A is listed more than once\\.");
248248
}
249249

250250
TEST_F(TypedTestSuitePStateDeathTest, DetectsExtraTest) {
251251
EXPECT_DEATH_IF_SUPPORTED(
252-
state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, C, D"),
252+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, C, D"),
253253
"foo\\.cc.1.?: No test named D can be found in this test suite\\.");
254254
}
255255

256256
TEST_F(TypedTestSuitePStateDeathTest, DetectsMissedTest) {
257257
EXPECT_DEATH_IF_SUPPORTED(
258-
state_.VerifyRegisteredTestNames("foo.cc", 1, "A, C"),
258+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, C"),
259259
"foo\\.cc.1.?: You forgot to list test B\\.");
260260
}
261261

262262
// Tests that defining a test for a parameterized test case generates
263263
// a run-time error if the test case has been registered.
264264
TEST_F(TypedTestSuitePStateDeathTest, DetectsTestAfterRegistration) {
265-
state_.VerifyRegisteredTestNames("foo.cc", 1, "A, B, C");
265+
state_.VerifyRegisteredTestNames("Suite", "foo.cc", 1, "A, B, C");
266266
EXPECT_DEATH_IF_SUPPORTED(
267267
state_.AddTestName("foo.cc", 2, "FooTest", "D"),
268268
"foo\\.cc.2.?: Test D must be defined before REGISTER_TYPED_TEST_SUITE_P"

0 commit comments

Comments
 (0)