ctest: Show error on invalid ctest arguments

Fixes: #24227

10 files changed, 88 insertions, 65 deletions

diff --git a/Source/CTest/cmCTestBuildAndTestHandler.cxx b/Source/CTest/cmCTestBuildAndTestHandler.cxx
index bb6ccc3..9faabf7 100644
--- a/Source/CTest/cmCTestBuildAndTestHandler.cxx
+++ b/Source/CTest/cmCTestBuildAndTestHandler.cxx
@@ -365,8 +365,9 @@ int cmCTestBuildAndTestHandler::RunCMakeAndTest(std::string* outstring)
 
 int cmCTestBuildAndTestHandler::ProcessCommandLineArguments(
   const std::string& currentArg, size_t& idx,
-  const std::vector<std::string>& allArgs)
+  const std::vector<std::string>& allArgs, bool& validArg)
 {
+  bool buildAndTestArg = true;
   // --build-and-test options
   if (cmHasLiteralPrefix(currentArg, "--build-and-test") &&
       idx < allArgs.size() - 1) {
@@ -385,78 +386,68 @@ int cmCTestBuildAndTestHandler::ProcessCommandLineArguments(
                    << std::endl);
       return 0;
     }
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-target") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-target") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildTargets.push_back(allArgs[idx]);
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-nocmake")) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-nocmake")) {
     this->BuildNoCMake = true;
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-run-dir") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-run-dir") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildRunDir = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-two-config")) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-two-config")) {
     this->BuildTwoConfig = true;
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-exe-dir") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-exe-dir") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->ExecutableDirectory = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--test-timeout") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--test-timeout") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->Timeout = cmDuration(atof(allArgs[idx].c_str()));
-  }
-  if (currentArg == "--build-generator" && idx < allArgs.size() - 1) {
+  } else if (currentArg == "--build-generator" && idx < allArgs.size() - 1) {
     idx++;
     this->BuildGenerator = allArgs[idx];
-  }
-  if (currentArg == "--build-generator-platform" && idx < allArgs.size() - 1) {
+  } else if (currentArg == "--build-generator-platform" &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildGeneratorPlatform = allArgs[idx];
-  }
-  if (currentArg == "--build-generator-toolset" && idx < allArgs.size() - 1) {
+  } else if (currentArg == "--build-generator-toolset" &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildGeneratorToolset = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-project") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-project") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildProject = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-makeprogram") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-makeprogram") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->BuildMakeProgram = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-config-sample") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-config-sample") &&
+             idx < allArgs.size() - 1) {
     idx++;
     this->ConfigSample = allArgs[idx];
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-noclean")) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-noclean")) {
     this->BuildNoClean = true;
-  }
-  if (cmHasLiteralPrefix(currentArg, "--build-options")) {
+  } else if (cmHasLiteralPrefix(currentArg, "--build-options")) {
     while (idx + 1 < allArgs.size() && allArgs[idx + 1] != "--build-target" &&
            allArgs[idx + 1] != "--test-command") {
       ++idx;
       this->BuildOptions.push_back(allArgs[idx]);
     }
-  }
-  if (cmHasLiteralPrefix(currentArg, "--test-command") &&
-      idx < allArgs.size() - 1) {
+  } else if (cmHasLiteralPrefix(currentArg, "--test-command") &&
+             idx < allArgs.size() - 1) {
     ++idx;
     this->TestCommand = allArgs[idx];
     while (idx + 1 < allArgs.size()) {
       ++idx;
       this->TestCommandArgs.push_back(allArgs[idx]);
     }
+  } else {
+    buildAndTestArg = false;
   }
+  validArg = validArg || buildAndTestArg;
   return 1;
 }
diff --git a/Source/CTest/cmCTestBuildAndTestHandler.h b/Source/CTest/cmCTestBuildAndTestHandler.h
index e022e68..60b3a11 100644
--- a/Source/CTest/cmCTestBuildAndTestHandler.h
+++ b/Source/CTest/cmCTestBuildAndTestHandler.h
@@ -29,9 +29,9 @@ public:
   int ProcessHandler() override;
 
   //! Set all the build and test arguments
