Skip to content

Skip building tests by default #993

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 7 commits into from
Aug 9, 2023
Merged

Skip building tests by default #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f40a8b4
Skip building tests by default
kkloberdanz Aug 4, 2023
7120e2a
Update src/bsoncxx/test/CMakeLists.txt
kkloberdanz Aug 7, 2023
6ac4c03
rename the variable to skip tests to BUILD_TESTING
kkloberdanz Aug 7, 2023
e288860
exclude from all if testing is not enabled
kkloberdanz Aug 8, 2023
90e0ecc
Merge branch 'master' into rename-skip-test-variable
kkloberdanz Aug 8, 2023
c98ef25
remove extra newline
kkloberdanz Aug 8, 2023
22327be
move this below project
kkloberdanz Aug 9, 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
2 changes: 1 addition & 1 deletion .evergreen/compile.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ case "$OS" in
esac

cd build
"${cmake_binary}" -G "$GENERATOR" "-DCMAKE_BUILD_TYPE=${BUILD_TYPE}" -DMONGOCXX_ENABLE_SLOW_TESTS=ON -DENABLE_UNINSTALL=ON "$@" ..
"${cmake_binary}" -G "$GENERATOR" "-DCMAKE_BUILD_TYPE=${BUILD_TYPE}" -DBUILD_TESTING=ON -DMONGOCXX_ENABLE_SLOW_TESTS=ON -DENABLE_UNINSTALL=ON "$@" ..
"${cmake_binary}" --build . --config $BUILD_TYPE -- $CMAKE_BUILD_OPTS
"${cmake_binary}" --build . --config $BUILD_TYPE --target install -- $CMAKE_BUILD_OPTS
"${cmake_binary}" --build . --config $BUILD_TYPE --target $CMAKE_EXAMPLES_TARGET -- $CMAKE_BUILD_OPTS
Expand Down
8 changes: 6 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ endif()

project(MONGO_CXX_DRIVER LANGUAGES CXX)

if(NOT DEFINED BUILD_TESTING)
set(BUILD_TESTING OFF) # Set this to OFF by default
endif()

if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.8.2")
message(FATAL_ERROR "Insufficient GCC version - GCC 4.8.2+ required")
Expand Down Expand Up @@ -102,8 +106,8 @@ if(NEED_DOWNLOAD_C_DRIVER)
add_subdirectory(${mongo-c-driver_SOURCE_DIR} ${mongo-c-driver_BINARY_DIR})
set(CMAKE_CXX_FLAGS ${OLD_CMAKE_CXX_FLAGS})
set(CMAKE_C_FLAGS ${OLD_CMAKE_C_FLAGS})
set(ENABLE_TESTS OLD_ENABLE_TESTS)
set(BUILD_TESTING OLD_BUILD_TESTING)
set(ENABLE_TESTS ${OLD_ENABLE_TESTS})
set(BUILD_TESTING ${OLD_BUILD_TESTING})
endif()
message(STATUS "Download and configure C driver version ${LIBMONGOC_DOWNLOAD_VERSION} ... end")
endif()
Expand Down
4 changes: 4 additions & 0 deletions src/bsoncxx/test/CMakeLists.txt
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

CMake has an semi-built-in option for this feature, BUILD_TESTING. At the top CMakeLists.txt, add:

if(NOT DEFINED BUILD_TESTING)
  set(BUILD_TESTING OFF)  # Set this to OFF by default
endif()
include(CTest)

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious if this will work, but I haven't tested it:

if(NOT BUILD_TESTING)
  set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL TRUE)
endif()

(and then the remainder of the file is reverted to as-it-was). IIUC, this should exclude every added target from the implicit all, but still defined them and are available for building by being named explicitly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with the suggestion to exclude test targets from all, but still have them defined.

Example: Targets in the examples directory are currently excluded from the all target with:

add_custom_target(examples DEPENDS ${MONGOCXX_EXAMPLE_EXECUTABLES} ${BSONCXX_EXAMPLE_EXECUTABLES})

But the example targets are still defined. CMake targets can be listed with cmake --build cmake-build --target help. An example target can be built by including the --target argument: cmake --build cmake-build --target aggregation_examples

Copy link
Contributor Author

@kkloberdanz kkloberdanz Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This doesn't seem to work when I try it. The test programs are built by default when I implement this like so:

CMakeLists.txt:

if(NOT DEFINED BUILD_TESTING)
  set(BUILD_TESTING OFF)  # Set this to OFF by default
endif()

src/bsoncxx/test/CMakeLists.txt and src/mongocxx/test/CMakeLists.txt:

if(NOT BUILD_TESTING)
  set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL TRUE)
endif()

include_directories(
  ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if I can figure why the directory property doesn't work, so just discard that suggestion for now. On the other hand, the BUILD_TESTING variable should work? Are you sure to clear it from your cache before configuring?

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 think I found the issue, and after applying the fix below, your suggestion then works. I pushed these changes and the tests are green.

-        set(ENABLE_TESTS OLD_ENABLE_TESTS)
-        set(BUILD_TESTING OLD_BUILD_TESTING)
+        set(ENABLE_TESTS ${OLD_ENABLE_TESTS})
+        set(BUILD_TESTING ${OLD_BUILD_TESTING})

if(NOT BUILD_TESTING)
set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL TRUE)
endif()

include_directories(
${MONGO_CXX_DRIVER_SOURCE_DIR}/src/third_party/catch/include
)
Expand Down
4 changes: 4 additions & 0 deletions src/mongocxx/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.

if(NOT BUILD_TESTING)
set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL TRUE)
endif()

include_directories(
${THIRD_PARTY_SOURCE_DIR}/catch/include
)
Expand Down