Skip to content

Commit

Permalink
Googletest export
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Abseil Team authored and Andy Soffer committed Jan 2, 2020
1 parent 008629a commit a13a062
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 14 deletions.
2 changes: 1 addition & 1 deletion googletest/include/gtest/gtest-typed-test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Expand Down
11 changes: 9 additions & 2 deletions googletest/include/gtest/internal/gtest-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, CodeLocation> RegisteredTestsMap;
Expand Down Expand Up @@ -750,6 +751,11 @@ class TypeParameterizedTest<Fixture, TestSel, internal::None> {
}
};

GTEST_API_ void RegisterTypeParameterizedTestSuite(const char* test_suite_name,
CodeLocation code_location);
GTEST_API_ void RegisterTypeParameterizedTestSuiteInstantiation(
const char* case_name);

// TypeParameterizedTestSuite<Fixture, Tests, Types>::Register()
// registers *all combinations* of 'Tests' and 'Types' with Google
// Test. The return value is insignificant - we just need to return
Expand All @@ -762,6 +768,7 @@ class TypeParameterizedTestSuite {
const char* test_names,
const std::vector<std::string>& type_names =
GenerateNames<DefaultNameGenerator, Types>()) {
RegisterTypeParameterizedTestSuiteInstantiation(case_name);
std::string test_name = StripTrailingSpaces(
GetPrefixUntilComma(test_names));
if (!state->TestExists(test_name)) {
Expand Down
28 changes: 28 additions & 0 deletions googletest/include/gtest/internal/gtest-param-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, TypeParameterizedTestSuiteInfo> suites_;
};

} // namespace internal

// Forward declarations of ValuesIn(), which is implemented in
Expand Down
9 changes: 9 additions & 0 deletions googletest/src/gtest-internal-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_;
Expand Down
5 changes: 4 additions & 1 deletion googletest/src/gtest-typed-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ static std::vector<std::string> 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;

Expand Down
59 changes: 59 additions & 0 deletions googletest/src/gtest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<std::string> g_argvs;

Expand Down Expand Up @@ -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;
}
}
Expand Down
13 changes: 9 additions & 4 deletions googletest/test/googletest-output-test-golden-lin.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<DetectNotInstantiatedTest>
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<DetectNotInstantiatedTest>
[ RUN ] GoogleTestVerification.UninstantiatedTypeParamaterizedTestSuite<DetectNotInstantiatedTypesTest>
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<DetectNotInstantiatedTypesTest>
[----------] Global test environment tear-down
BarEnvironment::TearDown() called.
googletest-output-test_.cc:#: Failure
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions googletest/test/googletest-output-test_.cc
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,21 @@ class TypedTestPNames {
INSTANTIATE_TYPED_TEST_SUITE_P(UnsignedCustomName, TypedTestP, UnsignedTypes,
TypedTestPNames);

template <typename T>
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<char, int, unsigned int> MyTypes;
// INSTANTIATE_TYPED_TEST_SUITE_P(All, DetectNotInstantiatedTypesTest, MyTypes);

#endif // GTEST_HAS_TYPED_TEST_P

#if GTEST_HAS_DEATH_TEST
Expand Down
5 changes: 5 additions & 0 deletions googletest/test/googletest-param-test-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,11 @@ namespace works_here {
// Never used not instantiated, this should work.
class NotUsedTest : public testing::TestWithParam<int> {};

///////
// Never used not instantiated, this should work.
template <typename T>
class NotUsedTypeTest : public testing::Test {};
TYPED_TEST_SUITE_P(NotUsedTypeTest);
} // namespace works_here

int main(int argc, char **argv) {
Expand Down
12 changes: 6 additions & 6 deletions googletest/test/gtest-typed-test_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,41 +228,41 @@ 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
// don't matter.
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"
Expand Down

0 comments on commit a13a062

Please sign in to comment.