Install tests/tests.h

[peterrum]

Many projects create a copy of this file. Why don't we simply install the file so that it can be accessed via #include <deal.II/tests/tests.h>?

FYI @slfuchs @blaisb

[bangerth]

Are we ready to guarantee backward compatibility of this file?

Separately, though, this file has this line:

using namespace dealii;

I am against having this in any installed file.

[masterleinad]

I am slightly against doing this. If there is functionality in it that people are using (and we want to export) we should just move these things somewhere else. We don't expose any other part of the test suite in an installation AFAICT (and we should not IMHO).

[blaisb]

It sure is useful for auxiliary projects like ours to be able to use this file. However, keeping a copy is not really troublesome either...
I don't have a good perspective of what this implies so I'll let you decide :).

[tamiko]

I am worried about this conditional: DEAL_II_HAVE_TESTS_DIRECTORY is false when the tests directory is not present. If you happen to now build and install deal.II without the tests directory you will end up not installing this header file and potentially break downstream projects.

What about we identify the useful and stable part of this header and simply migrate it to includes?

[peterrum]

I can live with that as long as I don't have too see another project that simply copies the test.h file because they want to use the deal.II test infrastructure but crucial functions are missing.

[peterrum]

I have move the classes related to logging into the new file. We can add additional functions here depending on the feedback of users.

[slfuchs]

@peterrum: I think in addition the header <fstream> and the definition void cat_file(const char *filename) need to be moved into this file. Otherwise, one gets a build error as cat_file is called in struct MPILogInitAll.

[peterrum]

Thanks for the feedback! I'll add it!

[bangerth]

I'm still not entirely convinced that putting things into the library is the right thing to do. Among the reasons is that application programs or tutorials will not use these things -- it's just stuff that's used for testing, and so my take is that it should be part of the testing infrastructure, not the library proper. But if the majority is in favor, I'm also not going to stand in the way.

Regardless, things will have to move out of the dealii namespace. Put them into a nested namespace instead, and then you can put a using namespace ... declaration into tests/tests.h.

[bangerth]

This is putting things into the global dealii namespace that I don't think should be there. If you must, we could have a nested namespace dealii::Testing where all of this could go.

[bangerth]

What do we do with this? I continue to think that installing tests.h is wrong, and there doesn't seem to be much enthusiasm about the idea from others either...

[peterrum]

What do we do with this? I continue to think that installing tests.h is wrong, and there doesn't seem to be much enthusiasm about the idea from others either...

Let me again explain the motivation for this PR. We already allow users to use our testing infrastructure in their own projects, most notably with DEAL_II_PICKUP_TESTS(). This is done in multiple projects and it works quite well. If users use the deal.II testing infrastructure, it makes sense that they want to write test similarly as in deal.II (MPILogInitAll, ...). What users are doing for this goal at the moment, they simply copy tests/test.h in their projects (e.g., https://github.com/lethe-cfd/lethe/blob/master/tests/tests.h, https://github.com/PartExa/PartExa/blob/master/tests/tests.h, https://github.com/peterrum/hyperdeal/blob/master/tests/tests.h - the last project is my own but I guess you get my point), since there is no alternative. Personally, I find this not an optimal state. We should not encourage/force users to copy these files, but give them a structured way to access the most useful testing utility functions. What is the purpose of letting users use the testing infrastructure, but not allowing to use the functions!?

[jppelteret]

FWIW, I don't copy the tests.h header into my projects, but I do find it and include it when I build tests.

##
# Configure the test / benchmark environment
##
IF(BUILD_TESTS OR BUILD_BENCHMARKS)
  MESSAGE(STATUS "CTest environment enabled")
  ENABLE_TESTING()
  INCLUDE(CTest)

  FIND_FILE(DEAL_II_TESTS_H tests.h
    HINTS ${deal.II_SOURCE_DIR}/tests ${DEAL_II_SOURCE_DIR} $ENV{DEAL_II_SOURCE_DIR}
    PATH_SUFFIXES tests
    NO_DEFAULT_PATH NO_CMAKE_ENVIRONMENT_PATH NO_CMAKE_PATH
    NO_SYSTEM_ENVIRONMENT_PATH NO_CMAKE_SYSTEM_PATH NO_CMAKE_FIND_ROOT_PATH
  )

  IF(EXISTS ${DEAL_II_TESTS_H})
    MESSAGE(STATUS "Path to deal.II test header: ${DEAL_II_TESTS_H}")
  ELSE()
    MESSAGE(STATUS "Tests could not be enabled: deal.II's test.h file could not be found.")
  ENDIF()

  IF(BUILD_TESTS AND EXISTS ${DEAL_II_TESTS_H})
    MESSAGE(STATUS "Tests enabled")
    INCLUDE_DIRECTORIES(${DEAL_II_SOURCE_DIR}/tests)
    ADD_SUBDIRECTORY(tests)
  ENDIF()

  IF(BUILD_BENCHMARKS AND EXISTS ${DEAL_II_TESTS_H})
    IF(${CMAKE_BUILD_TYPE} MATCHES "Release")
      MESSAGE(STATUS "Benchmarks enabled")

      # Set the time limit for the benchmark tests
      MATH(EXPR CUSTOM_TEST_TIME_LIMIT "24 * 60 * 60") # Seconds
      SET(TEST_TIME_LIMIT ${CUSTOM_TEST_TIME_LIMIT})

      INCLUDE_DIRECTORIES(${DEAL_II_SOURCE_DIR}/tests)
      ADD_SUBDIRECTORY(benchmarks)
    ELSE()
      MESSAGE(STATUS "Benchmarks disabled when not in release mode")
    ENDIF()
  ENDIF()
ENDIF()

[bangerth]

I think my concern is mostly that tests.h contains a rather rag-tag collection of stuff, and we've generally been quite liberal in putting whatever someone needed in at least two places into it. To me, it does not rise to the quality of things we typically make available. For example, it contains using namespace dealii; and none of the stuff we declare in that file is properly namespaced itself.

It's also not quite clear which parts a user project would actually want to use. I mean, is anyone going to use get_real_assert_zero_imag(const PETScWrappers::internal::VectorReference &a) or nonoverflow_add(Number a, Number b) or BoundingBox<dim> random_box(const double &min = 0.0, const double &max = 1.0)? It seems to me that projects that simply import that header lock stock and barrel have probably never taken a look at what's in there.

I'm not saying that there isn't useful stuff that would make for a good starting point for user tests.h files, and I'm not opposed to giving projects such a starting point, but I don't think that that file is it in its current form.

[peterrum]

Let me close this PR! The opposition is too large.

I went with a modified of @jppelteret's suggestion. Thanks!