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.

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

Fix test dependencies for subpackages and subpackage tests/examples enables (#63, #268, #299, #510) #512

Merged
merged 5 commits into from
Aug 11, 2022
Merged

Fix test dependencies for subpackages and subpackage tests/examples enables (#63, #268, #299, #510) #512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bfd9b6f
Add some build reference doc for existing behavior (#510, #511)
bartlettroscoe Aug 11, 2022
7128677
Factor out tribits_add_test_example_directories_assert_call_context()…
bartlettroscoe Aug 11, 2022
1b9f3b6
WIP: Add failing tests for test dependencies defect (#268, #510)
bartlettroscoe Aug 11, 2022
b00ab33
Fix test dependencies for subpackages and subpackage tests/examples e…
bartlettroscoe Aug 11, 2022
e1416eb
Make new tests pass for non-MPI config of TriBITS (#510)
bartlettroscoe Aug 11, 2022
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
141 changes: 141 additions & 0 deletions test/core/ExamplesUnitTests/TribitsExampleProject_Tests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2121,6 +2121,147 @@ tribits_add_advanced_test( TribitsExampleProject_SKIP_CTEST_ADD_TEST_Package_Bla
)


########################################################################

tribits_add_advanced_test( TribitsExampleProject_EnableWithSubpackagesB_EnableWithsubpackagesTests
OVERALL_WORKING_DIRECTORY TEST_NAME
OVERALL_NUM_MPI_PROCS 1
EXCLUDE_IF_NOT_TRUE ${PROJECT_NAME}_ENABLE_Fortran
XHOSTTYPE "Darwin"

TEST_0
MESSAGE "Configure enabling a subpackage and parent package tests"
CMND ${CMAKE_COMMAND}
ARGS
${TribitsExampleProject_COMMON_CONFIG_ARGS}
-DTribitsExProj_TRIBITS_DIR=${${PROJECT_NAME}_TRIBITS_DIR}
-DTribitsExProj_ENABLE_WithSubpackagesB=ON
-DWithSubpackages_ENABLE_TESTS=ON
-DTribitsExProj_DUMP_PACKAGE_DEPENDENCIES=ON
${${PROJECT_NAME}_TRIBITS_DIR}/examples/TribitsExampleProject
PASS_REGULAR_EXPRESSION_ALL
"Enabling subpackage tests/examples based on parent package tests/examples enables"
"-- Setting WithSubpackages_ENABLE_EXAMPLES=ON because WithSubpackages_ENABLE_TESTS=ON"
"-- Setting WithSubpackagesB_ENABLE_TESTS=ON because parent package WithSubpackages_ENABLE_TESTS=ON"
"-- Setting WithSubpackagesB_ENABLE_EXAMPLES=ON because parent package WithSubpackages_ENABLE_EXAMPLES=ON"
"-- WithSubpackagesB_TEST_ENABLED_DEPENDENCIES: MixedLang"
"-- WithSubpackagesB_TEST_ALL_DEPENDENCIES: MixedLang"
"Processing enabled package: WithSubpackages [(]A, B, Tests, Examples[)]"
"Configuring done"
"Generating done"
ALWAYS_FAIL_ON_NONZERO_RETURN

TEST_1 CMND make ARGS ${CTEST_BUILD_FLAGS}

TEST_2 CMND ${CMAKE_CTEST_COMMAND}
PASS_REGULAR_EXPRESSION_ALL
"WithSubpackagesB_test_of_b [.]* *Passed"
"WithSubpackagesB_test_of_b_mixed_lang.* [.]* *Passed"
"100% tests passed, 0 tests failed out of 2"

)
# NOTE: The above test covers one of the use cases for the the defect
# described in TriBITSPub/TriBITS#510. This shows that only the tests for
# WithSubpackagesB are enabled and run and not for any of the other
# subpackages of of the parent package WithSubpackages


########################################################################


tribits_add_advanced_test( TribitsExampleProject_EnableWithSubpackagesB_EnableWithsubpackagesExamples
OVERALL_WORKING_DIRECTORY TEST_NAME
OVERALL_NUM_MPI_PROCS 1
EXCLUDE_IF_NOT_TRUE ${PROJECT_NAME}_ENABLE_Fortran
XHOSTTYPE "Darwin"

TEST_0
MESSAGE "Configure enabling a subpackage and parent package examples"
CMND ${CMAKE_COMMAND}
ARGS
${TribitsExampleProject_COMMON_CONFIG_ARGS}
-DTribitsExProj_TRIBITS_DIR=${${PROJECT_NAME}_TRIBITS_DIR}
-DTribitsExProj_ENABLE_WithSubpackagesB=ON
-DWithSubpackages_ENABLE_EXAMPLES=ON
-DTribitsExProj_DUMP_PACKAGE_DEPENDENCIES=ON
${${PROJECT_NAME}_TRIBITS_DIR}/examples/TribitsExampleProject
PASS_REGULAR_EXPRESSION_ALL
"Enabling subpackage tests/examples based on parent package tests/examples enables"
"-- Setting WithSubpackagesB_ENABLE_EXAMPLES=ON because parent package WithSubpackages_ENABLE_EXAMPLES=ON"
"-- WithSubpackagesB_TEST_ENABLED_DEPENDENCIES: MixedLang"
"-- WithSubpackagesB_TEST_ALL_DEPENDENCIES: MixedLang"
"Processing enabled package: WithSubpackages [(]A, B, Examples[)]"
"Configuring done"
"Generating done"
ALWAYS_FAIL_ON_NONZERO_RETURN

TEST_1 CMND make ARGS ${CTEST_BUILD_FLAGS}

TEST_2 CMND ${CMAKE_CTEST_COMMAND}
PASS_REGULAR_EXPRESSION_ALL
"No tests were found"

)
# NOTE: The above test ensures that
# WithSubpackagesB_TEST_ENABLED_DEPENDENCIES=MixedLang gets set but that the
# tests are not actually run because only examples where enabled. This was
# not a use case for the Trilinos failures reported in TriBITSPub/TriBITS#510
# but this is a valid use case that needs to be supported and tested.


########################################################################


tribits_add_advanced_test( TribitsExampleProject_ST_EnableMixedLang_EnableAllForwardDepPackages
OVERALL_WORKING_DIRECTORY TEST_NAME
OVERALL_NUM_MPI_PROCS 1
EXCLUDE_IF_NOT_TRUE ${PROJECT_NAME}_ENABLE_Fortran
XHOSTTYPE "Darwin"

TEST_0
MESSAGE "Configure enabling an upstream package and enableee forward dep packages"
CMND ${CMAKE_COMMAND}
ARGS
${TribitsExampleProject_COMMON_CONFIG_ARGS}
-DTribitsExProj_TRIBITS_DIR=${${PROJECT_NAME}_TRIBITS_DIR}
-DTribitsExProj_ENABLE_SECONDARY_TESTED_CODE=ON
-DTribitsExProj_ENABLE_MixedLang=ON
-DTribitsExProj_ENABLE_TESTS=ON
-DTribitsExProj_ENABLE_ALL_FORWARD_DEP_PACKAGES=ON
-DTribitsExProj_DUMP_PACKAGE_DEPENDENCIES=ON
-DTribitsExProj_DUMP_FORWARD_PACKAGE_DEPENDENCIES=ON
${${PROJECT_NAME}_TRIBITS_DIR}/examples/TribitsExampleProject
PASS_REGULAR_EXPRESSION_ALL
"Sweep backward enabling all forward test dependent packages because TribitsExProj_ENABLE_ALL_FORWARD_DEP_PACKAGES=ON"
"-- Setting TribitsExProj_ENABLE_WithSubpackagesB=ON because TribitsExProj_ENABLE_MixedLang=ON"
"Enabling all tests and/or examples that have not been explicitly disabled because TribitsExProj_ENABLE_.TESTS,EXAMPLES.=ON"
"-- Setting MixedLang_ENABLE_TESTS=ON"
"-- Setting MixedLang_ENABLE_EXAMPLES=ON"
"-- Setting WithSubpackagesB_ENABLE_TESTS=ON"
"-- Setting WithSubpackagesB_ENABLE_EXAMPLES=ON"
"-- WithSubpackagesB_TEST_ENABLED_DEPENDENCIES: MixedLang"
"-- WithSubpackagesB_TEST_ALL_DEPENDENCIES: MixedLang"
"Final set of enabled packages: SimpleCxx MixedLang WithSubpackages 3"
"Final set of enabled SE packages: SimpleCxx MixedLang WithSubpackagesA WithSubpackagesB WithSubpackages 5"
"Processing enabled package: MixedLang [(]Libs, Tests, Examples[)]"
"Processing enabled package: WithSubpackages [(]A, B, Tests, Examples[)]"
"Configuring done"
"Generating done"
ALWAYS_FAIL_ON_NONZERO_RETURN

TEST_1 CMND make ARGS ${CTEST_BUILD_FLAGS}

TEST_2 CMND ${CMAKE_CTEST_COMMAND}
PASS_REGULAR_EXPRESSION_ALL
"MixedLang_RayTracerTests.* [.]* *Passed"
"WithSubpackagesB_test_of_b [.]* *Passed"
"WithSubpackagesB_test_of_b_mixed_lang.* [.]* *Passed"
"100% tests passed, 0 tests failed out of 3"
)
# NOTE: The above test covers one of the failure modes reported in
# TriBITSPub/TriBITS#510 that resulted in missing test dependencies.


########################################################################


Expand Down
18 changes: 17 additions & 1 deletion tribits/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,25 @@
ChangeLog for TriBITS
----------------------------------------


## 2022-08-11:

* **Changed:** Fix a bug where the test dependencies for an enabled
subpackage that resulted in a build error (see
[trilinos/Trilinos#10842](https://github.com/trilinos/Trilinos/issues/10842)
and
[TriBITSPub/TriBITS#510](https://github.com/TriBITSPub/TriBITS/issues/510)).

* **Changed:** Made setting parent package tests/examples enables correctly
propagate down to subpackages in a more intuitive way (see
[TriBITSPub/TriBITS#268](https://github.com/TriBITSPub/TriBITS/issues/268)).
This also results in not enabling tests for subpackages that are not
explicitly enabled or enabled as part of the forward sweep of packages
enables due to `<Project>_ENABLE_ALL_FORWARD_DEP_PACKAGES=ON`.

## 2022-07-20:

* ** Changed:** Fixed TriBITS generated and installed `<tplName>Config.cmake`
* **Changed:** Fixed TriBITS generated and installed `<tplName>Config.cmake`
files to not point into the build dir but instead point into relative dir
for upstream TPL's when calling find_dependency() (see
[TribitsPub/TriBITS#500](https://github.com/TriBITSPub/TriBITS/issues/500)).
Expand Down
13 changes: 3 additions & 10 deletions tribits/core/package_arch/TribitsAddTestHelpers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,21 +204,14 @@ endfunction()


# Determine if to add the test based on if testing is enabled for the current
# package or subpackage.
# package.
#
function(tribits_add_test_process_enable_tests ADD_THE_TEST_OUT)
if(${PACKAGE_NAME}_ENABLE_TESTS OR ${PARENT_PACKAGE_NAME}_ENABLE_TESTS)
if(${PACKAGE_NAME}_ENABLE_TESTS)
set(ADD_THE_TEST TRUE)
else()
if (PARENT_PACKAGE_NAME STREQUAL PACKAGE_NAME)
set(PARENT_EANBLE_TESTS_DISABLE_MSG)
else()
set(PARENT_EANBLE_TESTS_DISABLE_MSG
", ${PARENT_PACKAGE_NAME}_ENABLE_TESTS='${${PARENT_PACKAGE_NAME}_ENABLE_TESTS}'"
)
endif()
message_wrapper(
"-- ${TEST_NAME}: NOT added test because ${PACKAGE_NAME}_ENABLE_TESTS='${${PACKAGE_NAME}_ENABLE_TESTS}${PARENT_EANBLE_TESTS_DISABLE_MSG}'."
"-- ${TEST_NAME}: NOT added test because ${PACKAGE_NAME}_ENABLE_TESTS='${${PACKAGE_NAME}_ENABLE_TESTS}'."
)
set(ADD_THE_TEST FALSE)
endif()
Expand Down
81 changes: 65 additions & 16 deletions tribits/core/package_arch/TribitsAdjustPackageEnables.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,14 +999,59 @@ macro(tribits_apply_test_example_enables PACKAGE_NAME)
endmacro()


# Macro to set ${TRIBITS_SUBPACKAGE)_ENABLE_TESTS and
# ${TRIBITS_SUBPACKAGE)_ENABLE_EXAMPLES based on
# ${TRIBITS_PARENTPACKAGE)_ENABLE_TESTS or
# ${TRIBITS_PARENTPACKAGE)_ENABLE_EXAMPLES
macro(tribits_apply_subpackage_tests_examples_enables PARENT_PACKAGE_NAME)
if ("${${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES}" STREQUAL ""
AND ${PARENT_PACKAGE_NAME}_ENABLE_TESTS
)
message("-- " "Setting"
" ${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES=${${PARENT_PACKAGE_NAME}_ENABLE_TESTS}"
" because"
" ${PARENT_PACKAGE_NAME}_ENABLE_TESTS=${${PARENT_PACKAGE_NAME}_ENABLE_TESTS}")
set(${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES ${${PARENT_PACKAGE_NAME}_ENABLE_TESTS})
endif()
foreach(spkg IN LISTS ${PARENT_PACKAGE_NAME}_SUBPACKAGES)
set(fullSpkgName ${PARENT_PACKAGE_NAME}${spkg})
if (${PROJECT_NAME}_ENABLE_${fullSpkgName})
if (${PARENT_PACKAGE_NAME}_ENABLE_TESTS)
if ("${${fullSpkgName}_ENABLE_TESTS}" STREQUAL "")
message("-- " "Setting"
" ${fullSpkgName}_ENABLE_TESTS=${${PARENT_PACKAGE_NAME}_ENABLE_TESTS}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how picky you are about whitespace, but you've got some stray tabs here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Initial tabs are a no-no. Not sure how that got inserted (my emacs editor should be setup to indent with spaces only). I will diffidently fix that.

Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

This is addressed in new PR #515. I found a bunch of other places where tabs somehow got inserted. I wish there was a simple linter that could catch this. (Actually, to look for initial tabs in *.cmake files, then that is easy. I can do that with a simple bash scripts.)

" because parent package"
" ${PARENT_PACKAGE_NAME}_ENABLE_TESTS"
"=${${PARENT_PACKAGE_NAME}_ENABLE_TESTS}")
set(${fullSpkgName}_ENABLE_TESTS ${${PARENT_PACKAGE_NAME}_ENABLE_TESTS})
endif()
endif()
if (${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES)
if ("${${fullSpkgName}_ENABLE_EXAMPLES}" STREQUAL "")
message("-- " "Setting"
" ${fullSpkgName}_ENABLE_EXAMPLES=${${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES}"
" because parent package"
" ${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES"
"=${${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES}")
set(${fullSpkgName}_ENABLE_EXAMPLES ${${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES})
endif()
endif()
endif()
endforeach()
endmacro()
# NOTE: Above, the parent package may not actually be enabled yet
# (i.e. ${PROJECT_NAME}_ENABLE_${PARENT_PACKAGE_NAME} my not be TRUE) if only
# subpackages needed to be enabled in the forward sweep but we want the tests
# and examples for subpackage to be enabled if
# ${PARENT_PACKAGE_NAME}_ENABLE_TESTS=ON or just examples i
# f${PARENT_PACKAGE_NAME}_ENABLE_EXAMPLES=ON


macro(tribits_private_enable_forward_package FORWARD_DEP_PACKAGE_NAME PACKAGE_NAME)
tribits_implicit_package_enable_is_allowed( "" ${FORWARD_DEP_PACKAGE_NAME}
ALLOW_PACKAGE_ENABLE )
#message("TRIBITS_PRIVATE_ENABLE_FORWARD_PACKAGE: "
# "${FORWARD_DEP_PACKAGE_NAME} ${PACKAGE_NAME} ${ALLOW_PACKAGE_ENABLE}")
# Enable the forward package if it is not already set to ON or OFF
assert_defined(${PROJECT_NAME}_ENABLE_${FORWARD_DEP_PACKAGE_NAME})
if(${PROJECT_NAME}_ENABLE_${FORWARD_DEP_PACKAGE_NAME} STREQUAL ""
if("${${PROJECT_NAME}_ENABLE_${FORWARD_DEP_PACKAGE_NAME}}" STREQUAL ""
AND ALLOW_PACKAGE_ENABLE
)
message("-- " "Setting ${PROJECT_NAME}_ENABLE_${FORWARD_DEP_PACKAGE_NAME}=ON"
Expand All @@ -1017,15 +1062,10 @@ macro(tribits_private_enable_forward_package FORWARD_DEP_PACKAGE_NAME PACKAGE_
endmacro()


# Macro used to set ${PROJECT_NAME}_ENABLE_${FWD_PACKAGE_NAME)=ON for all optional
# and required forward library dependencies of the package ${PACKAGE_NAME}
# Macro used to set ${PROJECT_NAME}_ENABLE_${FWD_PACKAGE_NAME)=ON for all
#
macro(tribits_enable_forward_lib_package_enables PACKAGE_NAME)

#message("\nPACKAGE_ARCH_ENABLE_FORWARD_PACKAGE_ENABLES ${PACKAGE_NAME}")
#print_var(${PROJECT_NAME}_ENABLE_${PACKAGE_NAME})

# Enable the forward packages if this package is enabled
assert_defined(${PROJECT_NAME}_ENABLE_${PACKAGE_NAME})
if (${PROJECT_NAME}_ENABLE_${PACKAGE_NAME})

Expand All @@ -1042,15 +1082,12 @@ macro(tribits_enable_forward_lib_package_enables PACKAGE_NAME)
endmacro()


# Macro used to set ${PROJECT_NAME}_ENABLE_${FWD_PACKAGE_NAME)=ON for all optional
# and required forward test/example dependencies of the package ${PACKAGE_NAME}
# Macro used to set ${PROJECT_NAME}_ENABLE_${FWD_PACKAGE_NAME)=ON for all
# optional and required forward test dependencies of the package
# ${PACKAGE_NAME}
#
macro(tribits_enable_forward_test_package_enables PACKAGE_NAME)

#message("\nPACKAGE_ARCH_ENABLE_FORWARD_PACKAGE_ENABLES ${PACKAGE_NAME}")
#message("-- " "${PROJECT_NAME}_ENABLE_${PACKAGE_NAME}=${${PROJECT_NAME}_ENABLE_${PACKAGE_NAME}}")

# Enable the forward packages if this package is enabled
assert_defined(${PROJECT_NAME}_ENABLE_${PACKAGE_NAME})
if (${PROJECT_NAME}_ENABLE_${PACKAGE_NAME})

Expand Down Expand Up @@ -1378,6 +1415,18 @@ macro(tribits_adjust_package_enables)
# packages so that we don't enable tests that don't need to be enabled based
# on the use of the option ${PROJECT_NAME}_ENABLE_ALL_FORWARD_DEP_PACKAGES.

message("")
message("Enabling subpackage tests/examples based on parent package tests/examples enables ...")
message("")
foreach(TRIBITS_PACKAGE ${${PROJECT_NAME}_PACKAGES})
tribits_apply_subpackage_tests_examples_enables(${TRIBITS_PACKAGE})
endforeach()
# NOTE: We want to apply this logic here instead of later after the backward
# sweep of package enables because we don't want to enable the
# tests/examples for a subpackage if it is not needed based on set of
# requested subpackages and packages to be enabled and the optional forward
# sweep of downstream packages.

#
# D) Sweep backwards and enable upstream required and optional SE packages
#
Expand Down
Loading