-  int ProcessCommandLineArguments(
-    const std::string& currentArg, size_t& idx,
-    const std::vector<std::string>& allArgs) override;
+  int ProcessCommandLineArguments(const std::string& currentArg, size_t& idx,
+                                  const std::vector<std::string>& allArgs,
+                                  bool& validArg) override;
 
   /*
    * Get the output variable
diff --git a/Source/CTest/cmCTestGenericHandler.h b/Source/CTest/cmCTestGenericHandler.h
index a0197d6..a189d60 100644
--- a/Source/CTest/cmCTestGenericHandler.h
+++ b/Source/CTest/cmCTestGenericHandler.h
@@ -48,7 +48,7 @@ public:
    */
   virtual int ProcessCommandLineArguments(
     const std::string& /*currentArg*/, size_t& /*idx*/,
-    const std::vector<std::string>& /*allArgs*/)
+    const std::vector<std::string>& /*allArgs*/, bool& /*valid*/)
   {
     return 1;
   }
diff --git a/Source/CTest/cmCTestSubmitHandler.cxx b/Source/CTest/cmCTestSubmitHandler.cxx
index 62f03c1..e69a7fe 100644
--- a/Source/CTest/cmCTestSubmitHandler.cxx
+++ b/Source/CTest/cmCTestSubmitHandler.cxx
@@ -140,13 +140,14 @@ void cmCTestSubmitHandler::Initialize()
 
 int cmCTestSubmitHandler::ProcessCommandLineArguments(
   const std::string& currentArg, size_t& idx,
-  const std::vector<std::string>& allArgs)
+  const std::vector<std::string>& allArgs, bool& validArg)
 {
   if (cmHasLiteralPrefix(currentArg, "--http-header") &&
       idx < allArgs.size() - 1) {
     ++idx;
     this->HttpHeaders.push_back(allArgs[idx]);
     this->CommandLineHttpHeaders.push_back(allArgs[idx]);
+    validArg = true;
   }
   return 1;
 }
diff --git a/Source/CTest/cmCTestSubmitHandler.h b/Source/CTest/cmCTestSubmitHandler.h
index e8eb38a..d152b71 100644
--- a/Source/CTest/cmCTestSubmitHandler.h
+++ b/Source/CTest/cmCTestSubmitHandler.h
@@ -35,9 +35,9 @@ public:
   void Initialize() override;
 
   //! Set all the submit arguments
-  int ProcessCommandLineArguments(
-    const std::string& currentArg, size_t& idx,
-    const std::vector<std::string>& allArgs) override;
+  int ProcessCommandLineArguments(const std::string& currentArg, size_t& idx,
+                                  const std::vector<std::string>& allArgs,
+                                  bool& validArg) override;
 
   /** Specify a set of parts (by name) to submit.  */
   void SelectParts(std::set<cmCTest::Part> const& parts);
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index 1ae3cb5..29bf311 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -760,7 +760,9 @@ bool cmCTest::UpdateCTestConfiguration()
   }
   if (!this->GetCTestConfiguration("BuildDirectory").empty()) {
     this->Impl->BinaryDir = this->GetCTestConfiguration("BuildDirectory");
-    cmSystemTools::ChangeDirectory(this->Impl->BinaryDir);
+    if (this->Impl->TestDir.empty()) {
+      cmSystemTools::ChangeDirectory(this->Impl->BinaryDir);
+    }
   }
   this->Impl->TimeOut =
     std::chrono::seconds(atoi(this->GetCTestConfiguration("TimeOut").c_str()));
