Merge topic 'envmod-test-modifying-existing'

9c4d6404eb Tests/Environment: also test modifying ambient values
7d52d48a32 cmCTestRunTest: get the default value from the environment

Acked-by: Kitware Robot <[email protected]>
Merge-request: !6682

Source/CTest/cmCTestRunTest.cxx

@@ -784,6 +784,9 @@ bool cmCTestRunTest::ForkProcess(
 
   std::ostringstream envMeasurement;
   if (environment && !environment->empty()) {
+    // Environment modification works on the assumption that the environment is
+    // actually modified here. If another strategy is used, there will need to
+    // be updates below in `apply_diff`.
     cmSystemTools::AppendEnv(*environment);
     for (auto const& var : *environment) {
       envMeasurement << var << std::endl;
@@ -802,7 +805,20 @@ bool cmCTestRunTest::ForkProcess(
     auto apply_diff =
       [&env_application](const std::string& name,
                          std::function<void(std::string&)> const& apply) {
-        std::string output = env_application[name].value_or(std::string{});
+        cm::optional<std::string> old_value = env_application[name];
+        std::string output;
+        if (old_value) {
+          output = *old_value;
+        } else {
+          // This only works because the environment is actually modified above
+          // (`AppendEnv`). If CTest ever just creates an environment block
+          // directly, that block will need to be queried for the subprocess'
+          // value instead.
+          const char* curval = cmSystemTools::GetEnv(name);
+          if (curval) {
+            output = curval;
+          }
+        }
         apply(output);
         env_application[name] = output;
       };

Tests/CMakeLists.txt

@@ -1380,6 +1380,13 @@ if(BUILD_TESTING)
     --test-command ${CMAKE_CTEST_COMMAND} -V
     )
   list(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/Environment")
+  set_property(TEST Environment APPEND
+    PROPERTY ENVIRONMENT
+      "SET_FROM_AMBIENT_unset=base"
+      "SET_FROM_AMBIENT_replace=base"
+      "SET_FROM_AMBIENT_string=base"
+      "SET_FROM_AMBIENT_path=base"
+      "SET_FROM_AMBIENT_list=base")
 
   add_test(QtAutomocNoQt  ${CMAKE_CTEST_COMMAND}
     --build-and-test

Tests/Environment/CMakeLists.txt

@@ -26,8 +26,32 @@ set_tests_properties(Environment2 EchoEnvironment2 PROPERTIES
   FAIL_REGULAR_EXPRESSION "CMAKE_ENV.*Happy Thanksgiving"
 )
 
+set_property(TEST EchoEnvironment3
+  PROPERTY ENVIRONMENT
+    "SET_FROM_ENVIRONMENT_PROPERTY_unset=base"
+    "SET_FROM_ENVIRONMENT_PROPERTY_replace=base"
+    "SET_FROM_ENVIRONMENT_PROPERTY_string=base"
+    "SET_FROM_ENVIRONMENT_PROPERTY_path=base"
+    "SET_FROM_ENVIRONMENT_PROPERTY_list=base"
+)
+
 set_property(TEST EchoEnvironment3
   PROPERTY ENVIRONMENT_MODIFICATION
+    # Modifying variables set in the ambient environment (see properties for
+    # this test in `Tests/CMakeLists.txt`).
+    "SET_FROM_AMBIENT_unset=unset:"
+    "SET_FROM_AMBIENT_replace=set:new"
+    "SET_FROM_AMBIENT_string=string_append:new"
+    "SET_FROM_AMBIENT_path=path_list_append:new"
+    "SET_FROM_AMBIENT_list=cmake_list_append:new"
+
+    # Modifying variables set in the `ENVIRONMENT` property.
+    "SET_FROM_ENVIRONMENT_PROPERTY_unset=unset:"
+    "SET_FROM_ENVIRONMENT_PROPERTY_replace=set:new"
+    "SET_FROM_ENVIRONMENT_PROPERTY_string=string_append:new"
+    "SET_FROM_ENVIRONMENT_PROPERTY_path=path_list_append:new"
+    "SET_FROM_ENVIRONMENT_PROPERTY_list=cmake_list_append:new"
+
     # Variables expected to be unset.
     "UNSET_EXPLICIT=set:value"
     "UNSET_EXPLICIT=unset:"

Tests/Environment/check_mod.cmake

@@ -14,6 +14,8 @@ else ()
   set(path_sep ":")
 endif ()
 
+set(unexpect_SET_FROM_AMBIENT_unset "")
+set(unexpect_SET_FROM_ENVIRONMENT_PROPERTY_unset "")
 set(unexpect_UNSET_EXPLICIT "")
 set(unexpect_UNSET_VIA_RESET "")
 set(expect_DIRECT "new")
@@ -23,8 +25,24 @@ set(expect_CMAKE_LIST_MANIP "prefix;pre;core;post;suffix")
 set(expect_STRING_DNE "prefix-prepost-suffix")
 set(expect_PATH_DNE "prefix${path_sep}pre${path_sep}post${path_sep}suffix")
 set(expect_CMAKE_LIST_DNE "prefix;pre;post;suffix")
+set(expect_SET_FROM_AMBIENT_replace "new")
+set(expect_SET_FROM_AMBIENT_string "basenew")
+set(expect_SET_FROM_AMBIENT_path "base${path_sep}new")
+set(expect_SET_FROM_AMBIENT_list "base;new")
+set(expect_SET_FROM_ENVIRONMENT_PROPERTY_replace "new")
+set(expect_SET_FROM_ENVIRONMENT_PROPERTY_string "basenew")
+set(expect_SET_FROM_ENVIRONMENT_PROPERTY_path "base${path_sep}new")
+set(expect_SET_FROM_ENVIRONMENT_PROPERTY_list "base;new")
 
 set(expected_vars
+  SET_FROM_AMBIENT_replace
+  SET_FROM_AMBIENT_string
+  SET_FROM_AMBIENT_path
+  SET_FROM_AMBIENT_list
+  SET_FROM_ENVIRONMENT_PROPERTY_replace
+  SET_FROM_ENVIRONMENT_PROPERTY_string
+  SET_FROM_ENVIRONMENT_PROPERTY_path
+  SET_FROM_ENVIRONMENT_PROPERTY_list
   DIRECT
   STRING_MANIP
   PATH_MANIP