[GitHub] [geode] jchen21 commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

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

jchen21 commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500475855



##########
File path: geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java
##########
@@ -50,127 +48,146 @@
 
 @Category({GfshTest.class, LoggingTest.class})
 public class LogExporterIntegrationTest {
+  @Rule
+  public ServerStarterRule server = new ServerStarterRule();
 
   private final LogFilter filter = new LogFilter(Level.INFO, null, null);
 
-  private LogExporter logExporter;
-  private Properties properties;
-
-  @Rule
-  public ServerStarterRule server = new ServerStarterRule();
+  public Path serverFilesDir;
 
   @Before
-  public void before() {
-    properties = new Properties();
-    // make sure the server's working dir has no log files or stats file to begin with, since in
-    // some tests we are asserting on the # of log files and stats files created by the server
-    File workingDir = server.getWorkingDir();
-    Arrays.stream(workingDir.listFiles())
-        .filter(f -> (f.getName().endsWith(".log") || f.getName().endsWith(".gfs")))
-        .forEach(FileUtils::deleteQuietly);
+  public void createServerFilesDir() throws IOException {
+    // Name the directory after this test instance and the Gradle test worker, to ensure that tests
+    // running in parallel use different directories.
+    String testRunnerID = System.getProperty("org.gradle.test.worker", "standalone");
+    int testInstanceID = System.identityHashCode(this);
+    String className = getClass().getSimpleName();
+    String dirName = String.format("%s-%x-%s", className, testInstanceID, testRunnerID);
+    serverFilesDir = Files.createDirectories(Paths.get(dirName)).normalize().toAbsolutePath();
+  }
+
+  @After
+  public void deleteServerFilesDir() {
+    FileUtils.deleteQuietly(serverFilesDir.toFile());
   }
 
   @Test
   public void serverStartedWithWrongSuffix() throws Exception {
-    properties.setProperty(LOG_FILE, new File("test.txt").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.archive");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "stats.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).isEmpty();
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).isEmpty();
+    String logFileNameWithWrongSuffix = "test.txt";
+    String statsFileNameWithWrongSuffix = "archive.archive";
+
+    Path logFile = serverFilesDir.resolve(logFileNameWithWrongSuffix);
+    Path statsFile = serverFilesDir.resolve(statsFileNameWithWrongSuffix);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .isEmpty();
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .isEmpty();
   }
 
   @Test
   public void serverStartedWithCorrectSuffix() throws Exception {
-    // ("relative log file is problematic in the test environment")
-    properties.setProperty(LOG_FILE, new File("test.log").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.gfs");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "archive.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).hasSize(1);
-    assertThat(logFiles.get(0)).hasFileName("test.log");
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).hasSize(1);
-    assertThat(statsFiles.get(0)).hasFileName("archive.gfs");
+    String logFileName = "test.log";
+    String statsFileName = "archive.gfs";
+    Path logFile = serverFilesDir.resolve(logFileName);
+    Path statsFile = serverFilesDir.resolve(statsFileName);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .hasSize(1);
+    assertThat(logFiles.get(0)).hasFileName(logFileName);
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .hasSize(1);
+    assertThat(statsFiles.get(0)).hasFileName(statsFileName);
   }
 
   @Test
   @Ignore("GEODE-2574: fix .gz suffix")

Review comment:
       Not sure why this test is @Ignore. Any change to the test is not executed.

##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       By default, this follows the symbolic link. Just make sure this is expected.




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