[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1217: MINIFICPP-1555 Improve the coverage of GetFileTests

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

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



##########
File path: extensions/standard-processors/tests/unit/GetFileTests.cpp
##########
@@ -140,37 +142,103 @@ TEST_CASE("GetFile keeps the source file if KeepSourceFile is true") {
 
   test_controller.runSession();
 
-  REQUIRE(utils::file::FileUtils::exists(test_controller.input_file_name_));
+  REQUIRE(utils::file::FileUtils::exists(test_controller.getInputFilePath()));
 }
 
-TEST_CASE("GetFileHiddenPropertyCheck", "[getFileProperty]") {
-  TestController testController;
-  LogTestController::getInstance().setTrace<TestPlan>();
-  LogTestController::getInstance().setTrace<minifi::processors::GetFile>();
-  LogTestController::getInstance().setTrace<minifi::processors::LogAttribute>();
-  auto plan = testController.createPlan();
+TEST_CASE("Hidden files are read when IgnoreHiddenFile property is false", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::IgnoreHiddenFile, "false");
+
+  test_controller.runSession();
+
+  REQUIRE(LogTestController::getInstance().contains("Logged 3 flow files"));
+}
+
+TEST_CASE("Check if subdirectories are ignored or not if Recurse property is set", "[getFileProperty]") {
+  GetFileTestController test_controller;
+
+  auto subdir_path = test_controller.getFullPath("subdir");
+  utils::file::FileUtils::create_dir(subdir_path);
+  utils::putFileToDir(subdir_path, "subfile.txt", "Some content in a subfile\n");
+
+  SECTION("File in subdirectory is ignored when Recurse property set to false")  {
+    test_controller.setProperty(minifi::processors::GetFile::Recurse, "false");
+    test_controller.runSession();
+
+    REQUIRE(LogTestController::getInstance().contains("Logged 2 flow files"));
+  }
+
+  SECTION("File in subdirectory is logged when Recurse property set to true")  {
+    test_controller.setProperty(minifi::processors::GetFile::Recurse, "true");
+    test_controller.runSession();
+
+    REQUIRE(LogTestController::getInstance().contains("Logged 3 flow files"));
+  }
+}
+
+TEST_CASE("Only older files are read when MinAge property is set", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::MinAge, "1 hour");
+
+  utils::file::FileUtils::set_last_write_time(test_controller.getInputFilePath(), 936860949);
+
+  test_controller.runSession();
+
+  REQUIRE(LogTestController::getInstance().contains("Logged 1 flow files"));
+  REQUIRE(LogTestController::getInstance().contains("Size:44 Offset:0"));
+}
 
-  auto temp_path = testController.createTempDirectory();
-  std::string in_file(temp_path + utils::file::FileUtils::get_separator() + "testfifo");
-  std::string hidden_in_file(temp_path + utils::file::FileUtils::get_separator() + ".testfifo");
+TEST_CASE("Only newer files are read when MaxAge property is set", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::MaxAge, "1 hour");
+
+  // Set last write time to year 1999
+  utils::file::FileUtils::set_last_write_time(test_controller.getInputFilePath(), 936860949);
 
-  auto get_file = plan->addProcessor("GetFile", "Get");
-  plan->setProperty(get_file, minifi::processors::GetFile::IgnoreHiddenFile.getName(), "false");
+  test_controller.runSession();
 
-  plan->setProperty(get_file, minifi::processors::GetFile::Directory.getName(), temp_path);
-  auto log_attr = plan->addProcessor("LogAttribute", "Log", core::Relationship("success", "description"), true);
-  plan->setProperty(log_attr, minifi::processors::LogAttribute::FlowFilesToLog.getName(), "0");
+  REQUIRE(LogTestController::getInstance().contains("Logged 1 flow files"));
+  REQUIRE(LogTestController::getInstance().contains("Size:67 Offset:0"));
+}
 
