Goddard-Fortran-Ecosystem / pFUnit

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

Relative TEST_SOURCES paths in `add_pfunit_ctest` #426

Open Mikolaj-A-Kowalski opened 1 year ago

Mikolaj-A-Kowalski commented 1 year ago

Hi everybody!

I really hope I didn't do anything silly and miss some existing issue about it, but I have been trying to use add_pfunit_ctest function with a relative test file path (the attached small project relative_test.zip I used cmake 3.18.4):

cmake_minimum_required(VERSION 3.10)
project(RelTestFile)

enable_language(Fortran)

find_package(PFUNIT REQUIRED)

enable_testing()

add_pfunit_ctest (better_tests
  TEST_SOURCES ./tests/hello_test.pf
)

Which resulted in an error

CMake Error at /home/user/path/pFUnit/build/installed/PFUNIT-4.7/include/add_pfunit_ctest.cmake:71 (add_executable):
  Cannot find source file:

    hello_test.F90

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm

  .hpp .hxx .in .txx

From what I am able to tell it is caused by inconsistent treatment of the file paths between add_pfunit_ctest and add_pfunit_sources.

Basically add_pfunit_sources when is given a relative path it moves it from source directory to binary directory upon preprocessing. So file ${CMAKE_CURRENT_SOURCE_DIR}/tests/hello_test.pf will be mapped to ${CMAKE_CURRENT_BINARY_DIR}/tests/hello_test.F90. However, add_pfunit_ctest does not seem to take it into account in the script, the lines in the link below:

https://github.com/Goddard-Fortran-Ecosystem/pFUnit/blob/74e6f4a0c5ff622d3e127b17c29baafe2ca45e5d/include/add_pfunit_ctest.cmake#LL54C1-L75C6

Just add <basename>.F90 in the add_executable on line 71, which causes the error I have seen in my example.

After I modified add_pfunit_ctest to apply the same treatment as add_pfunit_sources everything worked fine for my example.

My question is if there is any particular reason I am missing why add_pfunit_ctest is as it is? If not I am happy to open a pull request to apply the same treatment for test_sources in both _ctest and _sources functions.

tclune commented 1 year ago

Apparently you are just the first "fortunate" soul to have encountered this. (Or the others decided to work around it.) This macro was contributed by a user and I've slowly evolved it since then. Feel free to put in a PR, or I can try to get to it this weekend. I'm drowning in unrelated work these days, so I'm loathe to promise much.

Mikolaj-A-Kowalski commented 1 year ago

No worries! Thank you for the quick reply and wonderful test framework! I will try to submit the PR promptly!