Add post-installation sanity test - #2389
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2389 +/- ##
=======================================
Coverage 82.28% 82.28%
=======================================
Files 125 125
Lines 20701 20701
=======================================
Hits 17033 17033
Misses 3668 3668 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I also noticed the existing |
spenke91
left a comment
There was a problem hiding this comment.
Thank you so much for this contribution, @tokito-99!👍
I like it a lot and only have a fewsuggestions and remarks. The only one not found in the inline comments is that I am not entirely convinced of having the post-install tests hidden in the doc folder (I know #2361 explicitly suggested that). What do you think about putting it in a sub-directory test/post_install?
In principal, I still like the idea of seeing it as an example on how to include t8code from your own cmake repo, but I'd argue the main purpose of the file is the post-install testing, so to me it just feels wrong in doc. In principal, the information on how to link your project to t8code is also given in the Wiki (https://github.com/DLR-AMR/t8code/wiki/Installation). If we feel (later) that we additionally want a folder doc/cmake_examples, of course we could also link / refer to the post-install tests, but for now the post-install tests suffice I would say.
What do you think? :-)
| include( CMakeFindDependencyMacro ) | ||
|
|
||
| set( T8CODE_ENABLE_MPI @T8CODE_ENABLE_MPI@ ) | ||
|
|
||
| if( T8CODE_ENABLE_MPI ) | ||
| find_dependency( MPI 3.0 COMPONENTS C ) | ||
| endif() |
There was a problem hiding this comment.
Could you explain why this change is necessary or how it is related to the psost-install tests?
| along with t8code; if not, write to the Free Software Foundation, Inc., | ||
| 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
| */ | ||
|
|
There was a problem hiding this comment.
| /** \file t8code_post_install_sanity.cxx | |
| * This file implements a sanity check that includes t8code from another target and calls some t8code functions. | |
| */ | |
|
|
||
| #include <t8.h> | ||
|
|
||
| int |
There was a problem hiding this comment.
| int | |
| /// Main function that tries some very basic t8code functionality for testing purposes. |
|
|
||
| set(T8CODE_POST_INSTALL_TEST_SOURCE_DIR ${CMAKE_INSTALL_FULL_DOCDIR}/cmake_examples/post_install_sanity) | ||
| set(T8CODE_POST_INSTALL_TEST_BINARY_DIR ${PROJECT_BINARY_DIR}/post_install_tests) | ||
| set(T8CODE_POST_INSTALL_TEST_CMAKE_ARGS | ||
| -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX} | ||
| ) | ||
|
|
||
| if(CMAKE_BUILD_TYPE) | ||
| list(APPEND T8CODE_POST_INSTALL_TEST_CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}) | ||
| endif() | ||
|
|
||
| if(DEFINED SC_DIR AND NOT "${SC_DIR}" STREQUAL "") | ||
| list(APPEND T8CODE_POST_INSTALL_TEST_CMAKE_ARGS -DSC_DIR=${SC_DIR}) | ||
| endif() | ||
|
|
||
| if(DEFINED P4EST_DIR AND NOT "${P4EST_DIR}" STREQUAL "") | ||
| list(APPEND T8CODE_POST_INSTALL_TEST_CMAKE_ARGS -DP4EST_DIR=${P4EST_DIR}) | ||
| endif() | ||
|
|
||
| install( DIRECTORY ${PROJECT_SOURCE_DIR}/doc/cmake_examples/post_install_sanity | ||
| DESTINATION ${CMAKE_INSTALL_DOCDIR}/cmake_examples | ||
| ) | ||
|
|
||
| add_custom_target( post_install_tests | ||
| COMMAND ${CMAKE_COMMAND} -E rm -rf ${T8CODE_POST_INSTALL_TEST_BINARY_DIR} | ||
| COMMAND ${CMAKE_COMMAND} -E make_directory ${T8CODE_POST_INSTALL_TEST_BINARY_DIR} | ||
| COMMAND ${CMAKE_COMMAND} | ||
| -G ${CMAKE_GENERATOR} | ||
| -S ${T8CODE_POST_INSTALL_TEST_SOURCE_DIR} | ||
| -B ${T8CODE_POST_INSTALL_TEST_BINARY_DIR} | ||
| ${T8CODE_POST_INSTALL_TEST_CMAKE_ARGS} | ||
| COMMAND ${CMAKE_COMMAND} --build ${T8CODE_POST_INSTALL_TEST_BINARY_DIR} | ||
| COMMAND ${CMAKE_CTEST_COMMAND} --test-dir ${T8CODE_POST_INSTALL_TEST_BINARY_DIR} --output-on-failure | ||
| COMMENT "Build and run the installed t8code post-installation sanity example" | ||
| VERBATIM | ||
| ) | ||
|
|
There was a problem hiding this comment.
I suggest to move everything related to the post-install tests to an own cmake file to keep the main t8code file clean.
Then we could simply say something like include(cmake/PostInstallTests.cmake) here.
Describe your changes here:
This PR adds a minimal post-installation sanity test for t8code.
It installs a small external CMake consumer project under
doc/cmake_examples/post_install_sanityand adds apost_install_teststarget. The target configures, builds, and runs the installed example out-of-tree against the installedT8CODEConfig.cmake.Closes #2361.
Changes
find_package(T8CODE CONFIG REQUIRED).post_install_teststarget.post_install_testsafterninja installin the main CMake CI workflow.T8CODEConfig.cmaketo find MPI when t8code was built with MPI support.Testing
Locally tested a reduced non-MPI install-tree workflow:
cmake -S . -B /tmp/t8code-2361-smoke \ -DCMAKE_INSTALL_PREFIX=/tmp/t8code-2361-install \ -DT8CODE_BUILD_TESTS=OFF \ -DT8CODE_BUILD_EXAMPLES=OFF \ -DT8CODE_BUILD_TUTORIALS=OFF \ -DT8CODE_BUILD_BENCHMARKS=OFF \ -DT8CODE_ENABLE_MPI=OFF cmake --build /tmp/t8code-2361-smoke --parallel 4 cmake --install /tmp/t8code-2361-smoke cmake --build /tmp/t8code-2361-smoke --target post_install_testsResult:
Also ran:
All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.All these boxes must be checked by the REVIEWERS before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
scripts/internal/find_all_source_files.shto check the indentation of these files.License
doc/(or already has one).