search

Goddard-Fortran-Ecosystem / pFUnit

Parallel Fortran Unit Testing Framework
Other
171 stars 45 forks source link

Allow overriding add_test arguments via add_pfunit_ctest #376

Closed jphill4 closed 1 year ago

jphill4 commented 1 year ago

Similar to #374, for symmetry with CMake's native add_test, we should be able to set WORKING_DIRECTORY, CONFIGURATIONS, etc. via the add_pfunit_ctest command, e.g.

add_pfunit_ctest(my_tests
    TEST_SOURCES ${test_srcs}
    LINK_LIBRARIES sut
    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}   # Doesn't work!
)

It looks pretty straight forward to add these options to add_pfunit_ctest, but I note that WORKING_DIRECTORY is already explicitly set to ${CMAKE_CURRENT_BINARY_DIR} in the add_pfunit_ctest function. Is there a reason for setting this explicitly, and therefore a reason we should not allow the user to specify this option?

In my testing, the tests seemed to run fine if I updated the WORKING_DIRECTORY after the fact, e.g.

set_tests_properties(my_tests PROPERTIES
    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
)
tclune commented 1 year ago

I doubt there is a (good) reason that WORKING_DIRECTORY is set explicitly. The basic macro was contributed by users, and subsequently modified by me. My best guess is that it solved some issue at some point that was actually being caused by some other problem, or it could simply be CMake ignorance on my part.

I will try to implement this on the develop branch and check with a few other downstream projects.

tclune commented 1 year ago

Done. Will merge after CI tests pass. (Might not be until tomorrow ...)

Let me know if you need a release with this feature - otherwise I may let it live on develop for the time being.

jphill4 commented 1 year ago

Thank you! This will work for me! However, you might also consider exposing the "CONFIGURATIONS" option from add_test as well, though this is less frequently used. It can be nice to split unit tests into a "fast" and "slow" set, or perhaps a "serial"/"parallel" set. Again, can be set after declaration if really needed.

tclune commented 1 year ago

OK - that's another aspect of CMake with which I have little experience.