From cb4090def796ab4ecdc5aa099329c4b8534d40a5 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Wed, 12 Jul 2023 06:19:27 -0600 Subject: [PATCH] Update logic for enabling TrilinosInstallTests in CI testing (#12024) * Now TrilinosInstallTests will be enabled if any Trilinos package gets enabled. * Removed dependence of TrilinosInstallTests on Tpetra. This should give more consistent installation testing with Trilinos. Prior to this commit, only changes to Tpetra and packages upstream from Tpetra would trigger the enable of the TrilinosInstallTests package and the running of these tests. Now, any change to any Trilinos packge will trigger some installation testing. See new TrilinosInstallTests/READMME.md file for details. --- .../get-changed-trilinos-packages.sh | 14 ++++-- packages/TrilinosInstallTests/CMakeLists.txt | 11 ++++- packages/TrilinosInstallTests/README.md | 45 +++++++++++++++++++ .../cmake/Dependencies.cmake | 10 +---- 4 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 packages/TrilinosInstallTests/README.md diff --git a/commonTools/framework/get-changed-trilinos-packages.sh b/commonTools/framework/get-changed-trilinos-packages.sh index fc68df2d7f97..b67a897cc07f 100755 --- a/commonTools/framework/get-changed-trilinos-packages.sh +++ b/commonTools/framework/get-changed-trilinos-packages.sh @@ -176,13 +176,21 @@ CHANGED_PACKAGES_FULL_LIST=`$TRIBITS_DIR/ci_support/get-tribits-packages-from-fi echo "CHANGED_PACKAGES_FULL_LIST='$CHANGED_PACKAGES_FULL_LIST'" echo -echo "D) Filter list of changed packages to get only the PT packages" +echo "D) Filter list of changed packages" echo CHANGED_PACKAGES_ST_LIST=$(trilinos_filter_packages_to_test "${CHANGED_PACKAGES_FULL_LIST}") echo "CHANGED_PACKAGES_ST_LIST='${CHANGED_PACKAGES_ST_LIST}'" echo -echo "E) Generate the ${CMAKE_PACKAGE_ENABLES_OUT} enables file" +echo "E) Enable Trilinos installation tests if any Trilinos packages are changed" +echo +if [[ "${CHANGED_PACKAGES_ST_LIST}" != "" ]] ; then + export CHANGED_PACKAGES_ST_LIST=${CHANGED_PACKAGES_ST_LIST},TrilinosInstallTests +fi +echo "CHANGED_PACKAGES_ST_LIST='${CHANGED_PACKAGES_ST_LIST}'" + +echo +echo "F) Generate the ${CMAKE_PACKAGE_ENABLES_OUT} enables file" echo echo " @@ -204,7 +212,7 @@ fi echo "Wrote file '$CMAKE_PACKAGE_ENABLES_OUT'" echo -echo "F) Generate the ${CTEST_LABELS_FOR_SUBPROJETS_OUT} file" +echo "G) Generate the ${CTEST_LABELS_FOR_SUBPROJETS_OUT} file" echo printf "set(CTEST_LABELS_FOR_SUBPROJECTS" > $CTEST_LABELS_FOR_SUBPROJETS_OUT diff --git a/packages/TrilinosInstallTests/CMakeLists.txt b/packages/TrilinosInstallTests/CMakeLists.txt index 41fbf453a4fc..6ea4927123de 100644 --- a/packages/TrilinosInstallTests/CMakeLists.txt +++ b/packages/TrilinosInstallTests/CMakeLists.txt @@ -87,6 +87,7 @@ tribits_add_advanced_test(reduced_tarball OVERALL_WORKING_DIRECTORY TEST_NAME HOSTTYPE Linux EXCLUDE_IF_NOT_TRUE ${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE + ${CMAKE_PROJECT_NAME}_ENABLE_Kokkos TEST_0 MESSAGE "Create the reduced tarball for the enabled packages" @@ -123,12 +124,18 @@ tribits_add_advanced_test(reduced_tarball ) # NOTE: The above test makes sure that you can create the reduced tarball for -# the given set of enabled packages. We only add the test of the -# configuration options are passed through a +# the given set of enabled packages. +# +# NOTE: We only add the test if the configuration options are passed through a # ${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE file. (Does not work if you to # include them with -C .cmake. You can only pass through # configure options through ${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE since # the CMake project knows these.) +# +# NOTE: Since the configuration of Trilinos requires the existence of some +# files under packages/kokkos/ so we don't try to bother creating a reduced +# Trilinos tarball when Kokkos is not enabled. Otherwise, the configuration +# of the untarred Trilinos directory will fail since it is missing Kokkos. ################################################################################ diff --git a/packages/TrilinosInstallTests/README.md b/packages/TrilinosInstallTests/README.md new file mode 100644 index 000000000000..29e56f6c2276 --- /dev/null +++ b/packages/TrilinosInstallTests/README.md @@ -0,0 +1,45 @@ +# TrilinosInstallTests package + +The purpose of the TrilinosInstallTests package is to test the installation of +Trilinos and related functionalities such as reduced tarball creation and +generated and installed `Config.cmake` files. It also builds and +tests the demo project `simpleBuildAgainstTrilinos` which depends on the +package Tpetra. + +## The role of the TrilinosInstallTests package in CI Testing + +The role of the package `TrilinosInstallTests` in Trilinos CI testing is a bit +tricky and the details are described below. + +Firs, the script `get-changed-trilinos-packages.sh` is set up to enable the +`TrilinosInstallTests` package if **any** Trilinos package is changed. The +tests `TrilinosInstallTests_doinstall` and +`TrilinosInstallTests_find_package_Trilinos` are always run no matter what +Trilinos packages are enabled (and will pass even if no other Trilinos +packages are enabled). This ensures that any changed package will have some +aspect of its configuration tested; that is, its install and its generated and +installed `Config.cmake` file are tested. + +If the Tpetra package is enabled in the CI build (e.g. as a consequence of +setting `Trilinos_ENABLE_ALL_FORWARD_DEP_PACKAGES=ON`), then the +`TrilinosInstallTests` package and the tests based on +`simpleBuildAgainstTrilinos` will be enabled. This ensures that any changes +to Tpetra or its upstream dependencies will trigger the testing with +`simpleBuildAgainstTrilinos` (and thereby provide some installation testing +for Tpetra and its upstream dependencies such as Kokkos.) + +However, if the directory `demos/simpleBuildAgainstTrilinos` is changed and +Tpetra is not enabled, then this will **not** trigger the enable of the tests +based on `simpleBuildAgainstTrilinos`. This is a gap in the CI testing +process and therefore testing for changes to `simpleBuildAgainstTrilinos` must +be done locally. + +Also, based on the above-described logic, not that if a package downstream +from Tpetra is changed, then this will trigger the enable of the tests based +on `simpleBuildAgainstTrilinos`, even though changing such a package can not +break `simpleBuildAgainstTrilinos` or the tests based on it. But the CMake +project `simpleBuildAgainstTrilinos` configures, builds, and runs its tests +very quickly, so this should not negatively impact the CI testing process. + +See more detailed comments in the file `./CMakeLists.txt` where the tests are +defined. diff --git a/packages/TrilinosInstallTests/cmake/Dependencies.cmake b/packages/TrilinosInstallTests/cmake/Dependencies.cmake index 7193e8d62892..98863330df97 100644 --- a/packages/TrilinosInstallTests/cmake/Dependencies.cmake +++ b/packages/TrilinosInstallTests/cmake/Dependencies.cmake @@ -1,8 +1,2 @@ -tribits_package_define_dependencies( - LIB_OPTIONAL_PACKAGES Tpetra) -# NOTE: By declaring a dependence on Tpetra, if that package or any package -# upstream from it changes, then it will trigger the enable of this package -# and the running of installation tests and the demo example. But since -# TrilinosInstallTests does not have any libs or execs of its own (all of its -# work is done in tests), there is no link-time dependency here in the build -# dir. +tribits_package_define_dependencies() +# See discussion of the CI testing process in the file ./../README.md