add post-installation test (answer issue #2361) by michelemartone · Pull Request #2362 · DLR-AMR/t8code

michelemartone · 2026-07-06T12:16:24Z

Address issue #2361 by adding a post-installation test goal (post_install_tests).

This works by:

generating a top-level CMakeLists.txt file with hardcoded paths reflecting the installation directories and installing it in a directory with documentation
installing two simple CMakeLists.txt files meant as t8code use examples; the two are located in respective sub-directories, both containing the same sample t8code program (the first tutorial program actually)
ensuring that the top-level CMakeLists.txt file is be usable from anywhere without further configuration, and uses the two CMakeLists.txt files to build two executables and run them (via the tests target)
having a post_install_tests target, to build the above two programs in the temporary post_install_tests_dir directory for test purposes
assuming the post_install_tests target to only work after install
keeping the test to be basic - the goal is to have detection of the t8code library to work, as well as build and execution of the two test programs

Current shortcomings:

test limited to Linux
not documented anywhere ( ideal would be in this files' tree, but current t8code practice uses t8code.wiki, which is separate )
it may assume that the two MPI-enabled executables can be ran straight-ahead
installation happens in a documentation directory, but to be enabled, this feature relies on T8CODE_BUILD_TUTORIALS, not T8CODE_BUILD_DOCUMENTATION

p.s.: Since this is my first contribution to t8code, I'm including my
authorization to treat this contribution under the "FreeBSD License"
and I'm signing (at least) this commit with my usual GPG key.

Describe your changes here:

All these boxes must be checked by the AUTHOR before requesting review:

The PR is small enough to be reviewed easily. If not, consider splitting up the changes in multiple PRs.
The title starts with one of the following prefixes: Documentation:, Bugfix:, Feature:, Improvement: or Other:.
If the PR is related to an issue, make sure to link it.
The author made sure that, as a reviewer, he/she would check all boxes below.

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

The reviewer executed the new code features at least once and checked the results manually.
The code follows the t8code coding guidelines.
New source/header files are properly added to the CMake files.
The code is well documented. In particular, all function declarations, structs/classes and their members have a proper doxygen documentation. Make sure to add a file documentation for each file!
README.md files are updated if necessary.
All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue).

Tests

The code is covered in an existing or new test case using Google Test.
The code coverage of the project (reported in the CI) should not decrease. If coverage is decreased, make sure that this is reasonable and acceptable.
Valgrind doesn't find any bugs in the new code. This script can be used to check for errors; see also this wiki article.

If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):

Should this use case be added to the github action?
If not, does the specific use case compile and all tests pass (check manually).

Scripts and Wiki

If a new directory with source files is added, it must be covered by the scripts/internal/find_all_source_files.sh to check the indentation of these files.
If this PR introduces a new feature, it must be covered in an example or tutorial and a Wiki article.

License

The author added a BSD statement to doc/ (or already has one).

Address issue #2361 by adding a post-installation test goal (`post_install_tests`). This works by: - generating a top-level CMakeLists.txt file with hardcoded paths reflecting the installation directories and installing it in a directory with documentation - installing two simple CMakeLists.txt files meant as t8code use examples; the two are located in respective sub-directories, both containing the same sample t8code program (the first tutorial program actually) - ensuring that the top-level CMakeLists.txt file is be usable from anywhere without further configuration, and uses the two CMakeLists.txt files to build two executables and run them (via the `tests` target) - having a `post_install_tests` target, to build the above two programs in the temporary `post_install_tests_dir` directory for test purposes - assuming the `post_install_tests` target to only work after `install` - keeping the test to be basic - the goal is to have detection of the t8code library to work, as well as build and execution of the two test programs Current shortcomings: - test limited to Linux - not documented anywhere ( ideal would be in this files' tree, but current t8code practice uses t8code.wiki, which is separate ) - it may assume that the two MPI-enabled executables can be ran straight-ahead - installation happens in a documentation directory, but to be enabled, this feature relies on T8CODE_BUILD_TUTORIALS, not T8CODE_BUILD_DOCUMENTATION p.s.: Since this is my first contribution to t8code, I'm including my authorization to treat this contribution under the "FreeBSD License" and I'm signing (at least) this commit with my usual GPG key.

