Merge branch 'googletest-discoveryusetestenv' into 'master'

Draft: GoogleTest: Add DISCOVERY_USE_TEST_ENV

See merge request cmake/cmake!11563

Help/release/dev/googletest-listseparator.rst

@@ -0,0 +1,5 @@
+googletest-listseparator
+------------------------
+
+* The :module:`GoogleTest` module :command:`gtest_discover_tests` command
+  gained a `LIST_SEPARATOR` option.

Modules/GoogleTest.cmake

@@ -174,12 +174,14 @@ same as the Google Test name (i.e. ``suite.testcase``); see also
                          [TEST_SUFFIX suffix]
                          [TEST_FILTER expr]
                          [NO_PRETTY_TYPES] [NO_PRETTY_VALUES]
+                         [LIST_SEPARATOR sep]
                          [PROPERTIES name1 value1...]
                          [TEST_LIST var]
                          [DISCOVERY_TIMEOUT seconds]
                          [XML_OUTPUT_DIR dir]
                          [DISCOVERY_MODE <POST_BUILD|PRE_TEST>]
                          [DISCOVERY_EXTRA_ARGS args...]
+                         [DISCOVERY_USE_TEST_ENV]
     )
 
   .. versionadded:: 3.10
@@ -328,6 +330,67 @@ same as the Google Test name (i.e. ``suite.testcase``); see also
     :prop_tgt:`CROSSCOMPILING_EMULATOR` target properties are preserved,
     see policy :policy:`CMP0178`.
 
+  ``LIST_SEPARATOR``
+    .. versionadded:: 4.3
+
+    By default, ``PROPERTIES`` key-value pairs cannot have lists as their
+    values. Providing a non-semicolon character delimiter, it is possible to
+    pass lists as property values. Do note that doing so the chosen character
+    cannot appear in any of the property values.
+
+  ``DISCOVERY_USE_TEST_ENV``
+    .. versionadded:: 4.3
+
+    By default, only test execution uses the environment described by the
+    :prop_test:`ENVIRONMENT` and :prop_test`ENVIRONMENT_MODIFICATION` test
+    properties. This option tells the module to also apply these properties to
+    the discovery step, regardless of ``DISCOVERY_MODE``.
+
+Examples
+========
+
+``DISCOVERY_USE_TEST_ENV`` allows describing test environments where even
+discovery depends on environment modifications. Combining it with
+``LIST_SEPARATOR`` enables even more complex scenarios requiring multiple
+environment modfiications. This can be crucial on Windows platforms, where
+RPATH isn't available to tell the linker about DLL dependency locations in
+the build tree. Without modifying the environment, one has to resort to
+other methods, typically copying DLLs next to test executables with
+``add_custom_command()``s. As mentioned in ``DISCOVERY_MODE``, setting the
+discovery mode to ``PRE_TEST`` helps in some cases, but is of limited use.
+Setting the environment as flexibly, robustly in a multi-config generator
+friendly way from a CMake preset JSON or an external script is challanging.
+
+.. code-block:: cmake
+
+  # Test executable depending on two SHARED libraries
+  # even during discovery.
+  add_executable(my_test my_test.cpp)
+
+  target_link_libraries(my_test PRIVATE
+      shared_lib1
+      shared_lib2
+      GTest::gtest
+      GTest::gtest_main
+  )
+
+  set(LABELS label1 label2)
+  set(ENVIRONMENT_MODIFICATION
+      PATH=path_list_prepend:$<TARGET_FILE_DIR:shared_lib1>
+      PATH=path_list_prepend:$<TARGET_FILE_DIR:shared_lib2>
+  )
+  set(LIST_SEPARATOR ",")
+  list(JOIN LABELS ${LIST_SEPARATOR} LABELS)
+  list(JOIN ENVIRONMENT_MODIFICATION ${LIST_SEPARATOR} ENVIRONMENT_MODIFICATION)
+
+  gtest_discover_tests(my_test
+      LIST_SEPARATOR ${LIST_SEPARATOR}
+      PROPERTIES
+          LABELS "${LABELS}"
+          ENVIRONMENT_MODIFICATION "${ENVIRONMENT_MODIFICATION}"
+          DISCOVERY_USE_TEST_ENV ON
+  )
+
 #]=======================================================================]
 
 # Save project's policies
