Have tests build with cmake all target when MFEM_ENABLE_TESTING=ON

[cjvogl]

Personally, I would prefer to have the tests build with the cmake all target when MFEM_ENABLE_TESTING is set to ON during configuration. I believe this would then conform to the behavior of MFEM_ENABLE_EXAMPLES and MFEM_ENABLE_MINIAPPS. It might also be worth having consistency in how such behavior is implemented in CMakelists.txt instead of the three different approaches currently there. I am happy to make everything consistent with how miniapps are handled below (with an additional line to call enable_testing() if MFEM_ENABLE_TESTING=ON:

for testing

if (MFEM_ENABLE_TESTING)
  enable_testing()
  set(MFEM_ALL_TESTS_TARGET_NAME tests)
  add_mfem_target(${MFEM_ALL_TESTS_TARGET_NAME} OFF)
  add_subdirectory(tests EXCLUDE_FROM_ALL)

  # Create a target for all benchmarks and, optionally, enable it.
  set(MFEM_ALL_BENCHMARKS_TARGET_NAME benchmarks)
  add_mfem_target(${MFEM_ALL_BENCHMARKS_TARGET_NAME}
                  ${MFEM_ENABLE_GOOGLE_BENCHMARKS})
  add_subdirectory(tests/benchmarks EXCLUDE_FROM_ALL)
endif()

for examples

set(MFEM_ALL_EXAMPLES_TARGET_NAME examples)
add_mfem_target(${MFEM_ALL_EXAMPLES_TARGET_NAME} ${MFEM_ENABLE_EXAMPLES})
if (MFEM_ENABLE_EXAMPLES)
  add_subdirectory(examples) #install examples if enabled
else()
  add_subdirectory(examples EXCLUDE_FROM_ALL)
endif()

and for miniapps

# Create a target for all miniapps and, optionally, enable it.
set(MFEM_ALL_MINIAPPS_TARGET_NAME miniapps)
add_mfem_target(${MFEM_ALL_MINIAPPS_TARGET_NAME} ${MFEM_ENABLE_MINIAPPS})
add_subdirectory(miniapps EXCLUDE_FROM_ALL)

[v-dobrev]

With regards to testing, I think we want to have 3 options actually:

1. No tests are defined by the MFEM CMake files (useful when MFEM is used as a sub-project, see Guard add_test calls with MFEM_ENABLE_TESTING #2631)
2. All tests are defined in the CMake files but they are not build by the default target, i.e. when running make without a target (this is currently the default behavior)
3. All tests are defined and they are build by the default target.

Maybe we can have MFEM_ENABLE_TESTING switch between 1 (OFF) and 2/3 (ON) with another new option for choosing 2 or 3. Another choice is to have MFEM_ENABLE_TESTING choose between 1/2 (OFF) and 3 (ON) with a another new option for choosing between 1 and 2.

Yet another choice is to treat MFEM_ENABLE_TESTING as a three-state option, so basically OFF will be 1, ON will be 3, and AUTO (or whatever makes most sense) will be 2.

[cjvogl]

Ahh, ok: needing to avoid calls to add_test when MFEM_ENABLE_TESTING=OFF certainly clarifies why the approach to testing is different than from examples and miniapps. Thank you for pointing out the need for option 1!

I personally would be happy with either of your two suggestions. I can see some advantage to leaving MFEM_ENABLE_TASTING to take only ON and OFF values... perhaps the new option in your first suggestion could be 'MFEM_ENABLE_TESTING_NOBUILDwhich isOFFby default to select 3 whenMFEM_ENABLE_TESTING=ON... but can be switched to ONto select 2 whenMFEM_ENABLE_TESTING=ON`?

[cjvogl]

Or perhaps this is cleaner and keeps more consistency with how MFEM_ENABLE_MINIAPPS=ON and MFEM_ENABLE_EXAMPLES=ON means that the miniapps and examples targets are included in the all target:

• introduce new MFEM_TESTING variable

  • OFF (default) means no test targets are created and no calls to add_test
  • ON means test targets are created with corresponding calls to add_test

• refactor meaning of MFEM_ENABLE_TESTING

  • OFF (default) means test targets are not added to all
  • ON sets MFEM_TESTING=ON and adds resulting test targets to all

  How does that sound?

[samuelpmishLLNL]

One issue with the current CMake logic is that regardless of what a user sets for MFEM_ENABLE_EXAMPLES and MFEM_ENABLE_MINIAPPS, the CMake code above will add_subdirectory(examples) and add_subdirectory(miniapps), which means those targets bleed through to the parent project (although not on the ALL target, thankfully).

The following pattern would be more conventional CMake practice (note: MFEM_ENABLE_TESTING option already follows the pattern):

if (MFEM_ENABLE_TESTING)
  enable_testing()
  add_subdirectory(tests)
endif()

if (MFEM_ENABLE_MINIAPPS)
  add_subdirectory(miniapps)
endif()

if (MFEM_ENABLE_BENCHMARKS)
  add_subdirectory(tests/benchmarks)
endif()

if (MFEM_ENABLE_EXAMPLES)
  add_subdirectory(examples)
endif()

That is, when a user says MFEM_ENABLE_FEATURE=OFF the associated targets should never be created.

[cjvogl]

@v-dobrev , this approach is the one that I am familiar with as well... is there something blocking this approach in MFEM? More specifically, is there are reason to call

add_subdirectory(miniapps, OFF)

when MFEM_ENABLE_MINIAPPS=OFF instead of just not calling add_subdirectory (as @samuelpmishLLNL suggested)?