fgerlits commented on a change in pull request #1040:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1040#discussion_r605441084



##########
File path: extensions/standard-processors/tests/unit/GetFileTests.cpp
##########
@@ -100,3 +122,17 @@ TEST_CASE("GetFile: Directory", "[getFileDir]") {
   auto get_file = plan->addProcessor("GetFile", "Get");
   REQUIRE_THROWS_AS(plan->runNextProcessor(), minifi::Exception&);
 }
+
+TEST_CASE("GetFileHiddenPropertyCheck", "[getFileProperty]") {
+  TestController testController;
+  LogTestController::getInstance().setTrace<TestPlan>();
+  LogTestController::getInstance().setTrace<processors::GetFile>();
+  LogTestController::getInstance().setTrace<processors::LogAttribute>();
+  auto plan = testController.createPlan();
+  auto repo = std::make_shared<TestRepository>();
+
+  auto get_file2 = plan->addProcessor("GetFile", "Get");
+  plan->setProperty(get_file2, 
processors::GetFile::IgnoreHiddenFile.getName(), "false");
+  checkLoggedFiles(testController, plan, get_file2);

Review comment:
       I guess the idea was to factor out the common code from the two test 
cases, but `checkLoggedFiles` is only used here.
   
   It would be better to either complete the plan, i.e. have a function which 
contains the common part of the tests and is called from both test cases, or 
inline `checkLoggedFiles` into this test case.

##########
File path: extensions/standard-processors/tests/unit/GetFileTests.cpp
##########
@@ -100,3 +122,17 @@ TEST_CASE("GetFile: Directory", "[getFileDir]") {
   auto get_file = plan->addProcessor("GetFile", "Get");
   REQUIRE_THROWS_AS(plan->runNextProcessor(), minifi::Exception&);
 }
+
+TEST_CASE("GetFileHiddenPropertyCheck", "[getFileProperty]") {
+  TestController testController;
+  LogTestController::getInstance().setTrace<TestPlan>();
+  LogTestController::getInstance().setTrace<processors::GetFile>();
+  LogTestController::getInstance().setTrace<processors::LogAttribute>();
+  auto plan = testController.createPlan();
+  auto repo = std::make_shared<TestRepository>();

Review comment:
       `repo` is not used anywhere

##########
File path: extensions/standard-processors/processors/PutFile.h
##########
@@ -79,6 +79,7 @@ class PutFile : public core::Processor {
 
   void onTrigger(core::ProcessContext *context, core::ProcessSession *session) 
override;
   void initialize() override;
+  friend class PutFileTestAccessor;

Review comment:
       `PutFileTestAccessor` does not exist, so this line can be deleted

##########
File path: extensions/standard-processors/tests/unit/PutFileTests.cpp
##########
@@ -467,4 +467,39 @@ TEST_CASE("TestPutFilePermissions", 
"[PutFilePermissions]") {
   REQUIRE(utils::file::FileUtils::get_permissions(putfiledir, perms));
   REQUIRE(perms == 0777);
 }
+
+TEST_CASE("PutFileCreateDirectoryTest", "[PutFilePermissions]") {

Review comment:
       please change the tag, as this test has nothing to do with permissions

##########
File path: libminifi/test/KamikazeProcessor.cpp
##########
@@ -53,14 +53,13 @@ void KamikazeProcessor::initialize() {
 
 void KamikazeProcessor::onSchedule(core::ProcessContext *context, 
core::ProcessSessionFactory* /*sessionFactory*/) {
   std::string value;
-  bool bool_value;
-
   _throwInOnTrigger = false;

Review comment:
       this line can be deleted, too

##########
File path: libminifi/src/RemoteProcessorGroupPort.cpp
##########
@@ -159,9 +160,7 @@ void RemoteProcessorGroupPort::onSchedule(const 
std::shared_ptr<core::ProcessCon
     ssl_service = 
std::static_pointer_cast<minifi::controllers::SSLContextService>(service);
   } else {
     std::string secureStr;
-    bool is_secure = false;
-    if (configure_->get(Configure::nifi_remote_input_secure, secureStr) &&
-        org::apache::nifi::minifi::utils::StringUtils::StringToBool(secureStr, 
is_secure)) {
+    if (configure_->get(Configure::nifi_remote_input_secure, secureStr) && 
org::apache::nifi::minifi::utils::StringUtils::toBool(secureStr).value_or(false))
 {

Review comment:
       minor, but `org::apache::nifi::minifi::` is not needed

##########
File path: extensions/standard-processors/tests/unit/PutFileTests.cpp
##########
@@ -467,4 +467,39 @@ TEST_CASE("TestPutFilePermissions", 
"[PutFilePermissions]") {
   REQUIRE(utils::file::FileUtils::get_permissions(putfiledir, perms));
   REQUIRE(perms == 0777);
 }
+
+TEST_CASE("PutFileCreateDirectoryTest", "[PutFilePermissions]") {
+  TestController testController;
+  LogTestController::getInstance().setDebug<minifi::processors::GetFile>();
+  LogTestController::getInstance().setDebug<TestPlan>();
+  LogTestController::getInstance().setDebug<minifi::processors::PutFile>();
+  
LogTestController::getInstance().setDebug<minifi::processors::PutFile::ReadCallback>();
+  
LogTestController::getInstance().setDebug<minifi::processors::LogAttribute>();
+
+  std::shared_ptr<TestPlan> plan = testController.createPlan();
+  std::shared_ptr<core::Processor> getfile = plan->addProcessor("GetFile", 
"getfileCreate2");
+  std::shared_ptr<core::Processor> putfile = plan->addProcessor("PutFile", 
"putfile", core::Relationship("success", "description"), true);
+  plan->addProcessor("LogAttribute", "logattribute", 
core::Relationship("success", "description"), true);
+
+  // Define Directory
+  char format[] = "/tmp/gt.XXXXXX";
+  auto dir = testController.createTempDirectory(format);
+  char format2[] = "/tmp/ft.XXXXXX";
+  // Defining a sub directory
+  auto putfiledir = testController.createTempDirectory(format2) + 
utils::file::FileUtils::get_separator() + "test_dir";
+  plan->setProperty(getfile, 
org::apache::nifi::minifi::processors::GetFile::Directory.getName(), dir);
+  plan->setProperty(putfile, 
org::apache::nifi::minifi::processors::PutFile::Directory.getName(), 
putfiledir);
+
+  plan->setProperty(putfile, 
org::apache::nifi::minifi::processors::PutFile::CreateDirs.getName(), "true");

Review comment:
       It would be good to test the other case, too, when `CreateDirs` is false 
so no output file is created.




-- 
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:
[email protected]


Reply via email to