Skip to content
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

Add MAIN_PROJECT check for test option #445

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Add MAIN_PROJECT check for test option #445

merged 2 commits into from
Dec 15, 2020

Conversation

globberwops
Copy link
Contributor

@globberwops globberwops commented Dec 9, 2020

Description

When adding doctest to a CMake project using FetchContent, testing has to be explicitly disabled.
This check only enables testing, if doctest is the main project being built.

This change enables the cleaner usage of doctest with FetchContent.

Before

FetchContent_Declare(
  onqtam_doctest
  GIT_REPOSITORY https://github.com/onqtam/doctest.git
  GIT_TAG dev)
set(DOCTEST_WITH_TESTS OFF)
FetchContent_MakeAvailable(onqtam_doctest)

After

FetchContent_Declare(
  onqtam_doctest
  GIT_REPOSITORY https://github.com/onqtam/doctest.git
  GIT_TAG dev)
FetchContent_MakeAvailable(onqtam_doctest)

GitHub Issues

none

@YarikTH
Copy link

YarikTH commented Dec 14, 2020

For reference, here is the same purpose code from {fmt}

# Determine if fmt is built as a subproject (using add_subdirectory)
# or if it is the master project.
set(MASTER_PROJECT OFF)
if (CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
  set(MASTER_PROJECT ON)
  message(STATUS "CMake version: ${CMAKE_VERSION}")
endif ()
...
...
...
# Options that control generation of various targets.
option(FMT_DOC "Generate the doc target." ${MASTER_PROJECT})
option(FMT_INSTALL "Generate the install target." ${MASTER_PROJECT})
option(FMT_TEST "Generate the test target." ${MASTER_PROJECT})
option(FMT_FUZZ "Generate the fuzz target." OFF)
option(FMT_CUDA_TEST "Generate the cuda-test target." OFF)
option(FMT_OS "Include core requiring OS (Windows/Posix) " ON)

So it might be a good idea at least copy explanatory comment and maybe use CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR check instead of CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR.

@globberwops
Copy link
Contributor Author

@YarikTH I am having a déjà vu ;) Done 👍🏻

@onqtam
Copy link
Member

onqtam commented Dec 15, 2020

Thanks for this! merging :)

@onqtam onqtam merged commit 12a4857 into doctest:dev Dec 15, 2020
onqtam pushed a commit that referenced this pull request Dec 15, 2020
* Add MAIN_PROJECT check for test option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants