instrumentation: Fix snippet `result` field

Give the actual exit code of the command from the snippet as intended;
currently, this always gives 0 for sub-commands like compile and link,
as well as `ctest`.

For now, the result in build snippets will be `null`.

Help/manual/cmake-instrumentation.7.rst

@@ -344,7 +344,8 @@ and contain the following data:
     The working directory in which the ``command`` was executed.
 
   ``result``
-    The exit-value of the command, an integer.
+    The exit code of the command, an integer. This will be ``null`` when
+    ``role`` is ``build``.
 
   ``role``
     The type of command executed, which will be one of the following values:

Source/CTest/cmCTestLaunch.cxx

@@ -306,7 +306,7 @@ int cmCTestLaunch::Run()
     this->Reporter.OptionCommandType, this->RealArgV,
     [this]() -> int {
       this->RunChild();
-      return 0;
+      return this->Reporter.ExitCode;
     },
     options, arrayOptions);
 

Source/cmCTest.cxx

@@ -2670,11 +2670,11 @@ int cmCTest::ExecuteTests(std::vector<std::string> const& args)
 
   cmInstrumentation instrumentation(this->GetBinaryDir());
   auto processHandler = [&handler]() -> int {
-    return handler.ProcessHandler();
+    return handler.ProcessHandler() < 0 ? cmCTest::TEST_ERRORS : 0;
   };
   int ret = instrumentation.InstrumentCommand("ctest", args, processHandler);
   instrumentation.CollectTimingData(cmInstrumentationQuery::Hook::PostCTest);
-  if (ret < 0) {
+  if (ret == cmCTest::TEST_ERRORS) {
     cmCTestLog(this, ERROR_MESSAGE, "Errors while running CTest\n");
     if (!this->Impl->OutputTestOutputOnTestFailure) {
       std::string const lastTestLog =
@@ -2685,10 +2685,8 @@ int cmCTest::ExecuteTests(std::vector<std::string> const& args)
                  "Use \"--rerun-failed --output-on-failure\" to re-run the "
                  "failed cases verbosely.\n");
     }
-    return cmCTest::TEST_ERRORS;
   }
-
-  return 0;
+  return ret;
 }
 
 int cmCTest::RunCMakeAndTest()

Source/cmInstrumentation.cxx

@@ -633,7 +633,6 @@ int cmInstrumentation::InstrumentCommand(
 
   // Execute Command
   int ret = callback();
-  root["result"] = ret;
 
   // Exit early if configure didn't generate a query
   if (reloadQueriesAfterCommand == LoadQueriesAfter::Yes) {
@@ -669,6 +668,9 @@ int cmInstrumentation::InstrumentCommand(
     }
   }
 
+  // See SpawnBuildDaemon(); this data is currently meaningless for build.
+  root["result"] = command_type == "build" ? Json::nullValue : ret;
+
   // Create empty config entry if config not found
   if (!root.isMember("config") &&
       (command_type == "compile" || command_type == "link" ||
@@ -852,6 +854,8 @@ int cmInstrumentation::CollectTimingAfterBuild(int ppid)
     while (0 == uv_kill(ppid, 0)) {
       cmSystemTools::Delay(100);
     };
+    // FIXME(#27331): Investigate a cross-platform solution to obtain the exit
+    // code given the `ppid` above.
     return 0;
   };
   int ret = this->InstrumentCommand(

Tests/RunCMake/Instrumentation/RunCMakeTest.cmake

@@ -20,6 +20,7 @@ function(instrument test)
     "MANUAL_HOOK"
     "PRESERVE_DATA"
     "NO_CONFIGURE"
+    "FAIL"
   )
   cmake_parse_arguments(ARGS "${OPTIONS}" "CHECK_SCRIPT;CONFIGURE_ARG" "" ${ARGN})
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test})
@@ -78,6 +79,9 @@ function(instrument test)
   if (ARGS_NO_WARN)
     list(APPEND ARGS_CONFIGURE_ARG "-Wno-dev")
   endif()
+  if (ARGS_FAIL)
+    list(APPEND ARGS_CONFIGURE_ARG "-DFAIL=ON")
+  endif()
   set(RunCMake_TEST_SOURCE_DIR ${RunCMake_SOURCE_DIR}/project)
   if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
     set(maybe_CMAKE_BUILD_TYPE -DCMAKE_BUILD_TYPE=Debug)
@@ -88,7 +92,29 @@ function(instrument test)
 
   # Follow-up Commands
   if (ARGS_BUILD)
