Set kErrorOnUninstantiatedParameterizedTest et al. to true

[bcsgh]

This will start reporting missing instantiations of parameterized tests as a failing test under the GoogleTestVerification test suite (they are currently reported, but just as a notice from a passing test).

NOTE: It is expected that this will surface existing issues in downstream consumers.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[asoffer]

As discussed, this may take some time to fix internal users, but we do intend to accept this eventually.

[bcsgh]

Yup. That was what I was expecting. Thanks.

[bcsgh]

BTW: seems this fails presubmit by catching an extra INSTANTIATE_TEST_SUITE_P in the tests. It seems the issue is a orphaned .cc file and another that an overly wide glob() is picking up:

The relevant bit of the log:

[----------] 2 tests from GoogleTestVerification [ RUN ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<ExternalInstantiationTest> googletest/test/googletest-param-test2-test.cc:51: Failure

Paramaterized test suite ExternalInstantiationTest is instantiated via INSTANTIATE_TEST_SUITE_P, but no tests are defined via TEST_P . No test cases will run.

Ideally, INSTANTIATE_TEST_SUITE_P should only ever be invoked from code that always depend on code that provides TEST_P. Failing to do so is often an indication of dead code, e.g. the last TEST_P was removed but the rest got left behind.

To suppress this error for this test suite, insert the following line (in a non-header) in the namespace it is defined in:

GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ExternalInstantiationTest); Stack trace: ...

[ FAILED ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<ExternalInstantiationTest> (1 ms) [ RUN ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<InstantiationInMultipleTranslationUnitsTest> googletest/test/googletest-param-test2-test.cc:60: Failure

Paramaterized test suite InstantiationInMultipleTranslationUnitsTest is instantiated via INSTANTIATE_TEST_SUITE_P, but no tests are defined via TEST_P . No test cases will run.

Ideally, INSTANTIATE_TEST_SUITE_P should only ever be invoked from code that always depend on code that provides TEST_P. Failing to do so is often an indication of dead code, e.g. the last TEST_P was removed but the rest got left behind.

To suppress this error for this test suite, insert the following line (in a non-header) in the namespace it is defined in:

GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InstantiationInMultipleTranslationUnitsTest); Stack trace: ... [ FAILED ] GoogleTestVerification.UninstantiatedParamaterizedTestSuite<InstantiationInMultipleTranslationUnitsTest> (0 ms)

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[bcsgh]

Bump? Any Progress on this?

[antoniovicente]

To allow gradual detection and fix: would it be possible to add a compile time option to enable this mode?

[bcsgh]

IIRC that was actually proposed (by me, internally to Google) at one point. I don't remember the details, but it got turned down. (I think it ended up with me reluctantly agreeing with the argument?)

[antoniovicente]

IIRC that was actually proposed (by me, internally to Google) at one point. I don't remember the details, but it got turned down. (I think it ended up with me reluctantly agreeing with the argument?)

So it goes. I was fortunate enough to be cc'ed on an google-internal report about some tests in an open-source project that I'm a part of which are not running. I'm sad that I can't protect the open source project from regressing once the relevant tests are fixed.

[yurykats]

It happened: https://github.com/google/googletest/commit/ec94d9f24c92a5090fda5567156d6dde99cdbf31

[mr-c]

Hello, has googletest switched to "live at HEAD" instead of regular releases? If so, has a refactoring tool been issued for this change, as per the policy ?

Is there a changelog or other announcement mechanism that developers should be subscribing to?

Debian packaged a recent git snapshot of googletest, and now other Debian packages that build and run googletest tests at packaging building time are broken :-(

[bcsgh]

Note, this is mostly a bunch of anecdotal observations, so take them with a suitable number of grains of salt.

tl;dr; I don't think it's practicable to automate a significant portion of the work needed to fix issues this surfaces. I would suggest the best course of action is to manually fix things. In the case where there isn't an underlying bug being masked by the missing test, this is easy. If there is such a bug, then that bug will likely be the majority of the work.

---

I did the original work for building and deploying this internal to Google. In doing that, specifically in fixing the existing cases, there are approximately three actions that were taken to fix things: 1) Delete the Test case/suite, 2) Add a GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST or 3) Add instantiations. And the split between them was not too far off equal.

As for automating things:

• Inside Google, case 2 was automated using clang's RefactoringEngine (that may still be around, but I no longer have access), but that was mostly because of the quantity of cases to be handled rater than them being hard to handle. And this is actually the least preferred resolution is most case as it can easily paper over actual bugs.
• Case 1 could also be easily automated in a similar way (and shouldn't be bulk applied, for similar reasons).
• Case 3 is fundamentally impossible to automate. Empirically, even tests value parameterized on bool have a non-negligible number of cases that can't just be instantiated for both true/false. I tried automating that for the cases I had access to and a bunch of cases still needed manual intervention (e.g. one or the other cases may only be valid in particular build environments).

And even then, the decision of which of those cases to apply will always require semantic understanding of the project that tools will never have (or if they do get that, then I'm out of work). At best, you can apply case 2 and file bugs to go back end manual inspect them.

Furthermore, in my experience, a non-trivial number of cases where the fix was to add instantiations surfaced real bugs which, in a nutshell, was the original motivation for the whole project.

[bcsgh]

kErrorOnUninstantiatedParameterizedTest et al. should probably be removed at this point. They have been true for 3.5 years.