-  std::ofstream in_file_stream(in_file);
-  in_file_stream << "This file is not hidden" << std::endl;
-  in_file_stream.close();
+TEST_CASE("Test BatchSize property for the maximum number of files read at once", "[getFileProperty]") {
+  GetFileTestController test_controller;
 
-  std::ofstream hidden_in_file_stream(hidden_in_file);
-  hidden_in_file_stream << "This file is hidden" << std::endl;
-  hidden_in_file_stream.close();
+  SECTION("BatchSize is set to 1 so only 1 file should be logged")  {
+    test_controller.setProperty(minifi::processors::GetFile::BatchSize, "1");
+    test_controller.runSession();
+    REQUIRE(LogTestController::getInstance().contains("Logged 1 flow files"));
+  }
+
+  SECTION("BatchSize is set to 5 so all 2 non-hidden files should be logged")  {
+    test_controller.setProperty(minifi::processors::GetFile::BatchSize, "5");
+    test_controller.runSession();
+    REQUIRE(LogTestController::getInstance().contains("Logged 2 flow files"));
+  }
+}
 
-  plan->runNextProcessor();
-  plan->runNextProcessor();
+TEST_CASE("Test file filtering of GetFile", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::FileFilter, ".?test\\.txt$");
+  test_controller.setProperty(minifi::processors::GetFile::IgnoreHiddenFile, "false");
+
+  test_controller.runSession();
 
   REQUIRE(LogTestController::getInstance().contains("Logged 2 flow files"));

Review comment:
       I think this test, and some of the others, too, would be clearer if we also checked the file names of the files expected to be read, eg:
   ```suggestion
     REQUIRE(LogTestController::getInstance().contains("Logged 2 flow files"));
     REQUIRE(LogTestController::getInstance().contains("key:filename value:test.txt"));
     REQUIRE(LogTestController::getInstance().contains("key:filename value:.test.txt"));
   ```

##########
File path: extensions/standard-processors/tests/unit/GetFileTests.cpp
##########
@@ -140,37 +142,103 @@ TEST_CASE("GetFile keeps the source file if KeepSourceFile is true") {
 
   test_controller.runSession();
 
-  REQUIRE(utils::file::FileUtils::exists(test_controller.input_file_name_));
+  REQUIRE(utils::file::FileUtils::exists(test_controller.getInputFilePath()));
 }
 
-TEST_CASE("GetFileHiddenPropertyCheck", "[getFileProperty]") {
-  TestController testController;
-  LogTestController::getInstance().setTrace<TestPlan>();
-  LogTestController::getInstance().setTrace<minifi::processors::GetFile>();
-  LogTestController::getInstance().setTrace<minifi::processors::LogAttribute>();
-  auto plan = testController.createPlan();
+TEST_CASE("Hidden files are read when IgnoreHiddenFile property is false", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::IgnoreHiddenFile, "false");
+
+  test_controller.runSession();
+
+  REQUIRE(LogTestController::getInstance().contains("Logged 3 flow files"));
+}
+
+TEST_CASE("Check if subdirectories are ignored or not if Recurse property is set", "[getFileProperty]") {
+  GetFileTestController test_controller;
+
+  auto subdir_path = test_controller.getFullPath("subdir");
+  utils::file::FileUtils::create_dir(subdir_path);
+  utils::putFileToDir(subdir_path, "subfile.txt", "Some content in a subfile\n");
+
+  SECTION("File in subdirectory is ignored when Recurse property set to false")  {
+    test_controller.setProperty(minifi::processors::GetFile::Recurse, "false");
+    test_controller.runSession();
+
+    REQUIRE(LogTestController::getInstance().contains("Logged 2 flow files"));
+  }
+
+  SECTION("File in subdirectory is logged when Recurse property set to true")  {
+    test_controller.setProperty(minifi::processors::GetFile::Recurse, "true");
+    test_controller.runSession();
+
+    REQUIRE(LogTestController::getInstance().contains("Logged 3 flow files"));
+  }
+}
+
+TEST_CASE("Only older files are read when MinAge property is set", "[getFileProperty]") {
+  GetFileTestController test_controller;
+  test_controller.setProperty(minifi::processors::GetFile::MinAge, "1 hour");
+
+  utils::file::FileUtils::set_last_write_time(test_controller.getInputFilePath(), 936860949);

Review comment:
       I would use some expression rather that a magic constant, eg:
   ```suggestion
     using namespace std::chrono;
     const auto more_than_an_hour_ago = system_clock::now() - 65min;
     utils::file::FileUtils::set_last_write_time(test_controller.getInputFilePath(), duration_cast<seconds>(more_than_an_hour_ago.time_since_epoch()).count());
   ```
   (same in the MaxAge test)




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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