-    run_cmake_command(${test}-build ${CMAKE_COMMAND} --build . --config Debug)
+    set(cmake_build_args --config Debug)
+    set(additional_build_args)
+    if (ARGS_FAIL)
+      # Tests with ARGS_FAIL expect all targets to build, including the ones
+      # which should succeed and those which should fail.
+      if (RunCMake_GENERATOR MATCHES "Ninja")
+        set(keep_going_arg -k 0)
+      elseif (RunCMake_GENERATOR MATCHES "FASTBuild")
+        set(keep_going_arg -nostoponerror)
+      else()
+        set(keep_going_arg -k)
+      endif()
+      string(APPEND additional_build_args ${keep_going_arg})
+      # Merge stdout and stderr because different compilers will throw their
+      # errors to different places.
+      set(RunCMake_TEST_OUTPUT_MERGE 1)
+    endif()
+    run_cmake_command(${test}-build
+      ${CMAKE_COMMAND} --build . ${cmake_build_args} -- ${additional_build_args}
+    )
+    if (ARGS_FAIL)
+      unset(RunCMake_TEST_OUTPUT_MERGE)
+    endif()
   endif()
   if (ARGS_BUILD_MAKE_PROGRAM)
     set(RunCMake_TEST_OUTPUT_MERGE 1)
@@ -167,6 +193,16 @@ instrument(cmake-command-cmake-build
   NO_WARN BUILD
   CHECK_SCRIPT check-no-make-program-hooks.cmake
 )
