ctest_submit: Make CDASH_UPLOAD mode arguments more strict

Disallow mixing of arguments from different command signatures.
Extend the RunCMake.CTestSubmit test to cover such error cases.

Help/command/ctest_submit.rst

@@ -38,13 +38,15 @@ timed-out submission before attempting to re-submit.
 The RETRY_COUNT option specifies how many times to retry a timed-out
 submission.
 
+Submit to CDash Upload API
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 ::
 
-  ctest_submit([CDASH_UPLOAD file]
+  ctest_submit(CDASH_UPLOAD <file> [CDASH_UPLOAD_TYPE <type>])
-               [CDASH_UPLOAD_TYPE type_string])
 
 This second signature is used to upload files to CDash via the CDash
 file upload API. The api first sends a request to upload to CDash along
-with the md5 sum of the file. If CDash does not already have the file,
+with a content hash of the file. If CDash does not already have the file,
 then it is uploaded. Along with the file, a CDash type string is specified
 to tell CDash which handler to use to process the data.

Source/CTest/cmCTestSubmitCommand.cxx

@@ -145,7 +145,7 @@ cmCTestGenericHandler* cmCTestSubmitCommand::InitializeHandler()
   static_cast<cmCTestSubmitHandler*>(handler)->SetOption("InternalTest",
     this->InternalTest ? "ON" : "OFF");
 
-  if(this->CDashUploadFile.size())
+  if (this->CDashUpload)
     {
     static_cast<cmCTestSubmitHandler*>(handler)->
       SetOption("CDashUploadFile", this->CDashUploadFile.c_str());
@@ -155,51 +155,65 @@ cmCTestGenericHandler* cmCTestSubmitCommand::InitializeHandler()
   return handler;
 }
 
+//----------------------------------------------------------------------------
+bool cmCTestSubmitCommand::InitialPass(std::vector<std::string> const& args,
+                                       cmExecutionStatus& status)
+{
+  this->CDashUpload = !args.empty() && args[0] == "CDASH_UPLOAD";
+  return this->cmCTestHandlerCommand::InitialPass(args, status);
+}
 
 //----------------------------------------------------------------------------
 bool cmCTestSubmitCommand::CheckArgumentKeyword(std::string const& arg)
 {
-  // Look for arguments specific to this command.
+  if (this->CDashUpload)
-  if(arg == "PARTS")
     {
-    this->ArgumentDoing = ArgumentDoingParts;
+    if(arg == "CDASH_UPLOAD")
-    this->PartsMentioned = true;
+      {
-    return true;
+      this->ArgumentDoing = ArgumentDoingCDashUpload;
-    }
+      return true;
+      }
 
-  if(arg == "FILES")
+    if(arg == "CDASH_UPLOAD_TYPE")
-    {
+      {
-    this->ArgumentDoing = ArgumentDoingFiles;
+      this->ArgumentDoing = ArgumentDoingCDashUploadType;
-    this->FilesMentioned = true;
+      return true;
-    return true;
+      }
     }
+  else
+    {
+    // Look for arguments specific to this command.
+    if(arg == "PARTS")
+      {
+      this->ArgumentDoing = ArgumentDoingParts;
+      this->PartsMentioned = true;
+      return true;
+      }
 
-  if(arg == "RETRY_COUNT")
+    if(arg == "FILES")
-    {
+      {
-    this->ArgumentDoing = ArgumentDoingRetryCount;
+      this->ArgumentDoing = ArgumentDoingFiles;
-    return true;
+      this->FilesMentioned = true;
-    }
+      return true;
+      }
 
-  if(arg == "RETRY_DELAY")
+    if(arg == "RETRY_COUNT")
-    {
+      {
-    this->ArgumentDoing = ArgumentDoingRetryDelay;
+      this->ArgumentDoing = ArgumentDoingRetryCount;
-    return true;
+      return true;
-    }
+      }
 
-  if(arg == "CDASH_UPLOAD")
+    if(arg == "RETRY_DELAY")
-    {
+      {
-    this->ArgumentDoing = ArgumentDoingCDashUpload;
+      this->ArgumentDoing = ArgumentDoingRetryDelay;
-    return true;
+      return true;
-    }
+      }
-  if(arg == "CDASH_UPLOAD_TYPE")
+
-    {
+    if(arg == "INTERNAL_TEST_CHECKSUM")
-    this->ArgumentDoing = ArgumentDoingCDashUploadType;
+      {
-    return true;
+      this->InternalTest = true;
-    }
+      return true;
-  if(arg == "INTERNAL_TEST_CHECKSUM")
+      }
-    {
-    this->InternalTest = true;
-    return true;
     }
 
   // Look for other arguments.
@@ -260,11 +274,14 @@ bool cmCTestSubmitCommand::CheckArgumentValue(std::string const& arg)
 
   if(this->ArgumentDoing == ArgumentDoingCDashUpload)
     {
+    this->ArgumentDoing = ArgumentDoingNone;
     this->CDashUploadFile = arg;
     return true;
     }
+
   if(this->ArgumentDoing == ArgumentDoingCDashUploadType)
     {
+    this->ArgumentDoing = ArgumentDoingNone;
     this->CDashUploadType = arg;
     return true;
     }

Source/CTest/cmCTestSubmitCommand.h

@@ -32,6 +32,7 @@ public:
     this->InternalTest = false;
     this->RetryCount = "";
     this->RetryDelay = "";
+    this->CDashUpload = false;
     }
 
   /**
@@ -45,6 +46,9 @@ public:
     return ni;
     }
 
+  virtual bool InitialPass(std::vector<std::string> const& args,
+                           cmExecutionStatus &status);
+
   /**
    * The name of the command as specified in CMakeList.txt.
    */
@@ -76,6 +80,7 @@ protected:
   cmCTest::SetOfStrings Files;
   std::string RetryCount;
   std::string RetryDelay;
+  bool CDashUpload;
   std::string CDashUploadFile;
   std::string CDashUploadType;
 };

