diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b7291e9a23e..2f20701968b 100644 --- a/solr/CHANGES.txt +++ b/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 --------------------- diff --git a/solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java b/solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java index aeda7757593..79047a502e3 100644 --- a/solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java +++ b/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); + } } /** diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java index 98a0634ca5e..909a38479ad 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java +++ b/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 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 errors) { + if (!errors.isEmpty()) { + ObjectReleaseTracker.tryClose(); + } + } + }); /** * Sets the solr.default.confdir 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); diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index d5d630333a4..a5e256d2fac 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/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 savedClassLogLevels = new HashMap<>(); - - public static void initClassLogLevels() { - Class currentClass = RandomizedContext.current().getTargetClass(); - LogLevel annotation = currentClass.getAnnotation(LogLevel.class); - if (annotation == null) { - return; - } - Map previousLevels = LogLevel.Configurer.setLevels(annotation.value()); - savedClassLogLevels.putAll(previousLevels); - } - - private Map savedMethodLogLevels = new HashMap<>(); - - @Before - public void initMethodLogLevels() { - Method method = RandomizedContext.current().getTargetMethod(); - LogLevel annotation = method.getAnnotation(LogLevel.class); - if (annotation == null) { - return; - } - Map 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(); } diff --git a/solr/test-framework/src/java/org/apache/solr/util/LogLevel.java b/solr/test-framework/src/java/org/apache/solr/util/LogLevel.java index af7451944e9..4d4d0387df6 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/LogLevel.java +++ b/solr/test-framework/src/java/org/apache/solr/util/LogLevel.java @@ -38,6 +38,8 @@ * like this: * {@literal @}LogLevel("org.apache.solr=DEBUG;org.apache.solr.core=INFO") * + * + * @see LogLevelTestRule */ @Documented @Inherited diff --git a/solr/test-framework/src/java/org/apache/solr/util/LogLevelTestRule.java b/solr/test-framework/src/java/org/apache/solr/util/LogLevelTestRule.java new file mode 100644 index 00000000000..99ce3d02665 --- /dev/null +++ b/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 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 savedLogLevels = LogLevel.Configurer.setLevels(annotation.value()); + try { + delegate.evaluate(); + } finally { + LogLevel.Configurer.restoreLogLevels(savedLogLevels); + } + } + } +} diff --git a/solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java b/solr/test-framework/src/test/org/apache/solr/util/TestLogLevelAnnotations.java similarity index 89% rename from solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java rename to solr/test-framework/src/test/org/apache/solr/util/TestLogLevelAnnotations.java index 83b2888910e..ebfad4ea63c 100644 --- a/solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java +++ b/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. * *

We also don't want to initialize this in a @BeforeClass method because that - * will run after the @BeforeClass logic of our super class {@link - * SolrTestCaseJ4} where the @LogLevel annotation on this class will be parsed and - * evaluated -- modifying the log4j run time configuration. The @LogLevel - * configuration of this class should not affect the "root" Logger, but setting this in - * static class initialization protect us (as best we can) against the possibility that it - * might due to an unforseen (future) bug. + * will run after the @BeforeClass or {@code @ClassRule} logic of our super + * class where the @LogLevel annotation on this class will be parsed and evaluated -- + * modifying the log4j run time configuration. The @LogLevel configuration of this + * class should not affect the "root" Logger, but setting this in static class + * initialization protect us (as best we can) against the possibility that it might due + * to an unforseen (future) bug. * * @see #checkLogLevelsBeforeClass */ @@ -93,9 +97,8 @@ public static void checkLogLevelsBeforeClass() { * Check that the expected log level configurations have been reset after the test * *

NOTE: We only validate @LogLevel modifications made at the {@link - * #testMethodLogLevels} level, not at the 'class' level, because of the lifecycle of junit - * methods: This @AfterClass will run before the SolrTestCaseJ4#@AfterClass - * method where the 'class' @LogLevel 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