SolrTestCase now supports @loglevel (#2869)

Refactored the functionality into a TestRule, and moved from STCJ4 up to SolrTestCase.
Move reinstatement of the root log level to shutdown()

Author: dsmiley

solr/CHANGES.txt

@@ -177,6 +177,9 @@ Other Changes
 * SOLR-17579: Remove unused code and other refactorings in ReplicationHandler and tests.  Removed unused public 
   LOCAL_ACTIVITY_DURING_REPLICATION variable. (Eric Pugh)
 
+* GITHUB#2869: SolrTestCase now supports @LogLevel annotations (as SolrTestCaseJ4 has).  Added LogLevelTestRule
+  for encapsulation and reuse. (David Smiley)
+
 ==================  9.8.0 ==================
 New Features
 ---------------------

solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java

@@ -43,6 +43,8 @@ public final class StartupLoggingUtils {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   private static final ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory();
 
+  private static final String INITIAL_ROOT_LOG_LEVEL = getLogLevelString();
+
   /** Checks whether mandatory log dir is given */
   public static void checkLogDir() {
     if (EnvUtils.getProperty("solr.log.dir") == null) {
@@ -151,6 +153,11 @@ public static void shutdown() {
     }
     flushAllLoggers();
     LogManager.shutdown(true);
+
+    // re-instate original log level.
+    if (!INITIAL_ROOT_LOG_LEVEL.equals(getLogLevelString())) {
+      changeLogLevel(INITIAL_ROOT_LOG_LEVEL);
+    }
   }
 
   /**

solr/test-framework/src/java/org/apache/solr/SolrTestCase.java

@@ -22,8 +22,8 @@
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
-import com.carrotsearch.randomizedtesting.rules.StatementAdapter;
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter;
 import java.io.File;
 import java.lang.invoke.MethodHandles;
 import java.util.List;
@@ -37,14 +37,17 @@
 import org.apache.solr.common.util.ObjectReleaseTracker;
 import org.apache.solr.servlet.SolrDispatchFilter;
 import org.apache.solr.util.ExternalPaths;
+import org.apache.solr.util.LogLevelTestRule;
 import org.apache.solr.util.RevertDefaultThreadHandlerRule;
 import org.apache.solr.util.StartupLoggingUtils;
 import org.hamcrest.Matcher;
 import org.hamcrest.MatcherAssert;
+import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.ComparisonFailure;
+import org.junit.Rule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 import org.slf4j.Logger;
@@ -90,24 +93,23 @@ public class SolrTestCase extends LuceneTestCase {
               new VerifyTestClassNamingConvention(
                   "org.apache.solr.ltr", NAMING_CONVENTION_TEST_PREFIX))
           .around(new RevertDefaultThreadHandlerRule())
+          .around(new LogLevelTestRule())
           .around(
-              (base, description) ->
-                  new StatementAdapter(base) {
-                    @Override
-                    protected void afterIfSuccessful() {
-                      // if the tests passed, make sure everything was closed / released
-                      String orr = ObjectReleaseTracker.clearObjectTrackerAndCheckEmpty();
-                      assertNull(orr, orr);
-                    }
-
-                    @Override
-                    protected void afterAlways(List<Throwable> errors) {
-                      if (!errors.isEmpty()) {
-                        ObjectReleaseTracker.tryClose();
-                      }
-                      StartupLoggingUtils.shutdown();
-                    }
-                  });
+              new TestRuleAdapter() {
+                @Override
+                protected void afterIfSuccessful() {
+                  // if the tests passed, make sure everything was closed / released
+                  String orr = ObjectReleaseTracker.clearObjectTrackerAndCheckEmpty();
+                  assertNull(orr, orr);
+                }
+
+                @Override
+                protected void afterAlways(List<Throwable> errors) {
+                  if (!errors.isEmpty()) {
+                    ObjectReleaseTracker.tryClose();
+                  }
+                }
+              });
 
   /**
    * Sets the <code>solr.default.confdir</code> system property to the value of {@link
@@ -174,19 +176,27 @@ public static void checkSyspropForceBeforeClassAssumptionFailure() {
     assumeFalse(PROP + " == true", systemPropertyAsBoolean(PROP, false));
   }
 
+  @AfterClass
+  public static void afterClassShutdownLogging() {
+    StartupLoggingUtils.shutdown();
+  }
+
+  @Rule public TestRule methodRules = new LogLevelTestRule();
+
   /**
    * Special hook for sanity checking if any tests trigger failures when an Assumption failure
-   * occures in a {@link Before} method
+   * occurs in a {@link Before} method
    *
    * @lucene.internal
    */
   @Before
   public void checkSyspropForceBeforeAssumptionFailure() {
-    // ant test -Dargs="-Dtests.force.assumption.failure.before=true"
     final String PROP = "tests.force.assumption.failure.before";
     assumeFalse(PROP + " == true", systemPropertyAsBoolean(PROP, false));
   }
 
+  //              UTILITY METHODS FOLLOW
+
   public static void assertJSONEquals(String expected, String actual) {
     Object json1 = fromJSONString(expected);
     Object json2 = fromJSONString(actual);

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -41,7 +41,6 @@
 import java.lang.annotation.Target;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -75,7 +74,6 @@
 import java.util.stream.Collectors;
 import javax.xml.xpath.XPathExpressionException;
 import org.apache.http.client.HttpClient;
-import org.apache.logging.log4j.Level;
 import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.tests.analysis.MockAnalyzer;
 import org.apache.lucene.tests.analysis.MockTokenizer;
@@ -109,7 +107,6 @@
 import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.common.util.SolrNamedThreadFactory;
-import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.common.util.XML;
 import org.apache.solr.core.CoreContainer;
@@ -138,18 +135,14 @@
 import org.apache.solr.util.DirectoryUtil;
 import org.apache.solr.util.ErrorLogMuter;
 import org.apache.solr.util.ExternalPaths;
-import org.apache.solr.util.LogLevel;
 import org.apache.solr.util.RandomizeSSL;
 import org.apache.solr.util.RandomizeSSL.SSLRandomizer;
 import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.SSLTestConfig;
-import org.apache.solr.util.StartupLoggingUtils;
 import org.apache.solr.util.TestHarness;
 import org.apache.solr.util.TestInjection;
 import org.apache.zookeeper.KeeperException;
-import org.junit.After;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.rules.RuleChain;
@@ -187,8 +180,6 @@ public abstract class SolrTestCaseJ4 extends SolrTestCase {
 
   public static int DEFAULT_CONNECTION_TIMEOUT = 60000; // default socket connection timeout in ms
 
-  private static String initialRootLogLevel;
-
   protected static volatile ExecutorService testExecutor;
 
   protected void writeCoreProperties(Path coreDirectory, String coreName) throws IOException {
@@ -257,8 +248,6 @@ protected void assertExceptionThrownWithMessageContaining(
 
   @BeforeClass
   public static void setupTestCases() {
-    initialRootLogLevel = StartupLoggingUtils.getLogLevelString();
-    initClassLogLevels();
     resetExceptionIgnores();
 
     testExecutor =
@@ -348,10 +337,6 @@ public static void teardownTestCases() throws Exception {
       testSolrHome = null;
 
       IpTables.unblockAllPorts();
-
-      LogLevel.Configurer.restoreLogLevels(savedClassLogLevels);
-      savedClassLogLevels.clear();
-      StartupLoggingUtils.changeLogLevel(initialRootLogLevel);
     }
   }
 
@@ -391,38 +376,6 @@ public static void assumeWorkingMockito() {
     }
   }
 
-  @SuppressForbidden(reason = "Using the Level class from log4j2 directly")
-  private static Map<String, Level> savedClassLogLevels = new HashMap<>();
-
-  public static void initClassLogLevels() {
-    Class<?> currentClass = RandomizedContext.current().getTargetClass();
-    LogLevel annotation = currentClass.getAnnotation(LogLevel.class);
-    if (annotation == null) {
-      return;
-    }
-    Map<String, Level> previousLevels = LogLevel.Configurer.setLevels(annotation.value());
-    savedClassLogLevels.putAll(previousLevels);
-  }
-
-  private Map<String, Level> savedMethodLogLevels = new HashMap<>();
-
-  @Before
-  public void initMethodLogLevels() {
-    Method method = RandomizedContext.current().getTargetMethod();
-    LogLevel annotation = method.getAnnotation(LogLevel.class);
-    if (annotation == null) {
-      return;
-    }
-    Map<String, Level> previousLevels = LogLevel.Configurer.setLevels(annotation.value());
-    savedMethodLogLevels.putAll(previousLevels);
-  }
-
-  @After
-  public void restoreMethodLogLevels() {
-    LogLevel.Configurer.restoreLogLevels(savedMethodLogLevels);
-    savedMethodLogLevels.clear();
-  }
-
   protected static boolean isSSLMode() {
     return sslConfig != null && sslConfig.isSSLMode();
   }

solr/test-framework/src/java/org/apache/solr/util/LogLevel.java

@@ -38,6 +38,8 @@
  * like this: <code>
  *   {@literal @}LogLevel("org.apache.solr=DEBUG;org.apache.solr.core=INFO")
  * </code>
+ *
+ * @see LogLevelTestRule
  */
 @Documented
 @Inherited

solr/test-framework/src/java/org/apache/solr/util/LogLevelTestRule.java

@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util;
+
+import java.lang.annotation.Annotation;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.logging.log4j.Level;
+import org.apache.solr.common.util.SuppressForbidden;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/**
+ * A JUnit {@link TestRule} that sets (and resets) the Log4j2 log level based on the {@code
+ * LogLevel} annotation.
+ */
+public class LogLevelTestRule implements TestRule {
+
+  @Override
+  public Statement apply(Statement base, Description description) {
+    // loop over the annotations to find LogLevel
+    final Optional<Annotation> annotationOpt =
+        description.getAnnotations().stream()
+            .filter(a -> a.annotationType().equals(LogLevel.class))
+            .findAny();
+    if (annotationOpt.isEmpty()) {
+      return base;
+    }
+    final var annotation = (LogLevel) annotationOpt.get();
+    return new LogLevelStatement(base, annotation);
+  }
+
+  static class LogLevelStatement extends Statement {
+    private final Statement delegate;
+    private final LogLevel annotation;
+
+    protected LogLevelStatement(Statement delegate, LogLevel annotation) {
+      this.delegate = delegate;
+      this.annotation = annotation;
+    }
+
+    @SuppressForbidden(reason = "Using the Level class from log4j2 directly")
+    @Override
+    public void evaluate() throws Throwable {
+      Map<String, Level> savedLogLevels = LogLevel.Configurer.setLevels(annotation.value());
+      try {
+        delegate.evaluate();
+      } finally {
+        LogLevel.Configurer.restoreLogLevels(savedLogLevels);
+      }
+    }
+  }
+}

solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java renamed to solr/test-framework/src/test/org/apache/solr/util/TestLogLevelAnnotations.java

@@ -15,22 +15,26 @@
  * limitations under the License.
  */
 
-package org.apache.solr;
+package org.apache.solr.util;
 
 import java.util.Map;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.solr.SolrTestCase;
 import org.apache.solr.common.util.SuppressForbidden;
-import org.apache.solr.util.LogLevel;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
+/**
+ * @see LogLevel
+ * @see LogLevelTestRule
+ */
 @SuppressForbidden(reason = "We need to use log4J2 classes to access the log levels")
 @LogLevel(
     "org.apache.solr.bogus_logger.ClassLogLevel=error;org.apache.solr.bogus_logger.MethodLogLevel=warn")
-public class TestLogLevelAnnotations extends SolrTestCaseJ4 {
+public class TestLogLevelAnnotations extends SolrTestCase {
 
   private static final String bogus_logger_prefix = "org.apache.solr.bogus_logger";
 
@@ -42,12 +46,12 @@ public class TestLogLevelAnnotations extends SolrTestCaseJ4 {
    * the test.
    *
    * <p>We also don't want to initialize this in a <code>@BeforeClass</code> method because that
-   * will run <em>after</em> the <code>@BeforeClass</code> logic of our super class {@link
-   * SolrTestCaseJ4} where the <code>@LogLevel</code> annotation on this class will be parsed and
-   * evaluated -- modifying the log4j run time configuration. The <code>@LogLevel</code>
-   * configuration of this class <em>should</em> not affect the "root" Logger, but setting this in
-   * static class initialization protect us (as best we can) against the possibility that it
-   * <em>might</em> due to an unforseen (future) bug.
+   * will run <em>after</em> the <code>@BeforeClass</code> or {@code @ClassRule} logic of our super
+   * class where the <code>@LogLevel</code> annotation on this class will be parsed and evaluated --
+   * modifying the log4j run time configuration. The <code>@LogLevel</code> configuration of this
+   * class <em>should</em> not affect the "root" Logger, but setting this in static class
+   * initialization protect us (as best we can) against the possibility that it <em>might</em> due
+   * to an unforseen (future) bug.
    *
    * @see #checkLogLevelsBeforeClass
    */
@@ -93,9 +97,8 @@ public static void checkLogLevelsBeforeClass() {
    * Check that the expected log level <em>configurations</em> have been reset after the test
    *
    * <p><b>NOTE:</b> We only validate <code>@LogLevel</code> modifications made at the {@link
-   * #testMethodLogLevels} level, not at the 'class' level, because of the lifecycle of junit
-   * methods: This <code>@AfterClass</code> will run before the <code>SolrTestCaseJ4#@AfterClass
-   * </code> method where the 'class' <code>@LogLevel</code> modifications will be reset.
+   * #testMethodLogLevels} level, not at the 'class' level, because the lifecycle of junit methods
+   * that activate this logic prevent us from doing so.
    *
    * @see #checkLogLevelsBeforeClass
    * @see #testWhiteBoxMethods