Skip to content

Draft: Remove add_dune_all_flags from dune_add_test

Simon Praetorius requested to merge issue/remove-add-all-flags-from-tests into master

Summary

In the current master, the dune_add_test macro adds for the targets all registered flags (i.e. links all required and optional packages). This is almost always not necessary. Mostly we want to test a specific feature. If this feature depends on an optional package, it makes more sense to make this explicit, such that the test itself documents what it needs. This MR removes this add_dune_all_flags call an the created target and adds flags manually.

Details

The idea is to follow the same strategy for tests as for executables. You have to choose what you need and link it. While every required dependency is automatically linked, optional dependencies must be spelled out. If tests do not do this, it is not clear what they actually depend on. It cannot be inspected by looking at the added flags, since silently all found packages and all somewhere registered flags, whether they are relevant to this test or not, are added to all test targets. This could also lead in the future to clashes in the compile flags (and has already done in the past), e.g. if two external optional packages set the same flag, but these external packages are defined in different modules. If we cannot choose which of these packages we actually want, then we are stuck and have to go back to low-level add_test of cmake.

Another aspect: tests have (at least) two meanings, 1. they check that functions and classes are implemented correctly, 2. they show the user how to use these functions and classes. Thus it is also a documentation how to use dune. But if the dune_add_test macro sets silently flags that are needed for a specific tests and thus for a specific feature of the library but without documentation to the user, the second goal of tests is failed. With this MR, we go a little bit into a more expressive direction.

How to write/configure tests

If you write a test of an (optional) feature A, you already have to either check for existence of the feature by HAVE_A in the source file, or explicitly require it by CMAKE_GUARD HAVE_A in the dune_add_test. It is thus just one more step to say: If I require that feature and I check in my source that the feature exists, then I also need to activate that feature, otherwise it cannot be available. This is done by dune_add_A_flags(target).

Example

Let's write a test testA that requires an optional feature A and may additionally use a feature B if it is available, but works perfectly fine without it. Then we write in the corresponding CMAkeLists.txt file

dune_add_test(NAME testA SOURCES testA.cc 
              CMAKE_GUARD HAVE_A)
add_dune_A_flags(testA)
add_dune_B_flags(testA) # not required but can be used if found

and in the source file

#include <config.h>

#if !HAVE_A
#error "Require feature A"
#endif

int main() {
#if HAVE_B
  test_with_feature_B();
#else 
  test_with_feature_A();
#endif
}

Issues

Currently, we cannot filter registered flags or dune modules, e.g., we cannot link only against the module library only or against required dune module dependencies. We can only link against all found dune module libraries. This is (partially) resolved by !944 (closed).

Dependencies

TODO

  • Check downstream modules where tests fail due to missing optional packages
  • Merge !1263
  • Add changelog entry
Edited by Simon Praetorius

Merge request reports