[GitHub] [geode] demery-pivotal commented on a change in pull request #5625: GEODE-8609: Create a dunit suspect file per VM

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

demery-pivotal commented on a change in pull request #5625:
URL: https://github.com/apache/geode/pull/5625#discussion_r505009141



##########
File path: geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/DUnitLauncher.java
##########
@@ -343,59 +339,120 @@ public static void init(MasterRemote master) {
     System.setProperty(LAUNCHED_PROPERTY, "true");
   }
 
+  private static List<File> getDunitSuspectFiles() {
+    File[] suspectFiles = getDunitSuspectsDir()
+        .listFiles((dir, name) -> name.startsWith(SUSPECT_FILENAME_PREFIX));
+
+    return Arrays.asList(suspectFiles);
+  }
+
+  private static File getDunitSuspectsDir() {
+    return Paths.get(getWorkspaceDir()).toFile();
+  }
+
+  private static void deleteDunitSuspectFiles() {
+    getDunitSuspectFiles().forEach(File::delete);
+  }
+
+  private static File createDunitSuspectFile(int vmId, String workingDir) {
+    String suffix;
+
+    switch (vmId) {
+      case -2:
+        suffix = "locator";
+        break;
+      case -1:
+        suffix = "local";
+        break;
+      default:
+        suffix = "vm" + vmId;
+    }
+
+    File dunitSuspect = new File(getDunitSuspectsDir(),
+        String.format("%s-%s.log", SUSPECT_FILENAME_PREFIX, suffix));
+    dunitSuspect.deleteOnExit();
+
+    return dunitSuspect;
+  }
+
+  private static String getWorkspaceDir() {
+    String workspaceDir = System.getProperty(DUnitLauncher.WORKSPACE_DIR_PARAM);
+    workspaceDir = workspaceDir == null ? new File(".").getAbsolutePath() : workspaceDir;
+
+    return workspaceDir;
+  }
+
   public static void closeAndCheckForSuspects() {
-    if (isLaunched()) {
-      final List<Pattern> expectedStrings = ExpectedStrings.create("dunit");
-      final LogConsumer logConsumer = new LogConsumer(true, expectedStrings, "log4j", 5);
+    if (!isLaunched()) {
+      return;
+    }
 
-      final StringBuilder suspectStringBuilder = new StringBuilder();
+    List<File> suspectFiles = getDunitSuspectFiles();
 
-      BufferedReader buffReader = null;
-      FileChannel fileChannel = null;
-      try {
-        fileChannel = new FileOutputStream(DUNIT_SUSPECT_FILE, true).getChannel();
-        buffReader = new BufferedReader(new FileReader(DUNIT_SUSPECT_FILE));
-      } catch (FileNotFoundException e) {
-        System.err.println("Could not find the suspect string output file: " + e);
-        return;
-      }
+    if (suspectFiles.isEmpty()) {
+      throw new IllegalStateException("No dunit suspect log files found in '"
+          + getDunitSuspectsDir().getAbsolutePath()
+          + "' - perhaps a rule that is cleaning up before suspect processing has already run.");
+    }
+
+    for (File suspect : suspectFiles) {
+      checkSuspectFile(suspect);
+    }
+  }
+
+  public static void checkSuspectFile(File suspectFile) {
+    final List<Pattern> expectedStrings = ExpectedStrings.create("dunit");
+    final LogConsumer logConsumer = new LogConsumer(true, expectedStrings,
+        suspectFile.getName(), 5);
+
+    final StringBuilder suspectStringBuilder = new StringBuilder();
+
+    BufferedReader buffReader;
+    FileChannel fileChannel;
+    try {
+      fileChannel = new FileOutputStream(suspectFile, true).getChannel();
+      buffReader = new BufferedReader(new FileReader(suspectFile));
+    } catch (FileNotFoundException e) {
+      System.err.println("Could not find the suspect string output file: " + e);
+      return;
+    }
+
+    try {
+      String line;
       try {
-        String line;
-        try {
-          while ((line = buffReader.readLine()) != null) {
-            final StringBuilder builder = logConsumer.consume(line);
-            if (builder != null) {
-              suspectStringBuilder.append(builder);
-            }
+        while ((line = buffReader.readLine()) != null) {
+          final StringBuilder builder = logConsumer.consume(line);
+          if (builder != null) {
+            suspectStringBuilder.append(builder);
           }
-        } catch (IOException e) {
-          System.err.println("Could not read the suspect string output file: " + e);
         }
+      } catch (IOException e) {
+        System.err.println("Could not read the suspect string output file: " + e);
+      }
 
-        try {
-          fileChannel.truncate(0);
-        } catch (IOException e) {
-          System.err.println("Could not truncate the suspect string output file: " + e);
-        }
+      try {
+        fileChannel.truncate(0);
+      } catch (IOException e) {
+        System.err.println("Could not truncate the suspect string output file: " + e);
+      }
 
-      } finally {
-        try {
-          buffReader.close();
-          fileChannel.close();
-        } catch (IOException e) {
-          System.err.println("Could not close the suspect string output file: " + e);
-        }
+    } finally {
+      try {
+        buffReader.close();
+        fileChannel.close();
+      } catch (IOException e) {
+        System.err.println("Could not close the suspect string output file: " + e);
       }
+    }
 
-      if (suspectStringBuilder.length() != 0) {
-        System.err.println("Suspicious strings were written to the log during this run.\n"
-            + "Fix the strings or use IgnoredException.addIgnoredException to ignore.\n"
-            + suspectStringBuilder);
+    if (suspectStringBuilder.length() != 0) {

Review comment:
       This will fail on the first log file with suspect strings, and so will not report suspect strings from the remaining log files. If you move this check after the for loop in `closeAndCheckForSuspects()`, it will be able to report suspect strings from all log files.




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