Refactor and fix test/example enable/disable logic for packages and subpackages

[bartlettroscoe]

@fryeguy52

Description

Currently, as described in trilinos/Trilinos#1999 (comment), there is very basic logic for the enable of tests or examples by including test and example directories using tribits_add_test_directories() and tribits_add_example_directories() that looks like, for example:

MACRO(TRIBITS_ADD_TEST_DIRECTORIES)

  IF(${PACKAGE_NAME}_ENABLE_TESTS OR ${PARENT_PACKAGE_NAME}_ENABLE_TESTS)
    FOREACH(TEST_DIR ${ARGN})
      TRIBITS_TRACE_FILE_PROCESSING(PACKAGE  ADD_SUBDIR
        "${CMAKE_CURRENT_SOURCE_DIR}/${TEST_DIR}/CMakeLists.txt")
      ADD_SUBDIRECTORY(${TEST_DIR})
    ENDFOREACH()
  ENDIF()

ENDMACRO()

The problem with this is that if ${PACKAGE_NAME}_ENABLE_TESTS is OFF for some reason like it was turned off because of a missing upstream dependency as in trilinos/Trilinos#1999) but ${PACKAGE_NAME}_ENABLE_TESTS is ON, then the subpackages tests will be enabled and will attempt to build but then will fail.

Another problem with TriBITS is that if someone enables tests for a package as:

  -D <Project>_ENABLE_TESTS=ON \
  -D <Project>_ENABLE_<PackageA>=ON \

this this will, by default, enable both tests and examples for <PackageA>. But if someone sets:

  -D <Project>_ENABLE_<PackageA>=ON \
  -D <PackageA>_ENABLE_TESTS=ON \

then that will only enable the tests and not the examples for <PackageA>. That results in great confusion (which happened in the EMPIRE testing of Panzer and even confused me at one point).

Possible solution

I think the best solution to all of these problems is to make package and subpackage test and example enables/disables behave just like every other hierarchical TriBITS variable. That is, we flow down from the project, to the package, to the subpackage and we want disables to flow down as well. That is, if you disable the tests for a given package, that should disable the tests for all of its subpackages. I would argue the same for testing at the global level as well. But an explicitly set enable/disable at a lower level should override the higher-level setting. (For example, if <Project>_ENABLE_TESTS=OFF but <SomePackage>_ENABLE_TESTS=ON, then <SomePackage>_ENABLE_TESTS=ON should be kept.)

So I think what we want is the following enable logic for tests and examples (once you get to that point in the logic after all of the disables have been set due to disabled upstream dependencies being disabled).

• <Project>_ENABLE_TESTS=ON|OFF default set <Project>_ENABLE_EXAMPLES=ON|OFF
• For each parent package <Package> (order does not matter):
  • <Project>_ENABLE_TESTS=ON|OFF default set <Package>_ENABLE_TESTS=ON|OFF
  • <Project>_ENABLE_EXAMPLES=ON|OFF default set <Package>_ENABLE_EXAMPLES=ON|OFF
  • <Package>_ENABLE_TESTS=ON|OFF default set <Package>_ENABLE_EXAMPLES=ON|OFF
  • For each sub package <Package><SP> (order does not matter):
    • <Package>_ENABLE_TESTS=ON|OFF default set <Package><SP>_ENABLE_TESTS=ON|OFF
    • <Project>_ENABLE_EXAMPLES=ON|OFF default set <Package><SP>_ENABLE_EXAMPLES=ON|OFF
    • <Package><SP>_ENABLE_TESTS=ON|OFF default set <Package><SP>_ENABLE_EXAMPLES=ON|OFF

Then only enable tests for an SE package if ${PACKAGE_NAME}_ENABLE_TESTS=ON and only enable examples for an SE package if ${PACKAGE_NAME}_ENABLE_EXAMPLES=ON, period (where ${PACKAGE_NAME} is the name of the SE package which can either be a parent package or a subpackage). This logic works fine for tribits_add_test_directories() and tribits_add_example_directories() but it is a bit problematic for tests and examples that get added not in one of those directories with tribits_add_executable(), tribits_add_test() or tribits_add_advanced_test(). Really there should be independent functions tribits_add_test_executable(), tribits_add_example_executable(), tribits_add_test(), tribits_add_example_test(), etc. to make this clear but that would be a major refactoring. Instead, I think we can safely make tribits_add_test() and tribits_add_advanced_test() only add the test if either it is in an enabled example or test directory or if ${PACKAGE_NAME}_ENABLE_TESTS=ON.

Also, it would be really nice to see if subpackages tests and examples were enabled or not like:

Processing enabled package: Teuchos (Core(T,E), Parser(T,E), ParameterList(T,E), Comm(T,E), ..., Tests, Examples)

Or, if all of the subpackages had tests or example on of the parent package did, you might just simplify this as:

Processing enabled package: Teuchos (Core, Parser, ParameterList, Comm, ..., Tests, Examples)

which is how it looks now. Then we would only show the Test and Example enables for subpackages if they were different than for their parent setting like:

Processing enabled package: Teuchos (Core(T,E), Parser(T), ParameterList(T,E), Comm(T,E), ..., Tests, Examples)

Above, the examples for the ParameterList subpackage are not enabled due to TeuchosParameterList_ENABLE_EXAMPLES=OFF by some means.

ToDo:

• Update enable/disable logic for tests and examples as described above
• Update logic for tribits_add_test() and tribits_add_advanced_test() as described above
• Add unit tests for all of these new behaviors
• Update TriBITS developers guide and build reference guide for the new behaviors

[bartlettroscoe]

On second thought, I don't think that the logic:

• For each parent package <Package> (order does not matter):
  • If <Project>_ENABLE_TESTS=OFF then set <Package>_ENABLE_TESTS=OFF (override!)
  • If <Project>_ENABLE_EXAMPLES=OFF then set <Package>_ENABLE_EXAMPLES=OFF (override!)
  • For each sub package <Package><SP> (order does not matter):
    • If <Package>_ENABLE_TESTS=OFF then set <Package><SP>_ENABLE_TESTS=OFF (override!)
    • If <Project>_ENABLE_EXAMPLES=OFF then set <Package><SP>_ENABLE_EXAMPLES=OFF (override!)

is a good idea. That will break some existing configure scripts that I think I remember seeing. (For example, the current EMPIRE script for Trilinos sets Trilinos_ENABLE_TESTS=OFF but Panzer_ENABLE_TESTS=ON.)

Also, there is really no need to have disables prorate like this since the test suite from one package will never implicitly enable the test suite for another package. In fact, there is nothing in TriBITS that will implicitly enable any test suite except when setting an upper-level test/example enable triggers a lower-level test/example enable.

Therefore, I will just have test/example enables/disables propagate from project to parent package to subpackage.

I will update the "Proposed Solution" section above for this.

[bartlettroscoe]

I am actually working on this as part of #510.

[bartlettroscoe]

This is not quite complete due to a defect discovered by a customer and reported in #11002 (see trilinos/Trilinos#11002 (comment)).

[bartlettroscoe]

PR #529 should fix this defect.

[bartlettroscoe]

I was confirmed in trilinos/Trilinos#11002 (comment) that the problem reported in trilinos/Trilinos#11002 (comment) is addressed in Trilinos 'develop'.

Closing as complete again.