codecov · 2026-07-06T12:34:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.30%. Comparing base (e532b09) to head (7d1a135).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2362   +/-   ##
=======================================
  Coverage   82.30%   82.30%           
=======================================
  Files         125      125           
  Lines       20559    20559           
=======================================
  Hits        16922    16922           
  Misses       3637     3637

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sandro-elsweijer

Thanks for the contribution, this is a good and useful addition to our testing pipeline, especially for testing the validity of the installed packages.

For merging, we’ll need it a bit more aligned with our coding guidelines: clearer structure (one responsibility per folder), more descriptive naming instead of single-letter identifiers, and a more CMake-native approach instead of shell/system-specific assumptions. Also, hardcoding should really be avoided unless there’s no reasonable alternative.

With those adjustments, this looks ready to go.

sandro-elsweijer · 2026-07-06T12:50:20Z

+cmake_minimum_required(VERSION 3.22.0)
+project(t8test DESCRIPTION "t8code test program (requires CMAKE_PREFIX_PATH to point to T8CODE's package files are)" LANGUAGES CXX)
+find_package(T8CODE)
+add_executable(test_a test.cxx)


Can you please avoid using single letter file names, variable names, etc.?

sandro-elsweijer · 2026-07-06T12:51:40Z

+endif()
+find_path(SC_INCLUDE_DIR sc.h REQUIRED)
+find_path(T8CODE_INCLUDE_DIR t8.h REQUIRED)
+add_executable(test_b test.cxx)


It'd be good to have the directories under doc/cmake_examples easily enumerable via globbing from elsewhere -- i.e. with no need to know their identifier in other places. The current scheme was "single letter". I could introduce a prefix (e.g. example_mini and example_midi); that would be an epsilon more informative (not worth the cognitive effort IMO) but would be introducing redundancy (which I'm striving to reduce). Would you still prefer that way (say example_mini and example_midi) ?

What about post_installation_check_1 and post_installation_check_2?
There is nothing against using enumerations of any sort, but test_b and test.cxx are not really descriptive and it could mean anything.

sandro-elsweijer · 2026-07-06T12:55:44Z

    endif( T8CODE_EXPORT_COMPILE_COMMANDS )

    install( TARGETS ${ADD_T8_TUTORIAL_NAME} RUNTIME )
+    install( FILES ${ADD_T8_TUTORIAL_SOURCES} DESTINATION ${CMAKE_INSTALL_DOCDIR}/tutorials/general/ )


Can you please add a new configuration argument to do this and place this in its own cmake file inside the test folder? We want to keep all directories true to their purpose and the tutorial folder is just for tutorials. Same for the documentation folder.

Why are you suggesting a new configuration variable? test/CMakeLists.txt depends on T8CODE_BUILD_TESTS, which is an already existing configuration variable.

I think he meant that it should not be installed in ${ADD_T8_TUTORIAL_SOURCES}.

No, I meant that this is a post installation test and that it does not have anything to do with the tutorials. Yes, it uses a tutorial source file as test subject, but as a user i would not expect a post installation test in the tutorial folder. I would expect this to be in the test folder.

Also, I would not expect this post installation test to be dependent on the T8CODE_BUILD_TUTORIALS option since, as i already explained, it is a test.

sandro-elsweijer · 2026-07-06T12:56:37Z

