Merge topic 'ctest-generate-resource-spec-file'

c8c1dd0d95 CTest: Add ability to dynamically generate resource spec file
3f5a5a5856 cmCTestRunTest::StartFailure(): Add total argument

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !8699

6 files changed, 147 insertions, 42 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 7d22a87..ca07a08 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -35,6 +35,7 @@
 #include "cmCTestRunTest.h"
 #include "cmCTestTestHandler.h"
 #include "cmDuration.h"
+#include "cmJSONState.h"
 #include "cmListFileCache.h"
 #include "cmRange.h"
 #include "cmStringAlgorithms.h"
@@ -75,6 +76,7 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler()
   this->ProcessorsAvailable = cmAffinity::GetProcessorsAvailable();
   this->HaveAffinity = this->ProcessorsAvailable.size();
   this->HasCycles = false;
+  this->HasInvalidGeneratedResourceSpec = false;
   this->SerialTestRunning = false;
 }
 
@@ -95,7 +97,9 @@ void cmCTestMultiProcessHandler::SetTests(TestMap& tests,
   if (!this->CTest->GetShowOnly()) {
     this->ReadCostData();
     this->HasCycles = !this->CheckCycles();
-    if (this->HasCycles) {
+    this->HasInvalidGeneratedResourceSpec =
+      !this->CheckGeneratedResourceSpec();
+    if (this->HasCycles || this->HasInvalidGeneratedResourceSpec) {
       return;
     }
     this->CreateTestCostList();
@@ -125,7 +129,7 @@ void cmCTestMultiProcessHandler::SetTestLoad(unsigned long load)
 void cmCTestMultiProcessHandler::RunTests()
 {
   this->CheckResume();
-  if (this->HasCycles) {
+  if (this->HasCycles || this->HasInvalidGeneratedResourceSpec) {
     return;
   }
 #ifdef CMAKE_UV_SIGNAL_HACK
@@ -180,7 +184,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
   }
   testRun->SetIndex(test);
   testRun->SetTestProperties(this->Properties[test]);
-  if (this->TestHandler->UseResourceSpec) {
+  if (this->UseResourceSpec) {
     testRun->SetUseAllocatedResources(true);
     testRun->SetAllocatedResources(this->AllocatedResources[test]);
   }
@@ -229,15 +233,15 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
       }
       e << "\n";
     }
-    e << "Resource spec file:\n\n  " << this->TestHandler->ResourceSpecFile;
-    cmCTestRunTest::StartFailure(std::move(testRun), e.str(),
+    e << "Resource spec file:\n\n  " << this->ResourceSpecFile;
+    cmCTestRunTest::StartFailure(std::move(testRun), this->Total, e.str(),
                                  "Insufficient resources");
     return false;
   }
 
   cmWorkingDirectory workdir(this->Properties[test]->Directory);
   if (workdir.Failed()) {
-    cmCTestRunTest::StartFailure(std::move(testRun),
+    cmCTestRunTest::StartFailure(std::move(testRun), this->Total,
                                  "Failed to change working directory to " +
                                    this->Properties[test]->Directory + " : " +
                                    std::strerror(workdir.GetLastResult()),
@@ -253,7 +257,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
 
 bool cmCTestMultiProcessHandler::AllocateResources(int index)
 {
-  if (!this->TestHandler->UseResourceSpec) {
+  if (!this->UseResourceSpec) {
     return true;
   }
 
@@ -322,7 +326,7 @@ bool cmCTestMultiProcessHandler::TryAllocateResources(
 
 void cmCTestMultiProcessHandler::DeallocateResources(int index)
 {
-  if (!this->TestHandler->UseResourceSpec) {
+  if (!this->UseResourceSpec) {
     return;
   }
 
@@ -358,7 +362,7 @@ bool cmCTestMultiProcessHandler::AllResourcesAvailable()
 
 void cmCTestMultiProcessHandler::CheckResourcesAvailable()
 {
-  if (this->TestHandler->UseResourceSpec) {
+  if (this->UseResourceSpec) {
     for (auto test : this->SortedTests) {
       std::map<std::string, std::vector<cmCTestBinPackerAllocation>>
         allocations;
@@ -1445,3 +1449,81 @@ bool cmCTestMultiProcessHandler::CheckCycles()
                      this->Quiet);
   return true;
 }
+
+bool cmCTestMultiProcessHandler::CheckGeneratedResourceSpec()
+{
+  for (auto& test : this->Properties) {
+    if (!test.second->GeneratedResourceSpecFile.empty()) {
+      if (this->ResourceSpecSetupTest) {
+        cmCTestLog(
+          this->CTest, ERROR_MESSAGE,
+          "Only one test may define the GENERATED_RESOURCE_SPEC_FILE property"
+            << std::endl);
+        return false;
+      }
+
+      if (test.second->FixturesSetup.size() != 1) {
+        cmCTestLog(this->CTest, ERROR_MESSAGE,
+                   "Test that defines GENERATED_RESOURCE_SPEC_FILE must have "
+                   "exactly one FIXTURES_SETUP"
+                     << std::endl);
+        return false;
+      }
+
+      if (!cmSystemTools::FileIsFullPath(
+            test.second->GeneratedResourceSpecFile)) {
+        cmCTestLog(this->CTest, ERROR_MESSAGE,
+                   "GENERATED_RESOURCE_SPEC_FILE must be an absolute path"
+                     << std::endl);
+        return false;
+      }
+
+      this->ResourceSpecSetupTest = test.first;
+      this->ResourceSpecSetupFixture = *test.second->FixturesSetup.begin();
+    }
+  }
+
+  if (!this->ResourceSpecSetupFixture.empty()) {
+    for (auto& test : this->Properties) {
+      if (!test.second->ResourceGroups.empty() &&
+          !test.second->FixturesRequired.count(
+            this->ResourceSpecSetupFixture)) {
+        cmCTestLog(this->CTest, ERROR_MESSAGE,
+                   "All tests that have RESOURCE_GROUPS must include the "
+                   "resource spec generator fixture in their FIXTURES_REQUIRED"
+                     << std::endl);
+        return false;
+      }
+    }
+  }
+
+  if (!this->ResourceSpecFile.empty()) {
+    if (this->ResourceSpecSetupTest) {
+      cmCTestLog(this->CTest, ERROR_MESSAGE,
+                 "GENERATED_RESOURCE_SPEC_FILE test property cannot be used "
+                 "in conjunction with ResourceSpecFile option"
+                   << std::endl);
+      return false;
+    }
+    std::string error;
+    if (!this->InitResourceAllocator(error)) {
+      cmCTestLog(this->CTest, ERROR_MESSAGE, error << std::endl);
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool cmCTestMultiProcessHandler::InitResourceAllocator(std::string& error)
+{
+  if (!this->ResourceSpec.ReadFromJSONFile(this->ResourceSpecFile)) {
+    error = cmStrCat("Could not read/parse resource spec file ",
+                     this->ResourceSpecFile, ": ",
+                     this->ResourceSpec.parseState.GetErrorMessage());
+    return false;
+  }
+  this->UseResourceSpec = true;
+  this->ResourceAllocator.InitializeFromResourceSpec(this->ResourceSpec);
+  return true;
+}
diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h
index 2f5ad40..3b4e9c5 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.h
+++ b/Source/CTest/cmCTestMultiProcessHandler.h
@@ -11,15 +11,17 @@
 #include <string>
 #include <vector>
 
+#include <cm/optional>
+
 #include <cm3p/uv.h>
 
 #include "cmCTest.h"
 #include "cmCTestResourceAllocator.h"
+#include "cmCTestResourceSpec.h"
 #include "cmCTestTestHandler.h"
 #include "cmUVHandlePtr.h"
 
 struct cmCTestBinPackerAllocation;
-class cmCTestResourceSpec;
 class cmCTestRunTest;
 
 /** \class cmCTestMultiProcessHandler
@@ -90,13 +92,13 @@ public:
     this->RepeatCount = count;
   }
 
-  void SetQuiet(bool b) { this->Quiet = b; }
-
-  void InitResourceAllocator(const cmCTestResourceSpec& spec)
+  void SetResourceSpecFile(const std::string& resourceSpecFile)
   {
-    this->ResourceAllocator.InitializeFromResourceSpec(spec);
+    this->ResourceSpecFile = resourceSpecFile;
   }
 
+  void SetQuiet(bool b) { this->Quiet = b; }
+
   void CheckResourcesAvailable();
 
 protected:
@@ -158,6 +160,15 @@ protected:
     std::map<std::string, ResourceAllocationError>* errors = nullptr);
   void DeallocateResources(int index);
   bool AllResourcesAvailable();
+  bool InitResourceAllocator(std::string& error);
+  bool CheckGeneratedResourceSpec();
+
+  bool UseResourceSpec = false;
+  cmCTestResourceSpec ResourceSpec;
+  std::string ResourceSpecFile;
+  std::string ResourceSpecSetupFixture;
+  cm::optional<std::size_t> ResourceSpecSetupTest;
+  bool HasInvalidGeneratedResourceSpec;
 
   // map from test number to set of depend tests
   TestMap Tests;
diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index 563439a..4c57cf6 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -152,6 +152,18 @@ cmCTestRunTest::EndTestResult cmCTestRunTest::EndTest(size_t completed,
       }
     }
   }
+  std::string resourceSpecParseError;
+  if (!this->TestProperties->GeneratedResourceSpecFile.empty()) {
+    this->MultiTestHandler.ResourceSpecFile =
+      this->TestProperties->GeneratedResourceSpecFile;
+    if (!this->MultiTestHandler.InitResourceAllocator(
+          resourceSpecParseError)) {
+      reason = "Invalid resource spec file";
+      forceFail = true;
+    } else {
+      this->MultiTestHandler.CheckResourcesAvailable();
+    }
+  }
   std::ostringstream outputStream;
   if (res == cmProcess::State::Exited) {
     bool success = !forceFail &&
@@ -260,6 +272,16 @@ cmCTestRunTest::EndTestResult cmCTestRunTest::EndTest(size_t completed,
     cmCTestLog(this->CTest, HANDLER_OUTPUT, this->ProcessOutput << std::endl);
   }
 
+  if (!resourceSpecParseError.empty()) {
+    cmCTestLog(this->CTest, ERROR_MESSAGE,
+               resourceSpecParseError << std::endl);
+  } else if (!this->TestProperties->GeneratedResourceSpecFile.empty()) {
+    cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+               "Using generated resource spec file "
+                 << this->TestProperties->GeneratedResourceSpecFile
+                 << std::endl);
+  }
+
   if (this->TestHandler->LogFile) {
     *this->TestHandler->LogFile << "Test time = " << buf << std::endl;
   }
@@ -372,7 +394,8 @@ bool cmCTestRunTest::StartAgain(std::unique_ptr<cmCTestRunTest> runner,
   // change to tests directory
   cmWorkingDirectory workdir(testRun->TestProperties->Directory);
   if (workdir.Failed()) {
-    testRun->StartFailure("Failed to change working directory to " +
+    testRun->StartFailure(testRun->TotalNumberOfTests,
+                          "Failed to change working directory to " +
                             testRun->TestProperties->Directory + " : " +
                             std::strerror(workdir.GetLastResult()),
                           "Failed to change working directory");
@@ -437,25 +460,25 @@ void cmCTestRunTest::MemCheckPostProcess()
 }
 
 void cmCTestRunTest::StartFailure(std::unique_ptr<cmCTestRunTest> runner,
-                                  std::string const& output,
+                                  size_t total, std::string const& output,
                                   std::string const& detail)
 {
   auto* testRun = runner.get();
 
   testRun->TestProcess = cm::make_unique<cmProcess>(std::move(runner));
-  testRun->StartFailure(output, detail);
+  testRun->StartFailure(total, output, detail);
 
   testRun->FinalizeTest(false);
 }
 
-void cmCTestRunTest::StartFailure(std::string const& output,
+void cmCTestRunTest::StartFailure(size_t total, std::string const& output,
                                   std::string const& detail)
 {
   // Still need to log the Start message so the test summary records our
   // attempt to start this test
   if (!this->CTest->GetTestProgressOutput()) {
     cmCTestLog(this->CTest, HANDLER_OUTPUT,
-               std::setw(2 * getNumWidth(this->TotalNumberOfTests) + 8)
+               std::setw(2 * getNumWidth(total) + 8)
                  << "Start "
                  << std::setw(getNumWidth(this->TestHandler->GetMaxIndex()))
                  << this->TestProperties->Index << ": "
diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h
index fed7296..34f23c4 100644
--- a/Source/CTest/cmCTestRunTest.h
+++ b/Source/CTest/cmCTestRunTest.h
@@ -68,7 +68,7 @@ public:
                          size_t completed);
 
   static void StartFailure(std::unique_ptr<cmCTestRunTest> runner,
-                           std::string const& output,
+                           size_t total, std::string const& output,
                            std::string const& detail);
 
   struct EndTestResult
@@ -86,7 +86,8 @@ public:
 
   void ComputeWeightedCost();
 
-  void StartFailure(std::string const& output, std::string const& detail);
+  void StartFailure(size_t total, std::string const& output,
+                    std::string const& detail);
 
   cmCTest* GetCTest() const { return this->CTest; }
 
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index 94014a8..e4b9239 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -42,7 +42,6 @@
 #include "cmExecutionStatus.h"
 #include "cmGeneratedFileStream.h"
 #include "cmGlobalGenerator.h"
-#include "cmJSONState.h"
 #include "cmList.h"
 #include "cmMakefile.h"
 #include "cmState.h"
@@ -284,7 +283,6 @@ cmCTestTestHandler::cmCTestTestHandler()
   this->UseIncludeRegExpFlag = false;
   this->UseExcludeRegExpFlag = false;
   this->UseExcludeRegExpFirst = false;
-  this->UseResourceSpec = false;
 
   this->CustomMaximumPassedTestOutputSize = 1 * 1024;
   this->CustomMaximumFailedTestOutputSize = 300 * 1024;
@@ -891,8 +889,7 @@ bool cmCTestTestHandler::ComputeTestList()
   }
 
   if (this->RerunFailed) {
-    this->ComputeTestListForRerunFailed();
-    return true;
+    return this->ComputeTestListForRerunFailed();
   }
 
   cmCTestTestHandler::ListOfTests::size_type tmsize = this->TestList.size();
@@ -951,7 +948,7 @@ bool cmCTestTestHandler::ComputeTestList()
   return true;
 }
 
-void cmCTestTestHandler::ComputeTestListForRerunFailed()
+bool cmCTestTestHandler::ComputeTestListForRerunFailed()
 {
   this->ExpandTestsToRunInformationForRerunFailed();
 
@@ -978,6 +975,8 @@ void cmCTestTestHandler::ComputeTestListForRerunFailed()
   this->TestList = finalList;
 
   this->UpdateMaxTestNameWidth();
+
+  return true;
 }
 
 void cmCTestTestHandler::UpdateForFixtures(ListOfTests& tests) const
@@ -1351,18 +1350,6 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector<std::string>& passed,
   } else {
     parallel->SetTestLoad(this->CTest->GetTestLoad());
   }
-  if (!this->ResourceSpecFile.empty()) {
-    this->UseResourceSpec = true;
-    if (!this->ResourceSpec.ReadFromJSONFile(this->ResourceSpecFile)) {
-      cmCTestLog(this->CTest, ERROR_MESSAGE,
-                 "Could not read/parse resource spec file "
-                   << this->ResourceSpecFile << ": "
-                   << this->ResourceSpec.parseState.GetErrorMessage()
-                   << std::endl);
-      return false;
-    }
-    parallel->InitResourceAllocator(this->ResourceSpec);
-  }
 
   *this->LogFile
     << "Start testing: " << this->CTest->CurrentTime() << std::endl
@@ -1397,6 +1384,7 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector<std::string>& passed,
     tests[p.Index] = depends;
     properties[p.Index] = &p;
   }
+  parallel->SetResourceSpecFile(this->ResourceSpecFile);
   parallel->SetTests(tests, properties);
   parallel->SetPassFailVectors(&passed, &failed);
   this->TestResults.clear();
@@ -2336,6 +2324,8 @@ bool cmCTestTestHandler::SetTestsProperties(
             if (!ParseResourceGroupsProperty(val, rt.ResourceGroups)) {
               return false;
             }
+          } else if (key == "GENERATED_RESOURCE_SPEC_FILE"_s) {
+            rt.GeneratedResourceSpecFile = val;
           } else if (key == "SKIP_RETURN_CODE"_s) {
             rt.SkipReturnCode = atoi(val.c_str());
             if (rt.SkipReturnCode < 0 || rt.SkipReturnCode > 255) {
diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h
index 3e80c94..23f0a76 100644
--- a/Source/CTest/cmCTestTestHandler.h
+++ b/Source/CTest/cmCTestTestHandler.h
@@ -21,7 +21,6 @@
 
 #include "cmCTest.h"
 #include "cmCTestGenericHandler.h"
-#include "cmCTestResourceSpec.h"
 #include "cmCTestTypes.h" // IWYU pragma: keep
 #include "cmDuration.h"
 #include "cmListFileCache.h"
@@ -172,6 +171,7 @@ public:
     std::set<std::string> FixturesRequired;
     std::set<std::string> RequireSuccessDepends;
     std::vector<std::vector<cmCTestTestResourceRequirement>> ResourceGroups;
+    std::string GeneratedResourceSpecFile;
     // Private test generator properties used to track backtraces
     cmListFileBacktrace Backtrace;
   };
@@ -319,7 +319,7 @@ private:
 
   // compute the lists of tests that will actually run
   // based on LastTestFailed.log
-  void ComputeTestListForRerunFailed();
+  bool ComputeTestListForRerunFailed();
 
   // add required setup/cleanup tests not already in the
   // list of tests to be run and update dependencies between
@@ -360,8 +360,6 @@ private:
   cmsys::RegularExpression IncludeTestsRegularExpression;
   cmsys::RegularExpression ExcludeTestsRegularExpression;
 
-  bool UseResourceSpec;
-  cmCTestResourceSpec ResourceSpec;
   std::string ResourceSpecFile;
 
   void RecordCustomTestMeasurements(cmXMLWriter& xml, std::string content);