catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.22k stars 3.01k forks source link

Respect path order of DL_PATHS in catch_discover_tests #2878

Open cy20lin opened 1 week ago

cy20lin commented 1 week ago

Description

In catch_discover_tests function, the DL_PATHS opition is used to specify the directories of the dependent shared libraries. The specified paths are used in the function to setup the environment for (1) retrieving the list of test cases from the text executable and for (2) executing the tests.

With current implementation, given DL_PATHS is specified, the prepared execution environments are inconsistent between (1) and (2), and the order of the paths specified by the DL_PATHS for searching libraries is not respected as well.

This PR tweak the catch_discover_tests_impl function, so that the execution environments are made consistent and the order of the search paths specified by DL_PATHS are respected.

Detail

Current behavior

The code for modifing the execution environment for (1) is shown as below, where the original value of environment variable is discarded and its value is overwrited by the specified DL_PATHS.

if(dl_paths)
    cmake_path(CONVERT "${dl_paths}" TO_NATIVE_PATH_LIST paths)
    set(ENV{${dl_paths_variable_name}} "${paths}")
endif()

The code for modifing the execution environment for (2) is shown as follows, the paths in the DL_PATHS are transformed to the perform the path_list_prepend command for each specified path in order, which resulting in the order of searching DL_PATHS being reversed, and follows the searching order of the original values from the environment.

  if(dl_paths)
    foreach(path ${dl_paths})
      cmake_path(NATIVE_PATH path native_path)
      list(APPEND environment_modifications "${dl_paths_variable_name}=path_list_prepend:${native_path}")
    endforeach()
  endif()

For example, when running on Windows, the environment variable for searching dependent shared libraries is PATH (i.e. dl_paths_variable_name is setted to PATH).

Given

We could see that under the current implementation,

Which are inconsistent, and the specified order is not respected.

Proposed behavior

The code for (1) is modified such that the DL_PATHS is prepend before the original paths, instead of overwriting the original values.

  if(dl_paths)
    cmake_path(CONVERT "$ENV{${dl_paths_variable_name}}" TO_NATIVE_PATH_LIST env_dl_paths)
    list(PREPEND env_dl_paths "${dl_paths}")
    cmake_path(CONVERT "${env_dl_paths}" TO_NATIVE_PATH_LIST paths)
    set(ENV{${dl_paths_variable_name}} "${paths}")
  endif()

The code for (2) has been modified so that the commands in environment_modifications follow a "first path, last prepend" order. This ensures that the finding procedure would first search the paths in DL_PATHS with the specified order, followed by the paths from the original environment.

  if(dl_paths)
    foreach(path ${dl_paths})
      cmake_path(NATIVE_PATH path native_path)
      list(PREPEND environment_modifications "${dl_paths_variable_name}=path_list_prepend:${native_path}")
    endforeach()
  endif()

For example, given that

With the proposed implementation, the modified environment for (1) and (2) becomes: PATH=/path/to/dir/A;/path/to/dir/B;/path/to/dir/C;/path/to/dir/D

The paths order are respected and the execution environments are made consistent.

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.11%. Comparing base (4e8d92b) to head (c642c4b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## devel #2878 +/- ## ========================================== + Coverage 91.10% 91.11% +0.01% ========================================== Files 198 198 Lines 8493 8493 ========================================== + Hits 7737 7738 +1 + Misses 756 755 -1 ```