+# notice that in the following, we install files from doc/ and tutorials/ together:
+file(GLOB T8CODE_CMAKE_EXAMPLES RELATIVE "${PROJECT_SOURCE_DIR}/doc/cmake_examples/" "${PROJECT_SOURCE_DIR}/doc/cmake_examples/?") # for each single-character directory; notice there's no explicit listing here
+configure_file( ${PROJECT_SOURCE_DIR}/doc/cmake_examples/CMakeLists.txt.in ${CMAKE_BINARY_DIR}/doc/cmake_examples/CMakeLists.txt @ONLY)
+install(FILES ${CMAKE_BINARY_DIR}/doc/cmake_examples/CMakeLists.txt DESTINATION ${CMAKE_INSTALL_DOCDIR}/cmake_examples/ RENAME CMakeLists.txt) # reminder that we may want to name this differently (yet, no necessity for .in)
+foreach(_dir IN LISTS T8CODE_CMAKE_EXAMPLES)
+     install(DIRECTORY ${PROJECT_SOURCE_DIR}/doc/cmake_examples/${_dir} DESTINATION ${CMAKE_INSTALL_DOCDIR}/cmake_examples/) # notice we take directories from doc...
+     install(FILES ${PROJECT_SOURCE_DIR}/tutorials/general/t8_step0_helloworld.cxx DESTINATION ${CMAKE_INSTALL_DOCDIR}/cmake_examples/${_dir}/ RENAME test.cxx) #... and add the same sample tutorial program as a test
+endforeach()


You can happily re-use the tutorial files as test source code, but the actual testing should stay outside the tutorial folder

I don't understand your comment. Testing happens anyway in a post_install_tests_dir directory.

I meant, that the main purpose of this whole PR is to introduce a software test. Therefore, everything should be located in the test folder. The PR is not meant as documentation and also not as a tutorial. Yes, users could look at this to inspire them, but the main purpose is still that this is a software test.

sandro-elsweijer · 2026-07-06T13:25:21Z

+    if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) # post-install tests currently limited to our main development OS
+        add_custom_target(post_install_tests # notice that add_test() would not be appropriate for this test
+            COMMAND rm -fR post_install_tests_dir # we perform the test out of this cmake tree
+            COMMAND mkdir post_install_tests_dir
+            COMMAND cd post_install_tests_dir && cmake ${CMAKE_INSTALL_PREFIX}/share/doc/T8CODE/cmake_examples/ && make VERBOSE=1 && make VERBOSE=1 tests
+            COMMAND rm -fR post_install_tests_dir
+            WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+            VERBATIM
+            COMMENT "Post-install tests' target. It creates, uses, and deletes temporary directory 'post_install_tests_dir'. It assumes running under Linux."
+        )
+    endif()


I tried to translate this to cmake commands as good as possible. This way cmake peserves the used generator, build type, etc. and it is also OS agnostic.
The BINARY_DIR could be extracted from the (PROJECT_BINARY_DIR)[PROJECT_BINARY_DIR] cmake variable and passed to the test in the CMakeLists calling this build script.

Suggested change

if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) # post-install tests currently limited to our main development OS

add_custom_target(post_install_tests # notice that add_test() would not be appropriate for this test

COMMAND rm -fR post_install_tests_dir # we perform the test out of this cmake tree

COMMAND mkdir post_install_tests_dir

COMMAND cd post_install_tests_dir && cmake ${CMAKE_INSTALL_PREFIX}/share/doc/T8CODE/cmake_examples/ && make VERBOSE=1 && make VERBOSE=1 tests

COMMAND rm -fR post_install_tests_dir

WORKING_DIRECTORY ${PROJECT_BINARY_DIR}

VERBATIM

COMMENT "Post-install tests' target. It creates, uses, and deletes temporary directory 'post_install_tests_dir'. It assumes running under Linux."

)

endif()

# Remove any previous build directory.

file(REMOVE_RECURSE "${BINARY_DIR}/post_install_tests")

file(MAKE_DIRECTORY "${BINARY_DIR}")

set(EXAMPLE_DIR

"${INSTALL_PREFIX}/path_to_dir"

)

# Run configure step