Source/CTest/cmCTestSubmitHandler.cxx

@@ -1084,10 +1084,16 @@ void cmCTestSubmitHandler::ConstructCDashURL(std::string& dropMethod,
 int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
                                                 std::string const& typeString)
 {
-  if(!cmSystemTools::FileExists(file))
+  if (file.empty())
     {
     cmCTestLog(this->CTest, ERROR_MESSAGE,
-               "Upload file not found: " << file << "\n");
+               "Upload file not specified\n");
+    return -1;
+    }
+  if (!cmSystemTools::FileExists(file))
+    {
+    cmCTestLog(this->CTest, ERROR_MESSAGE,
+               "Upload file not found: '" << file << "'\n");
     return -1;
     }
   cmCTestCurl curl(this->CTest);
@@ -1118,6 +1124,7 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file,
     static_cast<cmCTestScriptHandler*>(this->CTest->GetHandler("script"));
   cmake* cm =  ch->GetCMake();
   const char* subproject = cm->GetProperty("SubProject", cmProperty::GLOBAL);
+  // TODO: Encode values for a URL instead of trusting caller.
   std::ostringstream str;
   str << "project="
       << this->CTest->GetCTestConfiguration("ProjectName") << "&";
@@ -1214,8 +1221,7 @@ int cmCTestSubmitHandler::ProcessHandler()
   const char* cdashUploadType = this->GetOption("CDashUploadType");
   if(cdashUploadFile && cdashUploadType)
     {
-    return this->HandleCDashUploadFile(std::string(cdashUploadFile),
+    return this->HandleCDashUploadFile(cdashUploadFile, cdashUploadType);
-                                       std::string(cdashUploadType));
     }
   std::string iscdash = this->CTest->GetCTestConfiguration("IsCDash");
   // cdash does not need to trigger so just return true

Tests/RunCMake/CTestSubmit/CDashUploadFILES-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadFILES-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/CDashUploadFILES/test.cmake:[0-9]+ \(ctest_submit\):
+  ctest_submit called with unknown argument "FILES".

Tests/RunCMake/CTestSubmit/CDashUploadFTP-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadFTP-stderr.txt

@@ -0,0 +1 @@
+Only http and https are supported for CDASH_UPLOAD

Tests/RunCMake/CTestSubmit/CDashUploadNone-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadNone-stderr.txt

@@ -0,0 +1 @@
+Upload file not specified

Tests/RunCMake/CTestSubmit/CDashUploadPARTS-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadPARTS-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/CDashUploadPARTS/test.cmake:[0-9]+ \(ctest_submit\):
+  ctest_submit called with unknown argument "PARTS".

Tests/RunCMake/CTestSubmit/CDashUploadRETRY_COUNT-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadRETRY_COUNT-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/CDashUploadRETRY_COUNT/test.cmake:[0-9]+ \(ctest_submit\):
+  ctest_submit called with unknown argument "RETRY_COUNT".

Tests/RunCMake/CTestSubmit/CDashUploadRETRY_DELAY-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/CDashUploadRETRY_DELAY-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/CDashUploadRETRY_DELAY/test.cmake:[0-9]+ \(ctest_submit\):
+  ctest_submit called with unknown argument "RETRY_DELAY".

Tests/RunCMake/CTestSubmit/PARTSCDashUpload-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/PARTSCDashUpload-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/PARTSCDashUpload/test.cmake:[0-9]+ \(ctest_submit\):
+  Part name "CDASH_UPLOAD" is invalid.

Tests/RunCMake/CTestSubmit/PARTSCDashUploadType-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestSubmit/PARTSCDashUploadType-stderr.txt

@@ -0,0 +1,2 @@
+CMake Error at .*/Tests/RunCMake/CTestSubmit/PARTSCDashUploadType/test.cmake:[0-9]+ \(ctest_submit\):
+  Part name "CDASH_UPLOAD_TYPE" is invalid.

Tests/RunCMake/CTestSubmit/RunCMakeTest.cmake

@@ -32,6 +32,19 @@ run_ctest_submit(BadArg bad-arg)
 run_ctest_submit(BadPARTS PARTS bad-part)
 run_ctest_submit(BadFILES FILES bad-file)
 run_ctest_submit(RepeatRETURN_VALUE RETURN_VALUE res RETURN_VALUE res)
+run_ctest_submit(PARTSCDashUpload PARTS Configure CDASH_UPLOAD)
+run_ctest_submit(PARTSCDashUploadType PARTS Configure CDASH_UPLOAD_TYPE)
+run_ctest_submit(CDashUploadPARTS CDASH_UPLOAD bad-upload PARTS)
+run_ctest_submit(CDashUploadFILES CDASH_UPLOAD bad-upload FILES)
+run_ctest_submit(CDashUploadRETRY_COUNT CDASH_UPLOAD bad-upload RETRY_COUNT)
+run_ctest_submit(CDashUploadRETRY_DELAY CDASH_UPLOAD bad-upload RETRY_DELAY)
+run_ctest_submit(CDashUploadNone CDASH_UPLOAD)
+
+function(run_ctest_CDashUploadFTP)
+  set(CASE_DROP_METHOD ftp)
+  run_ctest_submit(CDashUploadFTP CDASH_UPLOAD ${CMAKE_CURRENT_LIST_FILE})
+endfunction()
+run_ctest_CDashUploadFTP()
 
 #-----------------------------------------------------------------------------
 # Test failed drops by various protocols