Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: fortuno_discover_tests and fortuno_add_tests function #8

Open
LecrisUT opened this issue Feb 28, 2024 · 4 comments
Open

CMake: fortuno_discover_tests and fortuno_add_tests function #8

LecrisUT opened this issue Feb 28, 2024 · 4 comments

Comments

@LecrisUT
Copy link

LecrisUT commented Feb 28, 2024

This part should only focus on registering the tests, and there are two interfaces to implement:

  • fortuno_add_tests: Add tests by scanning the source code. This makes the tests and list of tests available at configure time. Could use some regex magic to parse this
  • fortuno_discover_tests: Discover the tests dynamically by listing the tests from the test-suite executable

At this stage, we will not worry about generating the test-suite app itself, and instead just automatically register the tests

From Catch2 and gtest implementation, we should mirror them as much as possible, so the synopsis should be like:

    fortuno_add_tests(TARGET target
                    [SOURCES src1...]
                    [EXTRA_ARGS arg1...]
                    [WORKING_DIRECTORY dir]
                    [TEST_PREFIX prefix]
                    [TEST_SUFFIX suffix]
                    [SKIP_DEPENDENCY]
                    [TEST_LIST outVar]
    )
    fortuno_discover_tests(target
                    [EXTRA_ARGS arg1...]
                    [WORKING_DIRECTORY dir]
                    [TEST_PREFIX prefix]
                    [TEST_SUFFIX suffix]
                    [TEST_FILTER expr]
                    [PROPERTIES name1 value1...]
                    [TEST_LIST var]
                    [DL_PATHS path1 ...]
                    [DISCOVERY_MODE <POST_BUILD|PRE_TEST>]
    )

(DL_PATHS is low priority, it's just for windows to add dll directory to PATH)

Reference:

@aradi
Copy link
Member

aradi commented Feb 28, 2024

Just two remarks:

  • I think, the regex magic is very fragile. Think about parameterized tests, which I could append as

    ...testitems=[(param_test(("my_test_ + as_char(i)", i = 1, 100)]
    

    I think, there is almost no way to get it right. The discover-ing could work much better IMO, then you compile the test-app and obtain all the registered tests.

  • As a more rethorical question: Does it really make sense, to make an individual CTest from every unit test. Imagine you have thousand MPI unit tests. When run individually via CTest, you have to set up the MPI-environment 1000 times for a unit test, which runs maybe only a few microseconds. This can be painfully slow. Also, the user has the possibility to make more than one testapps (let's say, 10 testapps with 100 tests each), if a certain kind of splitting the test set is needed.

@LecrisUT
Copy link
Author

  • I think, the regex magic is very fragile.

Indeed, this would only be for basic formats that follow a predictable pattern like test_..., and it would only regex the subroutine without parametrization. It is important to support a add_tests format in order to have the lists of tests that can be later manipulated, e.g. adding PROCESSORS property for MPI testing.

  • As a more rethorical question: Does it really make sense, to make an individual CTest from every unit test.

Yes, because the IDEs support running individual tests/labels, where you can add the debugger.

When run individually via CTest, you have to set up the MPI-environment 1000 times for a unit test, which runs maybe only a few microseconds.

Good point, I'm not sure how gtest and catch do it in this case. They do have test fixtures, but I don't think they take advantage of this in their design when they are run through ctest. I think there could be some conflict on how to report and measure the test result for individual tests. Worth raising upstream for some advice.

The only design I can think of is to use ctest TEST_FIXTURE to setup/cleanup mpi environments, and pass a PID, attach to a program, etc. so that the mpi communicator(s) can be reused. I think with C++ there are some neat ways of running these in the background, otherwise, it would be a cool idea to make ctest support such approach.

@aradi
Copy link
Member

aradi commented Feb 28, 2024

The only design I can think of is to use ctest TEST_FIXTURE to setup/cleanup mpi environments, and pass a PID, attach to a program, etc. so that the mpi communicator(s) can be reused. I think with C++ there are some neat ways of running these in the background, otherwise, it would be a cool idea to make ctest support such approach.

Hm, that sounds rather complicated, I am not even sure, whether a new process can be attached to an already running MPI-framework... So, if people discover the tests and run them one-by-one as separate processes, they would have to live with the penalty caused by the repeated MPI-setups...

@aradi
Copy link
Member

aradi commented Mar 21, 2024

OK. I had a quick look on the Catch and the GTest integration with CMake. Both test systems seems to prefer the discover-method, which uses the compiled test binary to extract the list of tests. I think, we should first concentrate on this as well as this is the only really robust and reliable way to get the list of tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants