ctest: Allow arbitrary characters in test names of CTestCostData.txt

This changes the way lines in CTestCostData.txt are parsed to allow for
spaces in the test name.

It does so by looking for space characters from the end; and once two
have been found, assumes everything from the beginning up to that
second-to-last-space is the test name.

Additionally, parsing the file should be much more efficient since there
is no string or vector heap allocation per line. The std::string used by
the parse function to convert the int and float should be within most
standard libraries' small string optimization.

Fixes: #26594

Source/CTest/cmCTestMultiProcessHandler.cxx

@@ -19,6 +19,7 @@
 
 #include <cm/memory>
 #include <cm/optional>
+#include <cm/string_view>
 #include <cmext/algorithm>
 
 #include <cm3p/json/value.h>
@@ -51,6 +52,48 @@ constexpr unsigned long kParallelLevelMinimum = 2u;
 // Under a job server, parallelism is effectively limited
 // only by available job server tokens.
 constexpr unsigned long kParallelLevelUnbounded = 0x10000u;
+
+struct CostEntry
+{
+  cm::string_view name;
+  int prevRuns;
+  float cost;
+};
+
+cm::optional<CostEntry> splitCostLine(cm::string_view line)
+{
+  std::string part;
+  cm::string_view::size_type pos1 = line.size();
+  cm::string_view::size_type pos2 = line.find_last_of(' ', pos1);
+  auto findNext = [line, &part, &pos1, &pos2]() -> bool {
+    if (pos2 != cm::string_view::npos) {
+      cm::string_view sub = line.substr(pos2 + 1, pos1 - pos2 - 1);
+      part.assign(sub.begin(), sub.end());
+      pos1 = pos2;
+      if (pos1 > 0) {
+        pos2 = line.find_last_of(' ', pos1 - 1);
+      }
+      return true;
+    }
+    return false;
+  };
+
+  // parse the cost
+  if (!findNext()) {
+    return cm::nullopt;
+  }
+  float cost = static_cast<float>(atof(part.c_str()));
+
+  // parse the previous runs
+  if (!findNext()) {
+    return cm::nullopt;
+  }
+  int prev = atoi(part.c_str());
+
+  // from start to the last found space is the name
+  return CostEntry{ line.substr(0, pos1), prev, cost };
+}
+
 }
 
 namespace cmsys {
@@ -794,24 +837,21 @@ void cmCTestMultiProcessHandler::UpdateCostData()
       if (line == "---") {
         break;
       }
-      std::vector<std::string> parts = cmSystemTools::SplitString(line, ' ');
       // Format: <name> <previous_runs> <avg_cost>
-      if (parts.size() < 3) {
+      cm::optional<CostEntry> entry = splitCostLine(line);
+      if (!entry) {
         break;
       }
 
-      std::string name = parts[0];
+      int index = this->SearchByName(entry->name);
-      int prev = atoi(parts[1].c_str());
-      float cost = static_cast<float>(atof(parts[2].c_str()));
-
-      int index = this->SearchByName(name);
       if (index == -1) {
         // This test is not in memory. We just rewrite the entry
-        fout << name << " " << prev << " " << cost << "\n";
+        fout << entry->name << " " << entry->prevRuns << " " << entry->cost
+             << "\n";
       } else {
         // Update with our new average cost
-        fout << name << " " << this->Properties[index]->PreviousRuns << " "
+        fout << entry->name << " " << this->Properties[index]->PreviousRuns
-             << this->Properties[index]->Cost << "\n";
+             << " " << this->Properties[index]->Cost << "\n";
         temp.erase(index);
       }
     }
@@ -847,28 +887,25 @@ void cmCTestMultiProcessHandler::ReadCostData()
         break;
       }
 
-      std::vector<std::string> parts = cmSystemTools::SplitString(line, ' ');
+      // Format: <name> <previous_runs> <avg_cost>
+      cm::optional<CostEntry> entry = splitCostLine(line);
 
       // Probably an older version of the file, will be fixed next run
-      if (parts.size() < 3) {
+      if (!entry) {
         fin.close();
         return;
       }
 
-      std::string name = parts[0];
+      int index = this->SearchByName(entry->name);
-      int prev = atoi(parts[1].c_str());
-      float cost = static_cast<float>(atof(parts[2].c_str()));
-
-      int index = this->SearchByName(name);
       if (index == -1) {
         continue;
       }
 
-      this->Properties[index]->PreviousRuns = prev;
+      this->Properties[index]->PreviousRuns = entry->prevRuns;
       // When not running in parallel mode, don't use cost data
       if (this->GetParallelLevel() > 1 && this->Properties[index] &&
           this->Properties[index]->Cost == 0) {
-        this->Properties[index]->Cost = cost;
+        this->Properties[index]->Cost = entry->cost;
       }
     }
     // Next part of the file is the failed tests
@@ -881,7 +918,7 @@ void cmCTestMultiProcessHandler::ReadCostData()
   }
 }
 
-int cmCTestMultiProcessHandler::SearchByName(std::string const& name)
+int cmCTestMultiProcessHandler::SearchByName(cm::string_view name)
 {
   int index = -1;
 

Source/CTest/cmCTestMultiProcessHandler.h

@@ -13,6 +13,7 @@
 #include <vector>
 
 #include <cm/optional>
+#include <cm/string_view>
 
 #include "cmCTest.h"
 #include "cmCTestResourceAllocator.h"
@@ -110,7 +111,7 @@ protected:
   void UpdateCostData();
   void ReadCostData();
   // Return index of a test based on its name
-  int SearchByName(std::string const& name);
+  int SearchByName(cm::string_view name);
 
   void CreateTestCostList();
 

Tests/CTestTestScheduler/CMakeLists.txt

@@ -1,9 +1,9 @@
-cmake_minimum_required(VERSION 3.10)
+cmake_minimum_required(VERSION 3.19)
 project (CTestTestScheduler)
 include (CTest)
 
 add_executable (Sleep sleep.c)
 
 foreach (time RANGE 1 4)
-  add_test (TestSleep${time} Sleep ${time})
+  add_test ("TestSleep ${time}" Sleep ${time})
 endforeach ()