@@ -1890,6 +1892,7 @@ bool cmCTest::HandleCommandLineArguments(size_t& i,
                                          std::string& errormsg)
 {
   std::string arg = args[i];
+  cm::string_view noTestsPrefix = "--no-tests=";
   if (this->CheckArgument(arg, "-F"_s)) {
     this->Impl->Failover = true;
   } else if (this->CheckArgument(arg, "-j"_s, "--parallel")) {
@@ -2151,8 +2154,7 @@ bool cmCTest::HandleCommandLineArguments(size_t& i,
     this->SetOutputJUnitFileName(std::string(args[i]));
   }
 
-  cm::string_view noTestsPrefix = "--no-tests=";
-  if (cmHasPrefix(arg, noTestsPrefix)) {
+  else if (cmHasPrefix(arg, noTestsPrefix)) {
     cm::string_view noTestsMode =
       cm::string_view(arg).substr(noTestsPrefix.length());
     if (noTestsMode == "error") {
@@ -2260,6 +2262,8 @@ bool cmCTest::HandleCommandLineArguments(size_t& i,
   else if (this->CheckArgument(arg, "--rerun-failed"_s)) {
     this->GetTestHandler()->SetPersistentOption("RerunFailed", "true");
     this->GetMemCheckHandler()->SetPersistentOption("RerunFailed", "true");
+  } else {
+    return false;
   }
   return true;
 }
@@ -2309,7 +2313,7 @@ bool cmCTest::ColoredOutputSupportedByConsole()
 }
 
 // handle the -S -SR and -SP arguments
-void cmCTest::HandleScriptArguments(size_t& i, std::vector<std::string>& args,
+bool cmCTest::HandleScriptArguments(size_t& i, std::vector<std::string>& args,
                                     bool& SRArgumentSpecified)
 {
   std::string arg = args[i];
@@ -2324,8 +2328,8 @@ void cmCTest::HandleScriptArguments(size_t& i, std::vector<std::string>& args,
     }
   }
 
-  if (this->CheckArgument(arg, "-SR"_s, "--script-run") &&
-      i < args.size() - 1) {
+  else if (this->CheckArgument(arg, "-SR"_s, "--script-run") &&
+           i < args.size() - 1) {
     SRArgumentSpecified = true;
     this->Impl->RunConfigurationScript = true;
     i++;
@@ -2333,7 +2337,8 @@ void cmCTest::HandleScriptArguments(size_t& i, std::vector<std::string>& args,
     ch->AddConfigurationScript(args[i], true);
   }
 
-  if (this->CheckArgument(arg, "-S"_s, "--script") && i < args.size() - 1) {
+  else if (this->CheckArgument(arg, "-S"_s, "--script") &&
+           i < args.size() - 1) {
     this->Impl->RunConfigurationScript = true;
     i++;
     cmCTestScriptHandler* ch = this->GetScriptHandler();
@@ -2341,7 +2346,10 @@ void cmCTest::HandleScriptArguments(size_t& i, std::vector<std::string>& args,
     if (!SRArgumentSpecified) {
       ch->AddConfigurationScript(args[i], true);
     }
+  } else {
+    return false;
   }
+  return true;
 }
 
 bool cmCTest::AddVariableDefinition(const std::string& arg)
@@ -2734,16 +2742,18 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
   for (size_t i = 1; i < args.size(); ++i) {
     // handle the simple commandline arguments
     std::string errormsg;
-    if (!this->HandleCommandLineArguments(i, args, errormsg)) {
+    bool validArg = this->HandleCommandLineArguments(i, args, errormsg);
+    if (!validArg && !errormsg.empty()) {
       cmSystemTools::Error(errormsg);
       return 1;
     }
+    std::string arg = args[i];
 
     // handle the script arguments -S -SR -SP
-    this->HandleScriptArguments(i, args, SRArgumentSpecified);
+    validArg =
+      validArg || this->HandleScriptArguments(i, args, SRArgumentSpecified);
 
     // --dashboard: handle a request for a dashboard
-    std::string arg = args[i];
     if (this->CheckArgument(arg, "-D"_s, "--dashboard") &&
         i < args.size() - 1) {
       this->Impl->ProduceXML = true;
@@ -2757,6 +2767,7 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
           executeTests = false;
         }
       }
+      validArg = true;
     }
 
     // If it's not exactly -D, but it starts with -D, then try to parse out
@@ -2767,16 +2778,17 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
     if (arg != "-D" && cmHasLiteralPrefix(arg, "-D")) {
       std::string input = arg.substr(2);
       this->AddVariableDefinition(input);
+      validArg = true;
     }
 
     // --test-action: calls SetTest(<stage>, /*report=*/ false) to enable
     // the corresponding stage
-    if (!this->HandleTestActionArgument(ctestExec, i, args)) {
+    if (!this->HandleTestActionArgument(ctestExec, i, args, validArg)) {
       executeTests = false;
     }
 
     // --test-model: what type of test model
-    if (!this->HandleTestModelArgument(ctestExec, i, args)) {
+    if (!this->HandleTestModelArgument(ctestExec, i, args, validArg)) {
       executeTests = false;
     }
 
@@ -2788,30 +2800,39 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
       if (!this->SubmitExtraFiles(args[i])) {
         return 0;
       }
+      validArg = true;
     }
 
     // --build-and-test options
     if (this->CheckArgument(arg, "--build-and-test"_s) &&
         i < args.size() - 1) {
       cmakeAndTest = true;
+      validArg = true;
     }
 
     // --schedule-random
     if (this->CheckArgument(arg, "--schedule-random"_s)) {
       this->Impl->ScheduleType = "Random";
+      validArg = true;
     }
 
     // pass the argument to all the handlers as well, but it may no longer be
     // set to what it was originally so I'm not sure this is working as
     // intended
     for (auto& handler : this->Impl->GetTestingHandlers()) {
-      if (!handler->ProcessCommandLineArguments(arg, i, args)) {
+      if (!handler->ProcessCommandLineArguments(arg, i, args, validArg)) {
         cmCTestLog(
           this, ERROR_MESSAGE,
           "Problem parsing command line arguments within a handler\n");
         return 0;
       }
     }
+
+    if (!validArg && cmHasLiteralPrefix(arg, "-") &&
+        !cmHasLiteralPrefix(arg, "--preset")) {
+      cmSystemTools::Error(cmStrCat("Invalid argument: ", arg));
+      return 1;
+    }
   } // the close of the for argument loop
 
   // handle CTEST_PARALLEL_LEVEL environment variable
@@ -2877,12 +2898,14 @@ int cmCTest::Run(std::vector<std::string>& args, std::string* output)
 }
 
 bool cmCTest::HandleTestActionArgument(const char* ctestExec, size_t& i,
-                                       const std::vector<std::string>& args)
+                                       const std::vector<std::string>& args,
+                                       bool& validArg)
 {
   bool success = true;
   std::string const& arg = args[i];
   if (this->CheckArgument(arg, "-T"_s, "--test-action") &&
       (i < args.size() - 1)) {
+    validArg = true;
     this->Impl->ProduceXML = true;
     i++;
     if (!this->SetTest(args[i], false)) {
@@ -2909,12 +2932,14 @@ bool cmCTest::HandleTestActionArgument(const char* ctestExec, size_t& i,
 }
 
 bool cmCTest::HandleTestModelArgument(const char* ctestExec, size_t& i,
-                                      const std::vector<std::string>& args)
+                                      const std::vector<std::string>& args,
+                                      bool& validArg)
 {
   bool success = true;
   std::string const& arg = args[i];
   if (this->CheckArgument(arg, "-M"_s, "--test-model") &&
       (i < args.size() - 1)) {
+    validArg = true;
     i++;
     std::string const& str = args[i];
     if (cmSystemTools::LowerCase(str) == "nightly"_s) {
diff --git a/Source/cmCTest.h b/Source/cmCTest.h
index 2ab810c..ae2431c 100644
--- a/Source/cmCTest.h
+++ b/Source/cmCTest.h
@@ -512,7 +512,7 @@ private:
   static bool ColoredOutputSupportedByConsole();
 
   /** handle the -S -SP and -SR arguments */
-  void HandleScriptArguments(size_t& i, std::vector<std::string>& args,
+  bool HandleScriptArguments(size_t& i, std::vector<std::string>& args,
                              bool& SRArgumentSpecified);
 
   /** Reread the configuration file */
@@ -531,11 +531,13 @@ private:
 
   /** Handle the --test-action command line argument */
   bool HandleTestActionArgument(const char* ctestExec, size_t& i,
-                                const std::vector<std::string>& args);
+                                const std::vector<std::string>& args,
+                                bool& validArg);
 
   /** Handle the --test-model command line argument */
   bool HandleTestModelArgument(const char* ctestExec, size_t& i,
-                               const std::vector<std::string>& args);
+                               const std::vector<std::string>& args,
+                               bool& validArg);
 
   int RunCMakeAndTest(std::string* output);
   int ExecuteTests();
diff --git a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake
index e56a4cb..bb6db23 100644
--- a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake
+++ b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake
@@ -566,6 +566,8 @@ set_tests_properties(test5 PROPERTIES  SKIP_REGULAR_EXPRESSION \"please skip\")
 endfunction()
 run_output_junit()
 
+run_cmake_command(invalid-ctest-argument ${CMAKE_CTEST_COMMAND} --not-a-valid-ctest-argument)
+
 if(WIN32)
   block()
     set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/TimeoutSignalWindows)
diff --git a/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-result.txt b/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-stderr.txt b/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-stderr.txt
new file mode 100644
index 0000000..4de2e77
--- /dev/null
+++ b/Tests/RunCMake/CTestCommandLine/invalid-ctest-argument-stderr.txt
@@ -0,0 +1 @@
+CMake Error: Invalid argument: --not-a-valid-ctest-argument