-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
Make Google Test usage configurable in CMake files #3628
Conversation
CMakeLists.txt
Outdated
string(REPLACE ".cc" "" __execname ${__srcname}) | ||
add_executable(${__execname} ${__srcpath}) | ||
list(APPEND TEST_EXECS ${__execname}) | ||
target_include_directories(${__execname} PUBLIC ${GTEST_INCLUDE_DIR}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added this line because clang
doesn't search in /usr/local/include
by default, but that's where the gtest
header is, so now we include that path.
Before adding this, I was getting a bug where the cpptest
target would be generated by CMake, but when I ran make cpptest
, it couldn't find gtest/gtest.h
.
I was hoping we can do something different. When user has gtest, make cpptest as normal. When the user do not have gtest, nothing will happen. but when user type make cpptest, it will echo the message asking the user to install gtest. In order to do that, we should add a different custom target that executes the echo command when gtest is not found. |
@tqchen Let me know if the most recent version is in line with what you requested. |
* Add USE_GTEST as a CMake variable * Add GTest section in installation docs * Incorporate feedback
* Add USE_GTEST as a CMake variable * Add GTest section in installation docs * Incorporate feedback
Google Test has been an implicit dependency to run the C++ tests. This PR makes it explicit and configurable in the CMake rules, which in turn allows us to give better error messages if it isn't installed (rather than secretly not adding
cpptest
as a build target).This PR additionally adds docs at the end of the current installation docs for enabling Google Test. Perhaps eventually separate doc pages for users and developers will be required, but for now, the difference in dependencies is not great enough to warrant a full split yet.
CC @tqchen @vinx13 @jroesch