Merge branch 'stable-3.1'

* stable-3.1:
  Make tests that rely on logging work in Eclipse
  Make IssueExtractorTest more compact
  Allow to assert log messages multiple times in one go
  On test failure, dump logged but not yet asserted messages

Change-Id: Idbbd7e5b32b106e12a336fc4d08612abfd72791a
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
index e7a137a..8ca3d5b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
@@ -14,6 +14,8 @@
 
 package com.googlesource.gerrit.plugins.its.base.testutil;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import com.google.common.collect.Lists;
 import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.BranchNameKey;
@@ -36,25 +38,38 @@
 
   private java.util.Collection<LogRecord> records;
 
-  protected final void assertLogMessageContains(String needle, Level level) {
-    LogRecord hit = null;
-    Iterator<LogRecord> iter = records.iterator();
-    while (hit == null && iter.hasNext()) {
-      LogRecord record = iter.next();
-      if (record.getMessage().contains(needle)) {
-        if (level == null || LogUtil.equalLevels(record.getLevel(), level)) {
-          hit = record;
+  protected final void assertLogMessageContains(String needle, Level level, int times) {
+    // We do not support `times == 0`, as it's ambiguous if it means the message does not occur at
+    // all, or message assertion should be skipped.
+    assertThat(times).isGreaterThan(0);
+
+    while (times-- > 0) {
+      LogRecord hit = null;
+      Iterator<LogRecord> iter = records.iterator();
+      while (hit == null && iter.hasNext()) {
+        LogRecord record = iter.next();
+        if (record.getMessage().contains(needle)) {
+          if (level == null || LogUtil.equalLevels(record.getLevel(), level)) {
+            hit = record;
+          }
         }
       }
+      removeLogHit(hit, "containing '" + needle + "'");
     }
-    assertNotNull("Could not find log message containing '" + needle + "'", hit);
-    assertTrue("Could not remove log message containing '" + needle + "'", records.remove(hit));
+  }
+
+  protected final void assertLogMessageContains(String needle, Level level) {
+    assertLogMessageContains(needle, level, 1);
   }
 
   protected final void assertLogMessageContains(String needle) {
     assertLogMessageContains(needle, null);
   }
 
+  protected final void assertLogMessageContains(String needle, int times) {
+    assertLogMessageContains(needle, null, times);
+  }
+
   protected final void assertLogThrowableMessageContains(String needle) {
     LogRecord hit = null;
     Iterator<LogRecord> iter = records.iterator();
@@ -66,10 +81,14 @@
         hit = record;
       }
     }
-    assertNotNull("Could not find log message with a Throwable containing '" + needle + "'", hit);
-    assertTrue(
-        "Could not remove log message with a Throwable containing '" + needle + "'",
-        records.remove(hit));
+    removeLogHit(hit, "with a Throwable containing '\" + needle + \"'");
+  }
+
+  private void removeLogHit(LogRecord hit, String description) {
+    if (hit == null) {
+      failWithUnassertedLogDump("Could not find log message " + description);
+    }
+    assertTrue("Could not remove log message " + description, records.remove(hit));
   }
 
   // As the PowerMock runner does not pass through runTest, we inject log
@@ -77,17 +96,27 @@
   @After
   public final void assertNoUnassertedLogEvents() {
     if (records.size() > 0) {
-      LogRecord record = records.iterator().next();
-      String msg = "Found untreated logged events. First one is:\n";
-      msg += record.getMessage();
-      Throwable t = record.getThrown();
-      if (t != null) {
-        msg += "\n" + t;
-      }
-      fail(msg);
+      failWithUnassertedLogDump("Found unasserted logged events.");
     }
   }
 
+  public final void failWithUnassertedLogDump(String msg) {
+    msg += "\n";
+    if (records.size() == 0) {
+      msg += "(All logged messages have already been asserted)";
+    } else {
+      msg += records.size() + " logged, but not yet asserted messages remain:";
+      for (LogRecord record : records) {
+        msg += "\n" + record.getMessage();
+        Throwable t = record.getThrown();
+        if (t != null) {
+          msg += "\n   with thrown " + t;
+        }
+      }
+    }
+    fail(msg);
+  }
+
   @Override
   public void setUp() throws Exception {
     super.setUp();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
index adccee8..17830db 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.its.base.testutil.log;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.flogger.FluentLogger;
 import java.util.Collection;
 import java.util.logging.Handler;
 import java.util.logging.Level;
@@ -86,14 +87,31 @@
    * @param level The level to user for the logger.
    */
   private static void logToCollectionJul(String logName, CollectionAppender appender, Level level) {
-    java.util.logging.Logger julLogger = java.util.logging.Logger.getLogger(logName);
+    // We'd love to simply get the logger of name `logName` and directly
+    // configure that. While this works for running the tests in bazel, it
+    // fails when running tests from within Eclipse. In Eclipse getting the
+    // logger of the same name here and from the class-under-test will get two
+    // different loggers, due to backend calling from a different class. So we
+    // instead resort to configuring the root logger and filtering the logName
+    // in the appender.
+
+    // The description above works for the 2nd, 3rd, ... test of a test case in
+    // Eclipse, but not for the first. To cover the first test as well, we
+    // beforehand tell Flogger to set things up by getting any random logger
+    // /before/ we configure the root logger.
+    @SuppressWarnings("unused")
+    FluentLogger unused = FluentLogger.forEnclosingClass();
+
+    java.util.logging.Logger julLogger = java.util.logging.Logger.getLogger("");
     julLogger.setLevel(level);
 
     julLogger.addHandler(
         new Handler() {
           @Override
           public void publish(LogRecord record) {
-            appender.append(record);
+            if (record.getLoggerName().equals(logName)) {
+              appender.append(record);
+            }
           }
 
           @Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
index a288f03..3ea73d0 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
@@ -213,11 +213,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 5);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -243,11 +239,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 5);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -273,11 +265,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 5);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -304,12 +292,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "body"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -336,12 +319,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "footer", "footer-Footer"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -375,15 +353,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "footer"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 9);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -414,12 +384,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "footer", "footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -449,12 +414,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "footer", "footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -485,12 +445,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "footer", "footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -522,12 +477,7 @@
     expected.put("4711", Sets.newHashSet("somewhere", "footer", "footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -554,9 +504,7 @@
     expected.put("176", Sets.newHashSet("somewhere", "body"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 3);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -583,9 +531,7 @@
     expected.put("176", Sets.newHashSet("somewhere", "body"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 3);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -613,9 +559,7 @@
     expected.put("176", Sets.newHashSet("somewhere", "body"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 3);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -636,9 +580,7 @@
     Map<String, Set<String>> expected = Maps.newHashMap();
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 3);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -736,12 +678,7 @@
     expected.put("5150", Sets.newHashSet("somewhere", "body", "footer", "footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 6);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -764,9 +701,7 @@
     Map<String, Set<String>> expected = Maps.newHashMap();
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 3);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -795,11 +730,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "subject", "added@somewhere", "added@subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 5);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -837,16 +768,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "subject", "added@somewhere", "added@subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 10);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -884,16 +806,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 10);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -935,16 +848,7 @@
     expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 10);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -988,17 +892,7 @@
         Sets.newHashSet("somewhere", "footer", "added@footer", "footer-Bug", "added@footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 11);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -1056,17 +950,7 @@
             "added@footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 11);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();
@@ -1126,18 +1010,7 @@
             "added@footer-Bug"));
     assertEquals("Extracted issues do not match", expected, actual);
 
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
-    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching", 12);
 
     verifyOneOrMore(itsConfig).getIssuePattern();
     verifyOneOrMore(itsConfig).getIssuePatternGroupIndex();