ctest: Allow passing -j without value to choose a contextual default

Under job server integration, added by commit 80fe56c481 (ctest: Add
support for running under a make job server on POSIX systems,
2023-11-15, v3.29.0-rc1~324^2), use a very high default so that
parallelism is effectively limited only by available job server tokens.

Otherwise, choose a default limit based on the number of processors.

Also allow passing `0` to specify unbounded parallelism.

Fixes: #25739

8 files changed, 134 insertions, 36 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 6efe008..53c5f76 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -43,6 +43,17 @@
 #include "cmUVJobServerClient.h"
 #include "cmWorkingDirectory.h"
 
+namespace {
+// For unspecified parallelism, limit to the number of processors,
+// but with a minimum greater than 1 so there is some parallelism.
+constexpr unsigned long kParallelLevelMinimum = 2u;
+
+// For "unbounded" parallelism, limit to a very high value.
+// Under a job server, parallelism is effectively limited
+// only by available job server tokens.
+constexpr unsigned long kParallelLevelUnbounded = 0x10000u;
+}
+
 namespace cmsys {
 class RegularExpression;
 }
@@ -72,6 +83,7 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler(
   , TestHandler(handler)
   , ProcessorsAvailable(cmAffinity::GetProcessorsAvailable())
   , HaveAffinity(this->ProcessorsAvailable.size())
+  , ParallelLevelDefault(kParallelLevelMinimum)
 {
 }
 
@@ -97,9 +109,43 @@ void cmCTestMultiProcessHandler::SetTests(TestMap tests,
 }
 
 // Set the max number of tests that can be run at the same time.
-void cmCTestMultiProcessHandler::SetParallelLevel(size_t level)
+void cmCTestMultiProcessHandler::SetParallelLevel(cm::optional<size_t> level)
+{
+  this->ParallelLevel = level;
+
+  if (!this->ParallelLevel) {
+    // '-j' was given with no value.  Limit by number of processors.
+    cmsys::SystemInformation info;
+    info.RunCPUCheck();
+    unsigned long processorCount = info.GetNumberOfLogicalCPU();
+
+    if (cm::optional<std::string> fakeProcessorCount =
+          cmSystemTools::GetEnvVar(
+            "__CTEST_FAKE_PROCESSOR_COUNT_FOR_TESTING")) {
+      unsigned long pc = 0;
+      if (cmStrToULong(*fakeProcessorCount, &pc)) {
+        processorCount = pc;
+      } else {
+        cmSystemTools::Error("Failed to parse fake processor count: " +
+                             *fakeProcessorCount);
+      }
+    }
+
+    this->ParallelLevelDefault =
+      std::max(kParallelLevelMinimum, processorCount);
+  }
+}
+
+size_t cmCTestMultiProcessHandler::GetParallelLevel() const
 {
-  this->ParallelLevel = level < 1 ? 1 : level;
+  if ((this->ParallelLevel && *this->ParallelLevel == 0) ||
+      (!this->ParallelLevel && this->JobServerClient)) {
+    return kParallelLevelUnbounded;
+  }
+  if (this->ParallelLevel) {
+    return *this->ParallelLevel;
+  }
+  return this->ParallelLevelDefault;
 }
 
 void cmCTestMultiProcessHandler::SetTestLoad(unsigned long load)
@@ -446,10 +492,11 @@ void cmCTestMultiProcessHandler::UnlockResources(int index)
 inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test)
 {
   size_t processors = static_cast<int>(this->Properties[test]->Processors);
+  size_t const parallelLevel = this->GetParallelLevel();
   // If processors setting is set higher than the -j
   // setting, we default to using all of the process slots.
-  if (processors > this->ParallelLevel) {
-    processors = this->ParallelLevel;
+  if (processors > parallelLevel) {
+    processors = parallelLevel;
   }
   // Cap tests that want affinity to the maximum affinity available.
   if (this->HaveAffinity && processors > this->HaveAffinity &&
@@ -503,8 +550,9 @@ void cmCTestMultiProcessHandler::StartNextTests()
 
   size_t numToStart = 0;
 
-  if (this->RunningCount < this->ParallelLevel) {
-    numToStart = this->ParallelLevel - this->RunningCount;
+  size_t const parallelLevel = this->GetParallelLevel();
+  if (this->RunningCount < parallelLevel) {
+    numToStart = parallelLevel - this->RunningCount;
   }
 
   if (numToStart == 0) {
@@ -518,7 +566,7 @@ void cmCTestMultiProcessHandler::StartNextTests()
   }
 
   bool allTestsFailedTestLoadCheck = false;
-  size_t minProcessorsRequired = this->ParallelLevel;
+  size_t minProcessorsRequired = this->GetParallelLevel();
   std::string testWithMinProcessors;
 
   cmsys::SystemInformation info;
@@ -813,7 +861,7 @@ void cmCTestMultiProcessHandler::ReadCostData()
 
       this->Properties[index]->PreviousRuns = prev;
       // When not running in parallel mode, don't use cost data
-      if (this->ParallelLevel > 1 && this->Properties[index] &&
+      if (this->GetParallelLevel() > 1 && this->Properties[index] &&
           this->Properties[index]->Cost == 0) {
         this->Properties[index]->Cost = cost;
       }
@@ -842,7 +890,7 @@ int cmCTestMultiProcessHandler::SearchByName(std::string const& name)
 
 void cmCTestMultiProcessHandler::CreateTestCostList()
 {
-  if (this->ParallelLevel > 1) {
+  if (this->GetParallelLevel() > 1) {
     this->CreateParallelTestCostList();
   } else {
     this->CreateSerialTestCostList();
diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h
index bbbfc7b..d4759f1 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.h
+++ b/Source/CTest/cmCTestMultiProcessHandler.h
@@ -63,7 +63,7 @@ public:
   // Set the tests
   void SetTests(TestMap tests, PropertiesMap properties);
   // Set the max number of tests that can be run at the same time.
-  void SetParallelLevel(size_t);
+  void SetParallelLevel(cm::optional<size_t> level);
   void SetTestLoad(unsigned long load);
   virtual void RunTests();
   void PrintOutputAsJson();
@@ -201,7 +201,15 @@ private:
     ResourceAvailabilityErrors;
   cmCTestResourceAllocator ResourceAllocator;
   std::vector<cmCTestTestHandler::cmCTestTestResult>* TestResults;
-  size_t ParallelLevel = 1; // max number of process that can be run at once
+
+  // Get the maximum number of processors that may be used at once.
+  size_t GetParallelLevel() const;
+
+  // With no '-j' option, default to serial testing.
+  cm::optional<size_t> ParallelLevel = 1;
+
+  // Fallback parallelism limit when '-j' is given with no value.
+  size_t ParallelLevelDefault;
 
   // 'make' jobserver client.  If connected, we acquire a token
   // for each test before running its process.
diff --git a/Source/CTest/cmCTestTestCommand.cxx b/Source/CTest/cmCTestTestCommand.cxx
index ed16354..98ce862 100644
--- a/Source/CTest/cmCTestTestCommand.cxx
+++ b/Source/CTest/cmCTestTestCommand.cxx
@@ -105,8 +105,8 @@ cmCTestGenericHandler* cmCTestTestCommand::InitializeHandler()
   if (this->StopOnFailure) {
     handler->SetOption("StopOnFailure", "ON");
   }
-  if (!this->ParallelLevel.empty()) {
-    handler->SetOption("ParallelLevel", this->ParallelLevel);
+  if (this->ParallelLevel) {
+    handler->SetOption("ParallelLevel", *this->ParallelLevel);
   }
   if (!this->Repeat.empty()) {
     handler->SetOption("Repeat", this->Repeat);
diff --git a/Source/CTest/cmCTestTestCommand.h b/Source/CTest/cmCTestTestCommand.h
index ce054ed..23661c5 100644
--- a/Source/CTest/cmCTestTestCommand.h
+++ b/Source/CTest/cmCTestTestCommand.h
@@ -8,7 +8,9 @@
 #include <utility>
 
 #include <cm/memory>
+#include <cm/optional>
 
+#include "cmArgumentParserTypes.h"
 #include "cmCTestHandlerCommand.h"
 #include "cmCommand.h"
 
@@ -56,7 +58,7 @@ protected:
   std::string ExcludeFixture;
   std::string ExcludeFixtureSetup;
   std::string ExcludeFixtureCleanup;
-  std::string ParallelLevel;
+  cm::optional<ArgumentParser::Maybe<std::string>> ParallelLevel;
   std::string Repeat;
   std::string ScheduleRandom;
   std::string StopTime;
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index 921ab1e..61a18a2 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -548,9 +548,21 @@ bool cmCTestTestHandler::ProcessOptions()
       return false;
     }
   }
-  if (this->GetOption("ParallelLevel")) {
-    this->CTest->SetParallelLevel(
-      std::stoi(*this->GetOption("ParallelLevel")));
+  if (cmValue parallelLevel = this->GetOption("ParallelLevel")) {
+    if (parallelLevel.IsEmpty()) {
+      // An empty value tells ctest to choose a default.
+      this->CTest->SetParallelLevel(cm::nullopt);
+    } else {
+      // A non-empty value must be a non-negative integer.
+      unsigned long plevel = 0;
+      if (!cmStrToULong(*parallelLevel, &plevel)) {
+        cmCTestLog(this->CTest, ERROR_MESSAGE,
+                   "ParallelLevel invalid value: " << *parallelLevel
+                                                   << std::endl);
+        return false;
+      }
+      this->CTest->SetParallelLevel(plevel);
+    }
   }
 
   if (this->GetOption("StopOnFailure")) {
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index 7acee4b..1ae3cb5 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -179,7 +179,7 @@ struct cmCTest::Private
 
   int MaxTestNameWidth = 30;
 
-  int ParallelLevel = 1;
+  cm::optional<size_t> ParallelLevel = 1;
   bool ParallelLevelSetInCli = false;
 
   unsigned long TestLoad = 0;
@@ -380,14 +380,14 @@ cmCTest::cmCTest()
 
 cmCTest::~cmCTest() = default;
 
-int cmCTest::GetParallelLevel() const
+cm::optional<size_t> cmCTest::GetParallelLevel() const
 {
   return this->Impl->ParallelLevel;
 }
 
-void cmCTest::SetParallelLevel(int level)
+void cmCTest::SetParallelLevel(cm::optional<size_t> level)
 {
-  this->Impl->ParallelLevel = level < 1 ? 1 : level;
+  this->Impl->ParallelLevel = level;
 }
 
 unsigned long cmCTest::GetTestLoad() const
@@ -1892,14 +1892,31 @@ bool cmCTest::HandleCommandLineArguments(size_t& i,
   std::string arg = args[i];
   if (this->CheckArgument(arg, "-F"_s)) {
     this->Impl->Failover = true;
-  } else if (this->CheckArgument(arg, "-j"_s, "--parallel") &&
-             i < args.size() - 1) {
-    i++;
-    int plevel = atoi(args[i].c_str());
-    this->SetParallelLevel(plevel);
+  } else if (this->CheckArgument(arg, "-j"_s, "--parallel")) {
+    cm::optional<size_t> parallelLevel;
+    // No value or an empty value tells ctest to choose a default.
+    if (i + 1 < args.size() && !cmHasLiteralPrefix(args[i + 1], "-")) {
+      ++i;
+      if (!args[i].empty()) {
+        // A non-empty value must be a non-negative integer.
+        unsigned long plevel = 0;
+        if (!cmStrToULong(args[i], &plevel)) {
+          errormsg =
+            cmStrCat("'", arg, "' given invalid value '", args[i], "'");
+          return false;
+        }
+        parallelLevel = plevel;
+      }
+    }
+    this->SetParallelLevel(parallelLevel);
     this->Impl->ParallelLevelSetInCli = true;
   } else if (cmHasPrefix(arg, "-j")) {
-    int plevel = atoi(arg.substr(2).c_str());
+    // The value must be a non-negative integer.
+    unsigned long plevel = 0;
+    if (!cmStrToULong(arg.substr(2), &plevel)) {
+      errormsg = cmStrCat("'", arg, "' given invalid value '", args[i], "'");
+      return false;
+    }
     this->SetParallelLevel(plevel);
     this->Impl->ParallelLevelSetInCli = true;
   }
@@ -2799,10 +2816,20 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
 
   // handle CTEST_PARALLEL_LEVEL environment variable
   if (!this->Impl->ParallelLevelSetInCli) {
-    std::string parallel;
-    if (cmSystemTools::GetEnv("CTEST_PARALLEL_LEVEL", parallel)) {
-      int plevel = atoi(parallel.c_str());
-      this->SetParallelLevel(plevel);
+    if (cm::optional<std::string> parallelEnv =
+          cmSystemTools::GetEnvVar("CTEST_PARALLEL_LEVEL")) {
+      if (parallelEnv->empty() ||
+          parallelEnv->find_first_not_of(" \t") == std::string::npos) {
+        // An empty value tells ctest to choose a default.
+        this->SetParallelLevel(cm::nullopt);
+      } else {
+        // A non-empty value must be a non-negative integer.
+        // Otherwise, ignore it.
+        unsigned long plevel = 0;
+        if (cmStrToULong(*parallelEnv, &plevel)) {
+          this->SetParallelLevel(plevel);
+        }
+      }
     }
   }
 
diff --git a/Source/cmCTest.h b/Source/cmCTest.h
index 1644d84..2ab810c 100644
--- a/Source/cmCTest.h
+++ b/Source/cmCTest.h
@@ -12,6 +12,7 @@
 #include <string>
 #include <vector>
 
+#include <cm/optional>
 #include <cm/string_view>
 
 #include "cmDuration.h"
@@ -116,8 +117,8 @@ public:
   cmDuration GetGlobalTimeout() const;
 
   /** how many test to run at the same time */
-  int GetParallelLevel() const;
-  void SetParallelLevel(int);
+  cm::optional<size_t> GetParallelLevel() const;
+  void SetParallelLevel(cm::optional<size_t> level);
 
   unsigned long GetTestLoad() const;
   void SetTestLoad(unsigned long);
diff --git a/Source/ctest.cxx b/Source/ctest.cxx
index c1c2071..f6a11b4 100644
--- a/Source/ctest.cxx
+++ b/Source/ctest.cxx
@@ -48,9 +48,9 @@ const cmDocumentationEntry cmDocumentationOptions[] = {
     "Truncate 'tail' (default), 'middle' or 'head' of test output once "
     "maximum output size is reached" },
   { "-F", "Enable failover." },
-  { "-j <jobs>, --parallel <jobs>",
-    "Run the tests in parallel using the "
-    "given number of jobs." },
+  { "-j [<level>], --parallel [<level>]",
+    "Run tests in parallel, "
+    "optionally limited to a given level of parallelism." },
   { "-Q,--quiet", "Make ctest quiet." },
   { "-O <file>, --output-log <file>", "Output to log file" },
   { "--output-junit <file>", "Output test results to JUnit XML file." },