+if(RunCMake_GENERATOR STREQUAL "Borland Makefiles")
+  # Borland 'make' has no '-k' flag.
+  set(Skip_COMMAND_FAILURES_Case 1)
+endif()
+if(NOT Skip_COMMAND_FAILURES_Case)
+  instrument(cmake-command-failures
+    FAIL NO_WARN BUILD TEST INSTALL
+    CHECK_SCRIPT check-data-dir.cmake
+  )
+endif()
 
 # Test CUSTOM_CONTENT
 instrument(cmake-command-custom-content

Tests/RunCMake/Instrumentation/check-data-dir.cmake

@@ -24,28 +24,43 @@ foreach(snippet IN LISTS snippets)
   string(JSON target ERROR_VARIABLE noTarget GET "${contents}" target)
   if (NOT target MATCHES NOTFOUND)
     set(targets "main;lib;customTarget;TARGET_NAME")
+    if (ARGS_FAIL)
+      list(APPEND targets "dummy")
+    endif()
     if (NOT ${target} IN_LIST targets)
       json_error("${snippet}" "Unexpected target: ${target}")
     endif()
   endif()
 
-  # Verify output
-  string(JSON result GET "${contents}" result)
-  if (NOT ${result} EQUAL 0)
-    json_error("${snippet}" "Compile command had non-0 result")
-  endif()
-
   # Verify contents of compile-* Snippets
   if (filename MATCHES "^compile-")
     string(JSON target GET "${contents}" target)
     string(JSON source GET "${contents}" source)
     string(JSON language GET "${contents}" language)
+    string(JSON result GET "${contents}" result)
     if (NOT language MATCHES "C\\+\\+")
       json_error("${snippet}" "Expected C++ compile language")
     endif()
     if (NOT source MATCHES "${target}.cxx$")
       json_error("${snippet}" "Unexpected source file")
     endif()
+    if (ARGS_FAIL)
+      if (source MATCHES "dummy.cxx" AND result EQUAL 0)
+        json_error("${snippet}"
+          "Expected nonzero exit code for compile command, got: ${result}"
+        )
+      elseif (NOT source MATCHES "dummy.cxx" AND NOT result EQUAL 0)
+        json_error("${snippet}"
+          "Expected zero exit code for compile command, got: ${result}"
+        )
+      endif()
+    else()
+      if (NOT result EQUAL 0)
+        json_error("${snippet}"
+          "Expected zero exit code for compile command, got: ${result}"
+        )
+      endif()
+    endif()
   endif()
 
   # Verify contents of link-* Snippets
@@ -80,8 +95,40 @@ foreach(snippet IN LISTS snippets)
   # Verify contents of test-* Snippets
   if (filename MATCHES "^test-")
     string(JSON testName GET "${contents}" testName)
-    if (NOT testName STREQUAL "test")
-      json_error("${snippet}" "Unexpected testName: ${testName}")
+    string(JSON result GET "${contents}" result)
+    if (ARGS_FAIL)
+      if (testName STREQUAL "test" AND NOT result EQUAL 0)
+        json_error("${snippet}" "Expected zero exit code for test")
+      elseif (testName STREQUAL "dummy" AND result EQUAL 0)
+        json_error("${snippet}"
+          "Expected nonzero exit code for dummy test, got: ${result}"
+        )
+      elseif (NOT testName MATCHES "test|dummy")
+        json_error("${snippet}" "Unexpected test name: ${testName}")
+      endif()
+    else()
+      if (NOT testName STREQUAL "test")
+        json_error("${snippet}" "Unexpected test name: ${testName}")
+      endif()
+      if (NOT result EQUAL 0)
+        json_error("${snippet}"
+          "Expected zero exit code for test, got: ${result}"
+        )
+      endif()
+    endif()
+  endif()
+
+  # Verify the overall result, in addition to the sub-commands above.
+  if (filename MATCHES "^cmakeInstall|^cmakeBuild|^ctest")
+    string(JSON result GET "${contents}" result)
+    if (ARGS_FAIL AND result EQUAL 0)
+      json_error("${snippet}"
+        "Expected nonzero exit code, got: ${result}"
+      )
+    elseif (NOT ARGS_FAIL AND NOT result EQUAL 0)
+      json_error("${snippet}"
+        "Expected zero exit code, got: ${result}"
+      )
     endif()
   endif()
 

Tests/RunCMake/Instrumentation/cmake-command-failures-build-result.txt

@@ -0,0 +1 @@
+[^0]

Tests/RunCMake/Instrumentation/cmake-command-failures-build-stdout.txt

@@ -0,0 +1 @@
+.*dummy\.cxx.*

Tests/RunCMake/Instrumentation/cmake-command-failures-install-result.txt

@@ -0,0 +1 @@
+[^0]

Tests/RunCMake/Instrumentation/cmake-command-failures-install-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at cmake_install\.cmake.*:
+  Failed install\.

Tests/RunCMake/Instrumentation/cmake-command-failures-test-result.txt

@@ -0,0 +1 @@
+8

Tests/RunCMake/Instrumentation/cmake-command-failures-test-stderr.txt

@@ -0,0 +1 @@
+Errors while running CTest

Tests/RunCMake/Instrumentation/cmake-command-failures-test-stdout.txt

@@ -0,0 +1 @@
+dummy \(Failed\)

Tests/RunCMake/Instrumentation/project/CMakeLists.txt

@@ -25,3 +25,12 @@ add_test(NAME test COMMAND $<TARGET_FILE:main>)
 install(TARGETS main)
 set_target_properties(main PROPERTIES LABELS "label1;label2")
 set_target_properties(lib PROPERTIES LABELS "label3")
+
+if (FAIL)
+  file(WRITE "${CMAKE_BINARY_DIR}/dummy.cxx"
+    "#error \"something which will not compile\"\n"
+  )
+  add_executable(dummy "${CMAKE_BINARY_DIR}/dummy.cxx")
+  add_test(NAME dummy COMMAND ${CMAKE_COMMAND} -E false)
+  install(CODE "message(FATAL_ERROR \"Failed install.\")")
+endif()

Tests/RunCMake/Instrumentation/query/cmake-command-failures.cmake

@@ -0,0 +1,4 @@
+cmake_instrumentation(
+  API_VERSION 1
+  DATA_VERSION 1
+)

Tests/RunCMake/Instrumentation/verify-snippet.cmake

@@ -5,10 +5,12 @@ include(${CMAKE_CURRENT_LIST_DIR}/json.cmake)
 function(snippet_has_fields snippet contents)
   get_filename_component(filename "${snippet}" NAME)
   json_has_key("${snippet}" "${contents}" role)
-  json_has_key("${snippet}" "${contents}" result)
   json_has_key("${snippet}" "${contents}" workingDir)
+  json_has_key("${snippet}" "${contents}" result)
   if (NOT filename MATCHES "^build-*")
     json_has_key("${snippet}" "${contents}" command)
+  else()
+    json_missing_key("${snippet}" "${contents}" command)
   endif()
   if (filename MATCHES "^link-*")
     json_has_key("${snippet}" "${contents}" target)
@@ -74,6 +76,13 @@ function(verify_snippet_data snippet contents)
   if (NOT version EQUAL 1)
     json_error("${snippet}" "Version must be 1, got: ${version}")
   endif()
+  get_filename_component(filename "${snippet}" NAME)
+  string(JSON result GET "${contents}" result)
+  if ("${filename}" MATCHES "^build-*" AND result)
+    json_error("${snippet}" "Result must be null for build snippets, got: ${result}")
+  elseif (NOT "${filename}" MATCHES "^build-*" AND NOT result MATCHES "^-?[0-9]+$")
+    json_error("${snippet}" "Result must be integer, got: ${result}")
+  endif()
   string(JSON outputs ERROR_VARIABLE noOutputs GET "${contents}" outputs)
   if (NOT outputs MATCHES NOTFOUND)
     string(JSON outputSizes ERROR_VARIABLE noOutputSizes GET "${contents}" outputSizes)