Set kErrorOnUninstantiatedParameterizedTest et al. to true#2683
Set kErrorOnUninstantiatedParameterizedTest et al. to true#2683bcsgh wants to merge 18 commits intogoogle:mainfrom
Conversation
…eporting missing instantiations as errors.
|
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). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
As discussed, this may take some time to fix internal users, but we do intend to accept this eventually. |
|
Yup. That was what I was expecting. Thanks. |
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: #2683 PiperOrigin-RevId: 291398123
|
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:
|
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: #2683 PiperOrigin-RevId: 291398123
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: #2683 PiperOrigin-RevId: 291398123
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: #2683 PiperOrigin-RevId: 291398123
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: #2683 PiperOrigin-RevId: 291398123
Add missing explicit keyword for gmock_Impl constructor. When switching to using GMOCK_PP in ACTION* macroses `explicit` keyword was missed in gmock_Impl constructor causing ClangTidy warnings in ACTION_P macro. PiperOrigin-RevId: 291159975
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: google#2683 PiperOrigin-RevId: 291398123
Move part of functionality of Matcher* class to the base one. Reduce copypaste. Make constructor and conversion operator of Matcher* class independent of pump. PiperOrigin-RevId: 291405510
Adds missing `#define` guard around `TEST_F(...)` PiperOrigin-RevId: 291703056
Create implementation macroses for matchers to move variadic parameters to the end of parameters list. To save backward compatibility, old macroses will be still taking `description` parameter as the last one. But they will use INTERNAL macro that takes `description` as the second parameter. PiperOrigin-RevId: 291724469
Add includes for type_traits and utility to gmock-function-mocker.h: macros in the file require these headers. PiperOrigin-RevId: 291782497
Add documentation for ASSERT_DEBUG_DEATH/EXPECT_DEBUG_DEATH PiperOrigin-RevId: 292138974
Fix use of reserved names. Minimize code duplication needed for explict-vs-nonexplicit constructor. PiperOrigin-RevId: 292555014
Disable warning C4800 for Visual Studio 2019. The compiler warning C4800 is disabled by default in Visual Studio 2019, but it can be enabled on the command line. The only version of Visual Studio that does not support warning C4800 is Visual Studio 2017. PiperOrigin-RevId: 292624510
Fix std::move to std::forward where appropriate to support reference types. PiperOrigin-RevId: 292923058
Pass method's parameters count to internal GMOCK_METHOD* macro. This will help removing copypaste in every GMOCK_METHOD* macro in future. PiperOrigin-RevId: 292932554
...text exposed to GitHub repo https://www.github.com/google/googletest PiperOrigin-RevId: 293438092
Get rid of gmock-generated-function-mockers.h and gmock-generated-function-mockers.h.pump. Stop using pump for GMOCK_METHOD* macroses generation. PiperOrigin-RevId: 293454519
Tag the function generated by MATCHER with GTEST_ATTRIBUTE_UNUSED_ to fix CI builds of gmock-matchers_test.cc vs. -Wunused-function. See google#2697 for breakage. PiperOrigin-RevId: 293669752
Get rid of gmock-generated-matchers.h and gmock-generated-matchers.h.pump. Stop using pump for MATCHER* macroses generation. PiperOrigin-RevId: 293878808
…eporting missing instantiations as errors.
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
Bump? Any Progress on this? |
|
To allow gradual detection and fix: would it be possible to add a compile time option to enable this mode? |
|
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. |
Commit Message: test: Fix missing instantiation of parameterized tests. Additional Description: gtest silently skips parameterized that have no INSTANTIATE_TEST_SUITE_P associated with them as described at google/googletest#2683 . Thanks @yurykats for pointing me at these failing tests. Also, filed #11246 for redis tests that are not running for this same reason. Risk Level: n/a Testing: test-only changes Docs Changes: n/a Release Notes: n/a Signed-off-by: Antonio Vicente <avd@google.com>
|
It happened: ec94d9f |
|
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 :-( |
|
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 As for automating things:
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. |
|
|
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: google/googletest#2683 PiperOrigin-RevId: 291398123
Deleted an orphaned duplicate file and exclude another that shouldn't be part of :gtest_all_test. This showed up while trying to debug the presubmit failure for: google/googletest#2683 PiperOrigin-RevId: 291398123
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.