Skip to content

Update GoogleTest to v1.14.0 #65823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Update GoogleTest to v1.14.0 #65823

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ff5ada
[unittest] Add missing gtest uninstantiated calls for disabled tests
zeroomega Sep 8, 2023
ff67955
[unittest] Enable ImportMatrixType in ASTTest
zeroomega Sep 8, 2023
ac0785c
Bump googletest to v1.14.0
zeroomega Sep 8, 2023
745a762
amend! Bump googletest to v1.14.0
zeroomega Sep 11, 2023
2fe51a3
amend! amend! Bump googletest to v1.14.0
zeroomega Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,8 @@ ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_SUITE(
INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, FunctionConservative,
DefaultTestValuesForRunOptions );
#else
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(FunctionConservative);
#endif
INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, TypedefConservative,
Expand Down Expand Up @@ -631,6 +633,8 @@ INSTANTIATE_TEST_SUITE_P(
//INSTANTIATE_TEST_SUITE_P(
//ODRViolationTests, VarTemplateConservative,
//DefaultTestValuesForRunOptions);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(VarTemplateConservative);

INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, FunctionTemplateSpecConservative,
DefaultTestValuesForRunOptions);
Expand All @@ -641,12 +645,15 @@ INSTANTIATE_TEST_SUITE_P(
//INSTANTIATE_TEST_SUITE_P(
//ODRViolationTests, VarTemplateSpecConservative,
//DefaultTestValuesForRunOptions);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(VarTemplateSpecConservative);

// FIXME: These fail on Windows.
#if !defined(_WIN32)
INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, FunctionLiberal,
DefaultTestValuesForRunOptions);
#else
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(FunctionLiberal);
#endif
INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, TypedefLiberal,
Expand Down Expand Up @@ -679,6 +686,8 @@ INSTANTIATE_TEST_SUITE_P(
// INSTANTIATE_TEST_SUITE_P(
// ODRViolationTests, VarTemplateLiberal,
// DefaultTestValuesForRunOptions);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(VarTemplateLiberal);

INSTANTIATE_TEST_SUITE_P(
ODRViolationTests, ClassTemplateSpecLiberal,
DefaultTestValuesForRunOptions);
Expand All @@ -689,6 +698,7 @@ INSTANTIATE_TEST_SUITE_P(
//INSTANTIATE_TEST_SUITE_P(
//ODRViolationTests, VarTemplateSpecLiberal,
//DefaultTestValuesForRunOptions );
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(VarTemplateSpecLiberal);

// clang-format on

Expand Down
8 changes: 8 additions & 0 deletions clang/unittests/AST/ASTImporterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9227,5 +9227,13 @@ INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportAttributes,
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportInjectedClassNameType,
DefaultTestValuesForRunOptions);

INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportMatrixType,
DefaultTestValuesForRunOptions);

// FIXME: Make ImportOpenCLPipe test work.
// INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportOpenCLPipe,
// DefaultTestValuesForRunOptions);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ImportOpenCLPipe);

} // end namespace ast_matchers
} // end namespace clang
10 changes: 5 additions & 5 deletions third-party/unittest/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,18 @@ endif ()
target_include_directories(llvm_gtest
PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/googletest/include>
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/googlemock/include>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/llvm-gtest/>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/llvm-gmock/>
$<INSTALL_INTERFACE:include/llvm-gtest/>
$<INSTALL_INTERFACE:include/llvm-gmock/>
PRIVATE googletest googlemock
)

add_subdirectory(UnitTestMain)

if (LLVM_INSTALL_GTEST)
install(TARGETS llvm_gtest llvm_gtest_main LLVMTestingSupport LLVMTestingAnnotations
ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" COMPONENT llvm_gtest)
install(DIRECTORY googletest/include/gtest/ DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/llvm-gtest/gtest/" COMPONENT llvm_gtest)
install(DIRECTORY googlemock/include/gmock/ DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/llvm-gmock/gmock/" COMPONENT llvm_gtest)
ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT llvm_gtest)
install(DIRECTORY googletest/include/gtest/ DESTINATION include/llvm-gtest/gtest/ COMPONENT llvm_gtest)
install(DIRECTORY googlemock/include/gmock/ DESTINATION include/llvm-gmock/gmock/ COMPONENT llvm_gtest)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is undoing a change made recently in 91b3ca3. Is that intentional or was your checkout just stale?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My checkout just stales. Thanks for pointing it out.

endif()

# When LLVM_LINK_LLVM_DYLIB is enabled, libLLVM.so is added to the interface
Expand Down
6 changes: 1 addition & 5 deletions third-party/unittest/googlemock/README.LLVM
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
LLVM notes
----------

This directory contains the 'googlemock' component of Google Test 1.10.0, with
This directory contains the 'googlemock' component of Google Test 1.14.0, with
all elements removed except for the actual source code, to minimize the
addition to the LLVM distribution.

Expand All @@ -10,14 +10,10 @@ Cleaned up as follows:
# Remove all the unnecessary files and directories
$ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore
$ rm -rf build-aux make msvc scripts test docs
$ rm -f `find . -name \*\.pump`
$ rm -f src/gmock_main.cc

# Put the license in the consistent place for LLVM.
$ mv LICENSE LICENSE.TXT

Modified as follows:
* Support for std::begin/std::end in gmock-matchers.h
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still true? If it is, it sounds like the kind of patch that ought to be made in upstream googletest. If not, please remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, gmock-matchers.h still have this part patched, see https://gist.github.com/zeroomega/fb24b1d1c4252b852200e15fda29384d#file-gtest_llvm_modification-patch-L251 . When locally test it, without the patch, LLVM unit tests will have build errors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems odd, and worth following up with upstream googletest. In the meantime, I think it would be clearer to future maintainers if we did something to clearly identify the code blocks that were changed, and why. For example,
// LLVM local change to support std::begin/std::end
followed by the new code, and then the old code commented-out. That way diffs will provide clues, and merges/commits will have conflicts that someone will need to sort out properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the local changes into commented blocks in change in amend commit: 2fe51a3 in this PR, please take a look.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at that specific commit, seems fine.

* IWYU pragmas
* Disabled -Wdeprecated-copy for clang
* Added IWYU pragmas from https://github.com/google/googletest/commit/100f6fbf5f81a82d163c1e29735e8a2936eacd4f
Loading