execute_process(

COMMAND

${CMAKE_COMMAND}

-S "${EXAMPLE_DIR}"

-B "${BINARY_DIR/post_install_tests}"

RESULT_VARIABLE result

)

if(NOT result EQUAL 0)

message(FATAL_ERROR "Failed to configure installed example.")

endif()

# Run build step

execute_process(

COMMAND

${CMAKE_COMMAND}

--build "${BINARY_DIR/post_install_tests}"

RESULT_VARIABLE result

)

if(NOT result EQUAL 0)

message(FATAL_ERROR "Failed to build installed example.")

endif()

# Run tests

execute_process(

COMMAND

${CMAKE_COMMAND}

--build "${BINARY_DIR/post_install_tests}"

--target tests

RESULT_VARIABLE result

)

if(NOT result EQUAL 0)

message(FATAL_ERROR "Installed example tests failed.")

endif()

file(REMOVE_RECURSE "${BINARY_DIR/post_install_tests}")

You removed the post_install_tests target along the way. We want to have that.

Ah sorry, I added it back

michelemartone requested a review from spenke91 July 6, 2026 12:35

sandro-elsweijer requested changes Jul 6, 2026

View reviewed changes

sandro-elsweijer assigned michelemartone Jul 6, 2026

-    if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) # post-install tests currently limited to our main development OS
-        add_custom_target(post_install_tests # notice that add_test() would not be appropriate for this test
-            COMMAND rm -fR post_install_tests_dir # we perform the test out of this cmake tree
-            COMMAND mkdir post_install_tests_dir
-            COMMAND cd post_install_tests_dir && cmake ${CMAKE_INSTALL_PREFIX}/share/doc/T8CODE/cmake_examples/ && make VERBOSE=1 && make VERBOSE=1 tests
-            COMMAND rm -fR post_install_tests_dir
-            WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
-            VERBATIM
-            COMMENT "Post-install tests' target. It creates, uses, and deletes temporary directory 'post_install_tests_dir'. It assumes running under Linux."
-        )
-    endif()
+# Remove any previous build directory.
+file(REMOVE_RECURSE "${BINARY_DIR}/post_install_tests")
+file(MAKE_DIRECTORY "${BINARY_DIR}")
+set(EXAMPLE_DIR
+    "${INSTALL_PREFIX}/path_to_dir"
+)
+# Run configure step
+execute_process(
+    COMMAND
+        ${CMAKE_COMMAND}
+        -S "${EXAMPLE_DIR}"
+        -B "${BINARY_DIR/post_install_tests}"
+    RESULT_VARIABLE result
+)
+if(NOT result EQUAL 0)
+    message(FATAL_ERROR "Failed to configure installed example.")
+endif()
+# Run  build step
+execute_process(
+    COMMAND
+        ${CMAKE_COMMAND}
+        --build "${BINARY_DIR/post_install_tests}"
+    RESULT_VARIABLE result
+)
+if(NOT result EQUAL 0)
+    message(FATAL_ERROR "Failed to build installed example.")
+endif()
+# Run tests
+execute_process(
+    COMMAND
+        ${CMAKE_COMMAND}
+        --build "${BINARY_DIR/post_install_tests}"
+        --target tests
+    RESULT_VARIABLE result
+)
+if(NOT result EQUAL 0)
+    message(FATAL_ERROR "Installed example tests failed.")
+endif()
+file(REMOVE_RECURSE "${BINARY_DIR/post_install_tests}")

Uh oh!

Conversation

michelemartone commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

General

Tests

Scripts and Wiki

License

Uh oh!

codecov Bot commented Jul 6, 2026

Codecov Report

Uh oh!

sandro-elsweijer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandro-elsweijer Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandro-elsweijer Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandro-elsweijer Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

michelemartone commented Jul 6, 2026 •

edited

Loading

sandro-elsweijer Jul 14, 2026 •

edited

Loading

sandro-elsweijer Jul 6, 2026 •

edited

Loading

sandro-elsweijer Jul 14, 2026 •

edited

Loading