Merge topic 'instrumentation-exit'

236207b81a instrumentation: Fix snippet `result` field d3add47973 Tests/Instrumentation: Remove unused files Acked-by: Kitware Robot <kwrobot@kitware.com> Merge-request: !11339
2026-01-04 12:49:36 -06:00 · 2025-10-28 13:38:02 +00:00
parent 7b661e5a66 236207b81a
commit 5f889a093c
20 changed files with 134 additions and 25 deletions
--- a/Help/manual/cmake-instrumentation.7.rst
+++ b/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:
--- a/Source/CTest/cmCTestLaunch.cxx
+++ b/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);

--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -2670,14 +2670,14 @@ 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;
  };
  std::map<std::string, std::string> data;
  data["showOnly"] = this->GetShowOnly() ? "1" : "0";
  int ret =
    instrumentation.InstrumentCommand("ctest", args, processHandler, data);
  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 =
@@ -2688,10 +2688,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()
--- a/Source/cmInstrumentation.cxx
+++ b/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) {
@@ -671,6 +670,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" ||
@@ -858,6 +860,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(
--- a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake
+++ b/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})
@@ -82,6 +83,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)
@@ -92,7 +96,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)
@@ -171,6 +197,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
--- a/Tests/RunCMake/Instrumentation/check-data-dir.cmake
+++ b/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()

--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-build-result.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-build-result.txt
@@ -0,0 +1 @@
+[^0]
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-build-stdout.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-build-stdout.txt
@@ -0,0 +1 @@
+.*dummy\.cxx.*
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-install-result.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-install-result.txt
@@ -0,0 +1 @@
+[^0]
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-install-stderr.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-install-stderr.txt
@@ -0,0 +1,2 @@
+CMake Error at cmake_install\.cmake.*:
+  Failed install\.
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-test-result.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-test-result.txt
@@ -0,0 +1 @@
+8
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-test-stderr.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-test-stderr.txt
@@ -0,0 +1 @@
+Errors while running CTest
--- a/Tests/RunCMake/Instrumentation/cmake-command-failures-test-stdout.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-failures-test-stdout.txt
@@ -0,0 +1 @@
+dummy \(Failed\)
--- a/Tests/RunCMake/Instrumentation/cmake-command-non-int-version-result.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-non-int-version-result.txt
@@ -1 +0,0 @@
-1
--- a/Tests/RunCMake/Instrumentation/cmake-command-non-int-version-stderr.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-non-int-version-stderr.txt
@@ -1,2 +0,0 @@
-CMake Error at CMakeLists\.txt:37 \(cmake_instrumentation\):
-  cmake_instrumentation given a non-integer DATA_VERSION\.
--- a/Tests/RunCMake/Instrumentation/cmake-command-unsupported-version-result.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-unsupported-version-result.txt
@@ -1 +0,0 @@
-1
--- a/Tests/RunCMake/Instrumentation/cmake-command-unsupported-version-stderr.txt
+++ b/Tests/RunCMake/Instrumentation/cmake-command-unsupported-version-stderr.txt
@@ -1,3 +0,0 @@
-CMake Error at CMakeLists\.txt:44 \(cmake_instrumentation\):
-  cmake_instrumentation given an unsupported API_VERSION "0" \(the only
-  currently supported version is 1\)\.
--- a/Tests/RunCMake/Instrumentation/project/CMakeLists.txt
+++ b/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()
--- a/Tests/RunCMake/Instrumentation/query/cmake-command-failures.cmake
+++ b/Tests/RunCMake/Instrumentation/query/cmake-command-failures.cmake
@@ -0,0 +1,4 @@
+cmake_instrumentation(
+  API_VERSION 1
+  DATA_VERSION 1
+)
--- a/Tests/RunCMake/Instrumentation/verify-snippet.cmake
+++ b/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)
@@ -76,6 +78,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)