[ https://issues.apache.org/jira/browse/GEODE-8572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17209045#comment-17209045 ]
ASF GitHub Bot commented on GEODE-8572: --------------------------------------- demery-pivotal commented on a change in pull request #5595: URL: https://github.com/apache/geode/pull/5595#discussion_r500530806 ########## 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: Though the test is ignored, I updated it to match the style of the tests I changed. I left it ignored because the system fails the 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > LogExporter throws if a directory matches its file selector > ----------------------------------------------------------- > > Key: GEODE-8572 > URL: https://issues.apache.org/jira/browse/GEODE-8572 > Project: Geode > Issue Type: Bug > Components: gfsh > Affects Versions: 1.13.0 > Reporter: Dale Emery > Priority: Major > Labels: GeodeOperationAPI, pull-request-available > > {{LogExporter}} tries to read and export any directory entry whose name is > accepted by its log file selector predicate. The predicate accepts any entry > whose name, when converted to lower case, contains ".log". If one of those > entries is directory, the predicate accepts it anyway. When {{LogExporter}} > attempts to create a {{FileReader}} read it, the {{FileReader}} constructor > throws {{FileNotFoundException}}. > There is a stat file selector predicate that works similarly. > {{LogExporter}}'s log and stat file selector predicates should accept only > files, and not directories. And perhaps the should accept only files whose > names _end_ in the appropriate extension, rather than _containing_ the > characters. The predicates are defined in {{LogExporter}}'s > {{findLogFiles()}} and {{findStatFiles()}} methods. > I discovered this defect by running {{LogExporterIntegrationTest}} on macOS. > Each test's preparation writes some to-be-exported files into a temporary > directory that, on macOS, may contain numerous other files and directories. > One of those directories (e.g. > /var/folders/yz/6y59fxln38d7lf2jxng1zgg40000gn/T/com.apple.LoginUserService), > which matches the {{LogExporter}}'s predicate. > These tests should also be changed to write their to-be-exported files to a > directory that is known to be empty. -- This message was sent by Atlassian Jira (v8.3.4#803005)