@@ -553,6 +616,7 @@ function(gtest_discover_tests target)
   set(options
     NO_PRETTY_TYPES
     NO_PRETTY_VALUES
+    DISCOVERY_USE_TEST_ENV
   )
   set(oneValueArgs
     TEST_PREFIX
@@ -562,6 +626,7 @@ function(gtest_discover_tests target)
     DISCOVERY_TIMEOUT
     XML_OUTPUT_DIR
     DISCOVERY_MODE
+    LIST_SEPARATOR
   )
   set(multiValueArgs
     EXTRA_ARGS
@@ -684,10 +749,34 @@ function(gtest_discover_tests target)
   endif()
 
   if(arg_DISCOVERY_MODE STREQUAL "POST_BUILD")
+    if(arg_DISCOVERY_USE_TEST_ENV)
+      foreach(PROP ENVIRONMENT ENVIRONMENT_MODIFICATION)
+        list(FIND arg_PROPERTIES ${PROP} ${PROP}_INDEX)
+        if(NOT ${PROP}_INDEX EQUAL -1)
+          math(EXPR VALUE_INDEX "${${PROP}_INDEX} + 1")
+          list(GET arg_PROPERTIES ${VALUE_INDEX} ${PROP}_VALUE)
+          string(REPLACE "${arg_LIST_SEPARATOR}" ";" ${PROP}_VALUE "${${PROP}_VALUE}")
+        endif()
+      endforeach()
+      # if discovery should use test env, then one of the relevant test props
+      # must have been set, otherwise TEST_ENV_FRAGMENT will be ill-formed.
+      if(NOT ENVIRONMENT_INDEX EQUAL -1 OR NOT ENVIRONMENT_MODIFICATION_INDEX EQUAL -1)
+        foreach(ENV_MOD_VALUE IN LISTS ENVIRONMENT_MODIFICATION_VALUE)
+          list(APPEND ENVIRONMENT_MODIFICATION_FRAGMENT "--modify" "${ENV_MOD_VALUE}")
+        endforeach()
+        set(TEST_ENV_FRAGMENT
+          -E env
+          ${ENVIRONMENT_VALUE}
+          ${ENVIRONMENT_MODIFICATION_FRAGMENT}
+          --
+          "${CMAKE_COMMAND}"
+        )
+      endif()
+    endif()
     add_custom_command(
       TARGET ${target} POST_BUILD
       BYPRODUCTS "${ctest_tests_file}"
-      COMMAND "${CMAKE_COMMAND}"
+      COMMAND "${CMAKE_COMMAND}" ${TEST_ENV_FRAGMENT}
               -D "TEST_TARGET=${target}"
               -D "TEST_EXECUTABLE=$<TARGET_FILE:${target}>"
               -D "TEST_EXECUTOR=${test_executor}"
@@ -699,6 +788,8 @@ function(gtest_discover_tests target)
               -D "TEST_FILTER=${arg_TEST_FILTER}"
               -D "NO_PRETTY_TYPES=${arg_NO_PRETTY_TYPES}"
               -D "NO_PRETTY_VALUES=${arg_NO_PRETTY_VALUES}"
+              -D "DISCOVERY_USE_TEST_ENV=${arg_DISCOVERY_USE_TEST_ENV}"
+              -D "LIST_SEPARATOR=${arg_LIST_SEPARATOR}"
               -D "TEST_LIST=${arg_TEST_LIST}"
               -D "CTEST_FILE=${ctest_tests_file}"
               -D "TEST_DISCOVERY_TIMEOUT=${arg_DISCOVERY_TIMEOUT}"

Modules/GoogleTestAddTests.cmake

@@ -52,7 +52,7 @@ macro(write_test_to_file)
   # Handle disabled tests
   set(maybe_DISABLED "")
   if(pretty_test_suite MATCHES "^DISABLED_" OR pretty_test_name MATCHES "^DISABLED_")
-    set(maybe_DISABLED DISABLED YES)
+    set(maybe_DISABLED "DISABLED YES")
     string(REGEX REPLACE "^DISABLED_" "" pretty_test_suite "${pretty_test_suite}")
     string(REGEX REPLACE "^DISABLED_" "" pretty_test_name "${pretty_test_name}")
   endif()
@@ -98,6 +98,10 @@ macro(write_test_to_file)
   # Add to script. Do not use add_command() here because it messes up the
   # handling of empty values when forwarding arguments, and we need to
   # preserve those carefully for arg_TEST_EXECUTOR and arg_EXTRA_ARGS.
+  # Test properties with values that are lists are also not handled
+  # correctly because the properties need to be expressed as a list
+  # of key-value pairs, but that list is flattened, so any values
+  # that are lists can't be differentiated from the next key.
   string(APPEND script "add_test(${guarded_testname} ${launcherArgs}")
   foreach(arg IN ITEMS
     "${arg_TEST_EXECUTABLE}"
@@ -121,18 +125,26 @@ macro(write_test_to_file)
 
   set(maybe_LOCATION "")
   if(NOT current_test_file STREQUAL "" AND NOT current_test_line STREQUAL "")
-    set(maybe_LOCATION DEF_SOURCE_LINE "${current_test_file}:${current_test_line}")
+    set(maybe_LOCATION "DEF_SOURCE_LINE [==[${current_test_file}:${current_test_line}]==]")
   endif()
 
-  add_command(set_tests_properties
-    "${guarded_testname}"
-    PROPERTIES
-      ${maybe_DISABLED}
-      ${maybe_LOCATION}
-      WORKING_DIRECTORY "${arg_TEST_WORKING_DIR}"
-      SKIP_REGULAR_EXPRESSION "\\[  SKIPPED \\]"
-      ${arg_TEST_PROPERTIES}
-  )
+  string(APPEND script "set_tests_properties(${guarded_testname} PROPERTIES ${maybe_DISABLED} ${maybe_LOCATION} WORKING_DIRECTORY [==[${arg_TEST_WORKING_DIR}]==] SKIP_REGULAR_EXPRESSION [==[\\[  SKIPPED \\]]==]")
+  if(arg_TEST_PROPERTIES)
+    # Making local copy of arg_TEST_PROPERTIES, as write_test_to_file is
+    # called in a loop and we don't want to destroy the parsed arg value
+    # by POP_FRONT-ing from it.
+    set(test_properties "${arg_TEST_PROPERTIES}")
+    list(LENGTH test_properties test_properties_length)
+    while(NOT test_properties_length EQUAL 0)
+      list(POP_FRONT test_properties property_name property_value)
+      list(LENGTH test_properties test_properties_length)
+      if(arg_LIST_SEPARATOR)
+        string(REPLACE "${arg_LIST_SEPARATOR}" ";" property_value "${property_value}")
+      endif()
+      string(APPEND script " ${property_name} [==[${property_value}]==]")
+    endwhile()
+  endif()
+  string(APPEND script ")\n")
 
   # possibly unbalanced square brackets render lists invalid so skip such
   # tests in ${arg_TEST_LIST}
@@ -281,6 +293,7 @@ function(gtest_discover_tests_impl)
   set(oneValueArgs
     NO_PRETTY_TYPES   # These two take a value, unlike gtest_discover_tests()
     NO_PRETTY_VALUES  #
+    LIST_SEPARATOR
     TEST_TARGET
     TEST_EXECUTABLE
     TEST_WORKING_DIR
@@ -435,5 +448,6 @@ if(CMAKE_SCRIPT_MODE_FILE)
     TEST_EXTRA_ARGS "${TEST_EXTRA_ARGS}"
     TEST_DISCOVERY_EXTRA_ARGS "${TEST_DISCOVERY_EXTRA_ARGS}"
     TEST_PROPERTIES "${TEST_PROPERTIES}"
+    LIST_SEPARATOR "${LIST_SEPARATOR}"
   )
 endif()

Tests/GoogleTest/Test/CMakeLists.txt

@@ -114,3 +114,28 @@ gtest_add_tests($<TARGET_FILE:test_gtest4> "" main4.h)
 if(NOT TEST GoogleTest.NoKeywords)
   message(FATAL_ERROR "Test case GoogleTest.NoKeywords not defined")
 endif()
+
+# Check if LIST_SEPARATOR allows passing both ENVIRONMENT values to test
+add_executable(test_gtest5 main5.cxx)
+target_link_libraries(test_gtest5 GTest::Main)
+set(ENVIRONMENT VALX=1 VALY=2)
+set(LIST_SEPARATOR ",")
+list(JOIN ENVIRONMENT ${LIST_SEPARATOR} ENVIRONMENT)
+gtest_discover_tests(test_gtest5
+  LIST_SEPARATOR ${LIST_SEPARATOR}
+  PROPERTIES
+    ENVIRONMENT "${ENVIRONMENT}"
+)
+
+# Check if DISCOVERY_USE_TEST_ENV really inherits relevant properties
+add_executable(test_gtest5env main5.cxx)
+target_link_libraries(test_gtest5env GTest::GTest)
+target_compile_definitions(test_gtest5env PRIVATE DISCOVERY_USE_TEST_ENV)
+set(ENVIRONMENT VALX=1)
+set(ENVIRONMENT_MODIFICATION VALY=set:2)
+gtest_discover_tests(test_gtest5env
+  PROPERTIES
+    ENVIRONMENT "${ENVIRONMENT}"
+    ENVIRONMENT_MODIFICATION "${ENVIRONMENT_MODIFICATION}"
+    DISCOVERY_USE_TEST_ENV ON
+)

Tests/GoogleTest/Test/main1.cxx

@@ -20,11 +20,10 @@ int main(int argc, char* argv[])
 {
   ::testing::InitGoogleTest(&argc, argv);
 
-  if (argc > 1) {
-    if (argv[1] != std::string("--forceFail")) {
-      throw "Unexpected argument";
+  for (int i = 0; i < argc; ++i) {
+    if (argv[i] == std::string("--forceFail")) {
+      shouldFail = true;
     }
-    shouldFail = true;
   }
   return RUN_ALL_TESTS();
 }

Tests/GoogleTest/Test/main5.cxx

@@ -0,0 +1,22 @@
+#include <cstdlib>
+
+#include <gtest/gtest.h>
+
+TEST(GoogleTest, Add)
+{
+  EXPECT_EQ(std::atoi(std::getenv("VALX")) + std::atoi(std::getenv("VALY")),
+            3);
+}
+
+#ifdef DISCOVERY_USE_TEST_ENV
+int main(int argc, char* argv[])
+{
+  if (std::getenv("VALX") == nullptr || std::getenv("VALY") == nullptr) {
+    std::cerr << "VALX or VALY not present in main." << std::endl;
+    return -1;
+  }
+
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
+#endif