Make StatusLogger self-contained and testable (#2249)

Author: vy

log4j-api-test/pom.xml

@@ -138,7 +138,6 @@
  <dependency>
  <groupId>org.mockito</groupId>
  <artifactId>mockito-core</artifactId>
- <scope>test</scope>
  </dependency>
  <dependency>
  <groupId>org.mockito</groupId>

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusLoggerExtension.java renamed to log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusListenerExtension.java

@@ -16,21 +16,16 @@
  */
 package org.apache.logging.log4j.test.junit;
 
-import java.io.IOException;
-import java.time.Instant;
-import java.time.ZoneId;
-import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Stream;
 import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
-import org.apache.logging.log4j.simple.SimpleLogger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
+import org.apache.logging.log4j.status.StatusConsoleListener;
 import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.status.StatusListener;
 import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.test.ListStatusListener;
-import org.apache.logging.log4j.util.PropertiesUtil;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -43,12 +38,12 @@
 import org.junit.platform.commons.support.ModifierSupport;
 import org.junit.platform.commons.support.ReflectionSupport;
 
-class StatusLoggerExtension extends TypeBasedParameterResolver<ListStatusListener>
+class StatusListenerExtension extends TypeBasedParameterResolver<ListStatusListener>
  implements BeforeAllCallback, BeforeEachCallback, TestExecutionExceptionHandler {
 
  private static final Object KEY = ListStatusListener.class;
 
- public StatusLoggerExtension() {
+ public StatusListenerExtension() {
  super(ListStatusListener.class);
  }
 
@@ -130,33 +125,19 @@ public ListStatusListener getStatusListener() {
  }
 
  @Override
- public void close() throws Throwable {
+ public void close() {
  statusLogger.removeListener(statusListener);
  }
 
  public void handleException(final ExtensionContext context, final Throwable throwable) {
- final Logger logger = new SimpleLogger(
- "StatusLoggerExtension",
- Level.ALL,
- false,
- false,
- false,
- false,
+ final StatusListener listener = new StatusConsoleListener(Level.ALL, System.err);
+ listener.log(new StatusData(
  null,
- ParameterizedNoReferenceMessageFactory.INSTANCE,
- PropertiesUtil.getProperties(),
- System.err);
- logger.error("Test {} failed.\nDumping status data:", context.getDisplayName());
- final DateTimeFormatter formatter = DateTimeFormatter.ISO_LOCAL_TIME.withZone(ZoneId.systemDefault());
- statusListener.getStatusData().forEach(data -> {
- logger.atLevel(data.getLevel())
- .withThrowable(data.getThrowable())
- .withLocation(data.getStackTraceElement())
- .log(
- "{} {}",
- formatter.format(Instant.ofEpochMilli(data.getTimestamp())),
- data.getMessage().getFormattedMessage());
- });
+ Level.ERROR,
+ new ParameterizedMessage("Test `{}` has failed, dumping status data...", context.getDisplayName()),
+ throwable,
+ null));
+ statusListener.getStatusData().forEach(listener::log);
  }
  }
 
@@ -186,14 +167,14 @@ public Level getStatusLevel() {
  }
 
  @Override
- public void close() throws IOException {
+ public void close() {
  // NOP
  }
 
  @Override
  public Stream<StatusData> getStatusData() {
  synchronized (statusData) {
- final List<StatusData> clone = (List<StatusData>) statusData.clone();
+ final List<StatusData> clone = new ArrayList<>(statusData);
  return parent != null ? Stream.concat(parent.getStatusData(), clone.stream()) : clone.stream();
  }
  }

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusLoggerMockExtension.java

@@ -0,0 +1,75 @@
+/*
+ * 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.logging.log4j.test.junit;
+
+import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.getAttribute;
+import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.setAttribute;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.when;
+
+import org.apache.logging.log4j.status.StatusConsoleListener;
+import org.apache.logging.log4j.status.StatusLogger;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+/**
+ * Replaces {@link StatusLogger} static instance with a mocked one.
+ * <p>
+ * <b>Warning!</b>
+ * Many classes store the result of {@link StatusLogger#getLogger()} in {@code static} field.
+ * Hence, the mock replacement must be performed before anybody tries to access it.
+ * Similarly, we cannot replace the mock in between tests, since it is already stored in {@code static} fields.
+ * That is why we only reset the mocked instance before each test.
+ * </p>
+ *
+ * @see UsingStatusLoggerMock
+ */
+class StatusLoggerMockExtension implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback {
+
+ private static final String KEY_PREFIX = StatusLoggerMockExtension.class.getSimpleName() + '.';
+
+ private static final String INITIAL_STATUS_LOGGER_KEY = KEY_PREFIX + "initialStatusLogger";
+
+ @Override
+ public void beforeAll(final ExtensionContext context) throws Exception {
+ setAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.getLogger(), context);
+ final StatusLogger statusLogger = mock(StatusLogger.class);
+ stubFallbackListener(statusLogger);
+ StatusLogger.setLogger(statusLogger);
+ }
+
+ @Override
+ public void beforeEach(final ExtensionContext context) throws Exception {
+ final StatusLogger statusLogger = StatusLogger.getLogger();
+ reset(statusLogger); // Stubs get reset too!
+ stubFallbackListener(statusLogger);
+ }
+
+ private static void stubFallbackListener(final StatusLogger statusLogger) {
+ final StatusConsoleListener fallbackListener = mock(StatusConsoleListener.class);
+ when(statusLogger.getFallbackListener()).thenReturn(fallbackListener);
+ }
+
+ @Override
+ public void afterAll(final ExtensionContext context) {
+ final StatusLogger statusLogger = getAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.class, context);
+ StatusLogger.setLogger(statusLogger);
+ }
+}

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusListener.java

@@ -37,5 +37,5 @@
 @Documented
 @ExtendWith(ExtensionContextAnchor.class)
 @ExtendWith(TestPropertyResolver.class)
-@ExtendWith(StatusLoggerExtension.class)
+@ExtendWith(StatusListenerExtension.class)
 public @interface UsingStatusListener {}

log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/QueueFullAsyncLoggerConfigLoggingFromToStringTest2.java renamed to log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java

@@ -14,24 +14,22 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.logging.log4j.core.async;
+package org.apache.logging.log4j.test.junit;
 
-import org.apache.logging.log4j.core.LoggerContext;
-import org.apache.logging.log4j.core.test.junit.Tags;
-import org.apache.logging.log4j.test.junit.SetTestProperty;
-import org.junit.jupiter.api.Tag;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 /**
- * Tests queue full scenarios with AsyncLoggers in configuration.
+ * Shortcut to {@link StatusLoggerMockExtension}.
  */
-@SetTestProperty(key = "log4j2.formatMsgAsync", value = "true")
-@Tag(Tags.ASYNC_LOGGERS)
-public class QueueFullAsyncLoggerConfigLoggingFromToStringTest2
- extends QueueFullAsyncLoggerConfigLoggingFromToStringTest {
-
- @Override
- protected void checkConfig(final LoggerContext ctx) throws ReflectiveOperationException {
- super.checkConfig(ctx);
- assertFormatMessagesInBackground();
- }
-}
+@Retention(RUNTIME)
+@Target({TYPE, METHOD})
+@Documented
+@ExtendWith({ExtensionContextAnchor.class, StatusLoggerMockExtension.class})
+public @interface UsingStatusLoggerMock {}

log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java

@@ -42,9 +42,12 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
+import org.junitpioneer.jupiter.SetSystemProperty;
 
 @StatusLoggerLevel("WARN")
 @ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ)
+@SetSystemProperty(key = "log4j2.status.entries", value = "200")
+@SetSystemProperty(key = "log4j2.StatusLogger.level", value = "WARN")
 public class AbstractLoggerTest {
 
  private static final StringBuilder CHAR_SEQ = new StringBuilder("CharSeq");

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java

@@ -19,11 +19,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.LogBuilder;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.MessageFactory;
 import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
-import org.apache.logging.log4j.simple.SimpleLogger;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -33,38 +31,19 @@ public class StatusConsoleListenerTest {
  public static final MessageFactory MESSAGE_FACTORY = ParameterizedNoReferenceMessageFactory.INSTANCE;
 
  @Test
- void SimpleLogger_should_be_used() {
-
- // Create a mock `SimpleLoggerFactory`.
- final SimpleLogger logger = Mockito.mock(SimpleLogger.class);
- final LogBuilder logBuilder = Mockito.mock(LogBuilder.class);
- Mockito.when(logger.atLevel(Mockito.any())).thenReturn(logBuilder);
- Mockito.when(logBuilder.withThrowable(Mockito.any())).thenReturn(logBuilder);
- Mockito.when(logBuilder.withLocation(Mockito.any())).thenReturn(logBuilder);
- final SimpleLoggerFactory loggerFactory = Mockito.mock(SimpleLoggerFactory.class);
- Mockito.when(loggerFactory.createSimpleLogger(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
- .thenReturn(logger);
+ void StatusData_getFormattedStatus_should_be_used() {
 
  // Create the listener.
  final PrintStream stream = Mockito.mock(PrintStream.class);
- final Level level = Mockito.mock(Level.class);
- final StatusConsoleListener listener = new StatusConsoleListener(level, stream, loggerFactory);
+ final StatusConsoleListener listener = new StatusConsoleListener(Level.ALL, stream);
 
  // Log a message.
- final StackTraceElement caller = Mockito.mock(StackTraceElement.class);
  final Message message = Mockito.mock(Message.class);
- final Throwable throwable = Mockito.mock(Throwable.class);
- final StatusData statusData = new StatusData(caller, level, message, throwable, null);
+ final StatusData statusData = Mockito.spy(new StatusData(null, Level.TRACE, message, null, null));
  listener.log(statusData);
 
  // Verify the call.
- Mockito.verify(loggerFactory)
- .createSimpleLogger(
- Mockito.eq("StatusConsoleListener"), Mockito.same(level), Mockito.any(), Mockito.same(stream));
- Mockito.verify(logger).atLevel(Mockito.same(level));
- Mockito.verify(logBuilder).withThrowable(Mockito.same(throwable));
- Mockito.verify(logBuilder).withLocation(Mockito.same(caller));
- Mockito.verify(logBuilder).log(Mockito.same(message));
+ Mockito.verify(statusData).getFormattedStatus();
  }
 
  @Test

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerLevelTest.java

@@ -0,0 +1,85 @@
+/*
+ * 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.logging.log4j.status;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.logging.log4j.Level;
+import org.junit.jupiter.api.Test;
+
+class StatusLoggerLevelTest {
+
+ @Test
+ void effective_level_should_be_the_least_specific_one() {
+
+ // Verify the initial level
+ final StatusLogger logger = StatusLogger.getLogger();
+ final Level fallbackListenerLevel = Level.ERROR;
+ assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel);
+
+ // Register a less specific listener
+ final StatusListener listener1 = mock(StatusListener.class);
+ Level listener1Level = Level.WARN;
+ when(listener1.getStatusLevel()).thenReturn(listener1Level);
+ logger.registerListener(listener1);
+ assertThat(listener1Level).isNotEqualTo(fallbackListenerLevel); // Verify that the level is distinct
+ assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the logger level is changed
+
+ // Register a less specific listener
+ final StatusListener listener2 = mock(StatusListener.class);
+ final Level listener2Level = Level.INFO;
+ when(listener2.getStatusLevel()).thenReturn(listener2Level);
+ logger.registerListener(listener2);
+ assertThat(listener2Level)
+ .isNotEqualTo(fallbackListenerLevel)
+ .isNotEqualTo(listener1Level); // Verify that the level is distinct
+ assertThat(logger.getLevel()).isEqualTo(listener2Level); // Verify that the logger level is changed
+
+ // Register a more specific listener
+ final StatusListener listener3 = mock(StatusListener.class);
+ final Level listener3Level = Level.ERROR;
+ when(listener3.getStatusLevel()).thenReturn(listener3Level);
+ logger.registerListener(listener3);
+ assertThat(listener3Level)
+ .isNotEqualTo(listener1Level)
+ .isNotEqualTo(listener2Level); // Verify that the level is distinct
+ assertThat(logger.getLevel()).isEqualTo(listener2Level); // Verify that the logger level is not changed
+
+ // Update a registered listener level
+ listener1Level = Level.DEBUG;
+ when(listener1.getStatusLevel()).thenReturn(listener1Level);
+ assertThat(listener1Level) // Verify that the level is distinct
+ .isNotEqualTo(fallbackListenerLevel)
+ .isNotEqualTo(listener2Level)
+ .isNotEqualTo(listener3Level);
+ assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the logger level is changed
+
+ // Remove the least specific listener
+ logger.removeListener(listener2);
+ assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the level is changed
+
+ // Remove the most specific listener
+ logger.removeListener(listener3);
+ assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the level is not changed
+
+ // Remove the last listener
+ logger.removeListener(listener1);
+ assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel); // Verify that the level is changed
+ }
+}