[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #820: MINIFICPP-1183 - Cleanup C2 Update tests

[GitBox <gi...@apache.org>]

arpadboda commented on a change in pull request #820:
URL: https://github.com/apache/nifi-minifi-cpp/pull/820#discussion_r450837057



##########
File path: extensions/coap/tests/CoapIntegrationBase.h
##########
@@ -27,8 +27,7 @@ int log_message(const struct mg_connection *conn, const char *message) {
   return 1;
 }
 
-int ssl_enable(void *ssl_context, void *user_data) {
-  struct ssl_ctx_st *ctx = (struct ssl_ctx_st *) ssl_context;
+int ssl_enable(void *ssl_context, void *) {

Review comment:
       I think the unused argument here is going to generate a warning.

##########
File path: extensions/http-curl/tests/HttpGetIntegrationTest.cpp
##########
@@ -17,46 +17,31 @@
  */
 
 #define CURLOPT_SSL_VERIFYPEER_DISABLE 1
-#include <sys/stat.h>
 #undef NDEBUG
 #include <cassert>
 #include <utility>
 #include <chrono>
-#include <fstream>
 #include <memory>
 #include <string>
 #include <thread>
-#include <type_traits>
 #include <vector>
 #include "HTTPClient.h"
 #include "InvokeHTTP.h"
 #include "TestServer.h"
 #include "TestBase.h"
 #include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
 #include "core/yaml/YamlConfiguration.h"
 #include "FlowController.h"
 #include "properties/Configure.h"
 #include "unit/ProvenanceTestHelper.h"
 #include "io/StreamFactory.h"
-#include "processors/InvokeHTTP.h"
-#include "processors/ListenHTTP.h"
 #include "processors/LogAttribute.h"
+#include "HTTPIntegrationBase.h"
 
-void waitToVerifyProcessor() {
-  std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-int log_message(const struct mg_connection *conn, const char *message) {
-  puts(message);
-  return 1;
-}
-
-int ssl_enable(void* /*ssl_context*/, void* /*user_data*/) {
-  puts("Enable ssl");
-  return 0;
+namespace {
+  void waitToVerifyProcessor() {
+    std::this_thread::sleep_for(std::chrono::seconds(10));

Review comment:
       I would move that to the scope of a follow-up ticket, our tests definitely need improvement in this aspect, and this is not the only one. 

##########
File path: extensions/http-curl/tests/C2NullConfiguration.cpp
##########
@@ -74,23 +53,23 @@ class VerifyC2Server : public CoapIntegrationBase {
     file.close();
   }
 
-  void cleanup() {
+  void cleanup() override {
     unlink(ss.str().c_str());

Review comment:
       Do we need it? It's created in a tempdir, which is created by the testcontroller, I suppose this should be removed automatically. 

##########
File path: extensions/http-curl/tests/TimeoutHTTPSiteToSiteTests.cpp
##########
@@ -152,25 +137,18 @@ void run_timeout_variance(std::string test_file_location, bool isSecure, std::st
 
   harness.run(test_file_location);
 
-  assert(LogTestController::getInstance().contains("limit (200ms) reached, terminating connection") == true);
+  assert(LogTestController::getInstance().contains("limit (200ms) reached, terminating connection"));
 
   LogTestController::getInstance().reset();
 }
 
 int main(int argc, char **argv) {
   transaction_id = 0;
   transaction_id_output = 0;
-  std::string key_dir, test_file_location, url;
-  if (argc > 1) {
-    test_file_location = argv[1];
-    key_dir = argv[2];
-    url = argv[3];
-  }
+  const cmd_args args = parse_cmdline_args_with_url(argc, argv);
 
-  bool isSecure = false;
-  if (url.find("https") != std::string::npos) {
-    isSecure = true;
-  }
+  // check https prefix
+  const bool isSecure = args.url.rfind("https://", 0) == 0;

Review comment:
       This could be a member function of args or some other helper function. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org