From d97a2098e387f22d7aa9a5645dff62e9ebc66dec Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 18:44:37 +0000 Subject: [PATCH 01/16] Create utils and Jupter extension to reset Logback * Add LogbackTestHelpers utility class. This provides utilities to reset Logback. * Add ResetLogbackLoggingExtension, a Jupiter extension to reset Logback after all tests have run. * Add UncheckedJoranException, which wraps the checked Logback JoranException. * Update several tests that use Dropwizard extensions to use ResetLogbackLoggingExtension to restore logging. Closes #460 Closes #461 --- .../jupiter/ResetLogbackLoggingExtension.java | 71 ++++++++ .../test/logback/LogbackTestHelpers.java | 50 ++++++ .../test/logback/UncheckedJoranException.java | 42 +++++ .../app/DropwizardAppTestsTest.java | 2 + ...esAppTestExtensionConfigOverridesTest.java | 3 + ...resAppTestExtensionCustomPropertyTest.java | 3 + .../app/PostgresAppTestExtensionTest.java | 3 + .../LogbackTestHelpersIntegrationTest.java | 165 ++++++++++++++++++ .../test/logback/LogbackTestHelpersTest.java | 34 ++++ .../logback/UncheckedJoranExceptionTest.java | 32 ++++ .../invalid-logback-test.xml | 18 ++ src/test/resources/logback-test.xml | 8 + 12 files changed, 431 insertions(+) create mode 100644 src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java create mode 100644 src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java create mode 100644 src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java create mode 100644 src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java create mode 100644 src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java create mode 100644 src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java create mode 100644 src/test/resources/LogbackTestHelpersTest/invalid-logback-test.xml diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java new file mode 100644 index 00000000..17333f65 --- /dev/null +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -0,0 +1,71 @@ +package org.kiwiproject.test.junit.jupiter; + +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.NoArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.kiwiproject.test.logback.LogbackTestHelpers; + +import ch.qos.logback.classic.ClassicConstants; + +/** + * A JUnit Jupiter {@link org.junit.jupiter.api.extension.Extension Extension} to reset + * the Logback logging system after all tests have completed. + *

+ * This is useful if something misbehaves, for example Dropwizard's + * DropwizardAppExtension and + * DropwizardClientExtension + * extensions both stop and detach all appenders after all tests complete! Both of those extensions + * reset Logback in + * DropwizardTestSupport. + * Once this happens, there is no logging output from subsequent tests (since there are no more appenders). + * We consider this to be bad, since logging output is useful to track down causes if + * there are other test failures. And, it's just not nice behavior to completely hijack logging! + *

+ * You can use this extension in tests that are using misbehaving components to ensure that Logback + * is reset after all tests complete, so that subsequent tests have log output. + *

+ * For example to use the default {@code logback-test.xml} as the logging configuration you + * can just use {@code @ExtendWith} on the test class: + * 

+ *  {@literal @}ExtendWith(DropwizardExtensionsSupport.class)
+ *  {@literal @}ExtendWith(ResetLogbackLoggingExtension.class)
+ *   class CustomClientTest {
+ *
+ *       // test code that uses DropwizardClientExtension
+ *   }
+ *
+ * Alternatively, you can register the extension programmatically to use a custom + * logging configuration: + * 
+ *  {@literal @}ExtendWith(DropwizardExtensionsSupport.class)
+ *   class CustomClientTest {
+ *
+ *      {@literal @}RegisterExtension
+ *       static final ResetLogbackLoggingExtension RESET_LOGBACK = ResetLogbackLoggingExtension.builder()
+ *               .logbackConfigFilePath("acme-special-logback.xml")
+ *               .build();
+ *
+ *       // test code that uses DropwizardClientExtension
+ *   }
+ *
+ */ +@Builder +@NoArgsConstructor +@AllArgsConstructor(access = AccessLevel.PACKAGE) +@Slf4j +public class ResetLogbackLoggingExtension implements AfterAllCallback { + + @Builder.Default + private String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; + + @Override + public void afterAll(ExtensionContext context) throws Exception { + LogbackTestHelpers.resetLogback(logbackConfigFilePath); + LOG.info("Logback was reset using configuration: {}", logbackConfigFilePath); + } +} diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java new file mode 100644 index 00000000..1ae59edd --- /dev/null +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java @@ -0,0 +1,50 @@ +package org.kiwiproject.test.logback; + +import com.google.common.io.Resources; + +import lombok.experimental.UtilityClass; + +import ch.qos.logback.classic.ClassicConstants; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.joran.JoranConfigurator; +import ch.qos.logback.core.joran.spi.JoranException; + +import org.slf4j.LoggerFactory; + +/** + * Static test utilities that provide Logback-related functionality. + */ +@UtilityClass +public class LogbackTestHelpers { + + /** + * Reset the Logback logging system using the default test configuration file ({@code logback-test.xml}). + * + * @see ClassicConstants#TEST_AUTOCONFIG_FILE + * @throws UncheckedJoranException if an error occurs resetting Logback + */ + public static void resetLogback() { + resetLogback(ClassicConstants.TEST_AUTOCONFIG_FILE); + } + + /** + * Reset the Logback logging system using the given configuration file. + * + * @param logbackConfigFilePath the location of the custom Logback configuration file + * @throws UncheckedJoranException if an error occurs resetting Logback + */ + public static void resetLogback(String logbackConfigFilePath) { + try { + var loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); + loggerContext.stop(); + + var joranConfigurator = new JoranConfigurator(); + joranConfigurator.setContext(loggerContext); + var logbackConfigUrl = Resources.getResource(logbackConfigFilePath); + joranConfigurator.doConfigure(logbackConfigUrl); + loggerContext.start(); + } catch (JoranException e) { + throw new UncheckedJoranException(e); + } + } +} diff --git a/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java b/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java new file mode 100644 index 00000000..e5a0acd9 --- /dev/null +++ b/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java @@ -0,0 +1,42 @@ +package org.kiwiproject.test.logback; + +import static org.kiwiproject.base.KiwiPreconditions.requireNotNull; + +import ch.qos.logback.core.joran.spi.JoranException; + +/** + * Wraps a {@link JoranException} with an unchecked exception. + */ +public class UncheckedJoranException extends RuntimeException { + + /** + * Construct an instance. + * + * @param message the message, which can be null + * @param cause the cause, which cannot be null + * @throws IllegalArgumentException if cause is null + */ + public UncheckedJoranException(String message, JoranException cause) { + super(message, requireNotNull(cause)); + } + + /** + * Construct an instance. + * + * @param cause the cause, which cannot be null + * @throws IllegalArgumentException if cause is null + */ + public UncheckedJoranException(JoranException cause) { + super(requireNotNull(cause)); + } + + /** + * Returns the cause of this exception. + * + * @return the {@link JoranException} which is the cause of this exception + */ + @Override + public JoranException getCause() { + return (JoranException) super.getCause(); + } +} diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/DropwizardAppTestsTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/DropwizardAppTestsTest.java index 7ff50e78..0c8f9061 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/DropwizardAppTestsTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/DropwizardAppTestsTest.java @@ -27,12 +27,14 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; import java.util.List; @DisplayName("DropwizardAppTests") @ExtendWith(SoftAssertionsExtension.class) @ExtendWith(DropwizardExtensionsSupport.class) +@ExtendWith(ResetLogbackLoggingExtension.class) class DropwizardAppTestsTest { @Getter diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java index a7b9662b..2463b066 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java @@ -18,9 +18,12 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; @DisplayName("PostgresAppTestExtension: ConfigOverrides") +@ExtendWith(ResetLogbackLoggingExtension.class) class PostgresAppTestExtensionConfigOverridesTest { @Getter diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java index 56b47c25..53e51d0c 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java @@ -16,12 +16,15 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; /** * Tests that we can specify a custom name in the configuration for the DataSourceFactory. */ @DisplayName("PostgresAppTestExtension (custom DataSourceFactory property") +@ExtendWith(ResetLogbackLoggingExtension.class) class PostgresAppTestExtensionCustomPropertyTest { @Getter diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java index 783afb46..c6092702 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java @@ -16,9 +16,12 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; @DisplayName("PostgresAppTestExtension") +@ExtendWith(ResetLogbackLoggingExtension.class) class PostgresAppTestExtensionTest { @Getter diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java new file mode 100644 index 00000000..7f02e489 --- /dev/null +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java @@ -0,0 +1,165 @@ +package org.kiwiproject.test.logback; + +import static java.util.Objects.nonNull; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +import com.codahale.metrics.health.HealthCheck; + +import io.dropwizard.core.Application; +import io.dropwizard.core.Configuration; +import io.dropwizard.core.setup.Environment; +import io.dropwizard.testing.junit5.DropwizardAppExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import lombok.extern.slf4j.Slf4j; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.extension.ExtendWith; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Logger; + +/** + * This integration test uses DropwizardAppExtension which resets Logback when it + * starts the test application class. It first verifies that there is no appender + * for this class' Logger, and then uses {@link LogbackTestHelpers#resetLogback()} + * to reset Loback to the default logging configuration. Finally, it ensures that + * the InMemoryAppender was reset and that it receives the expected messages. + *

+ * The tests are designed to execute in a specific order and use Jupiter's + * method ordering feature. The first test executes after DropwizardAppExtension + * has reset Logback, so we expect the appender to be null. That test resets + * Logback, after which all subsequent tests should get a non-null appender. + *

+ * In case of failure, this test uses the ResetLogbackLoggingExtension to restore + * the Logback logging configuration. However, since that extension simply uses + * {@link LogbackTestHelpers}, it might not work if there is actually a bug and + * is therefore a bit circular. But, since it uses {@link LogbackTestHelpers#resetLogback(String)} + * with {@link ch.qos.logback.classic.ClassicConstants#TEST_AUTOCONFIG_FILE} as its + * argument, instead of the no-arg method, it is slightly different than here. + */ +@DisplayName("LogbackTestHelpers (Integration Test)") +@ExtendWith(DropwizardExtensionsSupport.class) +@ExtendWith(ResetLogbackLoggingExtension.class) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +@Slf4j +public class LogbackTestHelpersIntegrationTest { + + private static final String APPENDER_NAME = "LogbackTestHelpersIntegrationTestAppender"; + + public static class MyConfig extends Configuration { + } + + public static class MyApp extends Application { + + @Override + public void run(MyConfig config, Environment environment) throws Exception { + // does nothing at all + + environment.healthChecks().register("noop", new HealthCheck() { + @Override + protected Result check() throws Exception { + return Result.healthy("Everything's fine here, how are you?"); + } + }); + } + } + + static final DropwizardAppExtension APP_EXTENSION = new DropwizardAppExtension<>(MyApp.class); + + private Logger logbackLogger; + private InMemoryAppender appender; + + @BeforeEach + void setUp() { + logbackLogger = getLogbackLogger(); + appender = getAppender(); + + assertThat(logbackLogger) + .describedAs("Getting the logger as a Logback Logger should always return the same Logger instance") + .isSameAs(LOG); + } + + @AfterEach + void tearDown() { + // This must re-fetch the appender, since the instance field can be null (for the first test) + var freshAppender = getAppender(); + if (nonNull(freshAppender)) { + freshAppender.clearEvents(); + } + } + + @Test + @Order(1) + void shouldResetDefaultLoggingConfiguration() { + assertThat(appender) + .describedAs("appender should be null; DropwizardAppExtension is expected to have reset Logback") + .isNull(); + + assertThatCode(() -> { + LOG.debug("message 1"); + LOG.info("message 2"); + LOG.warn("message 3"); + LOG.error("message 4"); + }) + .describedAs("We should still be able to log things (they just won't go anywhere)") + .doesNotThrowAnyException(); + + LogbackTestHelpers.resetLogback(); + + LOG.trace("message 5"); + LOG.debug("message 6"); + LOG.info("message 7"); + LOG.warn("message 8"); + LOG.error("message 9"); + + var resetAppender = getAppender(); + assertThat(resetAppender).isNotNull(); + + assertThat(resetAppender.orderedEventMessages()).containsExactly( + "message 6","message 7","message 8", "message 9"); + } + + @Test + @Order(2) + void shouldHaveAppenderOnceReset() { + assertThat(appender) + .describedAs("appender should not be null; previous test should have reset it") + .isNotNull(); + + LOG.trace("message 0"); + LOG.debug("message A"); + LOG.info("message B"); + LOG.warn("message C"); + + assertThat(appender.orderedEventMessages()).containsExactly( + "message A", "message B", "message C"); + } + + @Test + @Order(3) + void shouldStillHaveAppender() { + assertThat(appender) + .describedAs("appender should not be null; first test should have reset it") + .isNotNull(); + + LOG.debug("message Z"); + + assertThat(appender.orderedEventMessages()).containsExactly("message Z"); + } + + private static Logger getLogbackLogger() { + return (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(LogbackTestHelpersIntegrationTest.class); + } + + private InMemoryAppender getAppender() { + return (InMemoryAppender) logbackLogger.getAppender(APPENDER_NAME); + } +} diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java new file mode 100644 index 00000000..ffaab058 --- /dev/null +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java @@ -0,0 +1,34 @@ +package org.kiwiproject.test.logback; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; + +import ch.qos.logback.core.joran.spi.JoranException; + +/** + * Unit test for {@link LogbackTestHelpers}. This mainly tests invalid arguments + * and invalid Logback configuration. {@link LogbackTestHelpersIntegrationTest} + * performs a full integration test of the reset behavior. + */ +@DisplayName("LogbackTestHelpers") +@ExtendWith(ResetLogbackLoggingExtension.class) +public class LogbackTestHelpersTest { + + @Test + void shouldThrowIllegalArgument_WhenInvalidLogbackConfigPath() { + assertThatIllegalArgumentException() + .isThrownBy(() -> LogbackTestHelpers.resetLogback("dne.xml")); + } + + @Test + void shouldThrowUncheckedJoranException_WhenInvalidLogbackConfig() { + assertThatExceptionOfType(UncheckedJoranException.class) + .isThrownBy(() -> LogbackTestHelpers.resetLogback("LogbackTestHelpersTest/invalid-logback-test.xml")) + .withCauseExactlyInstanceOf(JoranException.class); + } +} diff --git a/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java b/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java new file mode 100644 index 00000000..7ffc159b --- /dev/null +++ b/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java @@ -0,0 +1,32 @@ +package org.kiwiproject.test.logback; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.junit.jupiter.api.Assertions.assertAll; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import ch.qos.logback.core.joran.spi.JoranException; + +@DisplayName("UncheckedJoranException") +class UncheckedJoranExceptionTest { + + @Test + void shouldRequireCause() { + assertAll( + () -> assertThatIllegalArgumentException().isThrownBy(() -> new UncheckedJoranException("oops", null)), + () -> assertThatIllegalArgumentException().isThrownBy(() -> new UncheckedJoranException(null)) + ); + } + + @Test + void shouldSetCause() { + var joranException = new JoranException("Invalid XML"); + + assertAll( + () -> assertThat(new UncheckedJoranException("oops", joranException)).hasCause(joranException), + () -> assertThat(new UncheckedJoranException(joranException)).hasCause(joranException) + ); + } +} diff --git a/src/test/resources/LogbackTestHelpersTest/invalid-logback-test.xml b/src/test/resources/LogbackTestHelpersTest/invalid-logback-test.xml new file mode 100644 index 00000000..7dcf49f6 --- /dev/null +++ b/src/test/resources/LogbackTestHelpersTest/invalid-logback-test.xml @@ -0,0 +1,18 @@ + + + + + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{5} - %msg%n + + + + + + + + + diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index 44d3f35d..727e53fa 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -26,6 +26,14 @@ + + + + + + + + From 2e2345c3d4ecf5ee504def326fbb9c3578f71e6e Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:57:20 -0500 Subject: [PATCH 02/16] Code cleanup * Remove redundant throws clauses * Replace duplicate code in InMemoryAppenderExtension with call to LogbackTestHelpers#resetLogback * Make visibility of Dropwizard Configuration classes in several tests the same as the Application class * Suppress "throwable not thrown" in UncheckedJoranExceptionTest --- .../jupiter/ResetLogbackLoggingExtension.java | 6 ++---- .../test/logback/InMemoryAppenderExtension.java | 17 +++-------------- .../test/logback/LogbackTestHelpers.java | 7 ++----- ...gresAppTestExtensionConfigOverridesTest.java | 2 +- ...tgresAppTestExtensionCustomPropertyTest.java | 2 +- .../app/PostgresAppTestExtensionTest.java | 2 +- .../junit/jupiter/Jdbi3DaoExtensionTest.java | 2 +- .../LogbackTestHelpersIntegrationTest.java | 9 +++------ .../test/logback/LogbackTestHelpersTest.java | 5 ++--- .../logback/UncheckedJoranExceptionTest.java | 4 ++-- 10 files changed, 18 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java index 17333f65..e6268bbe 100644 --- a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -1,17 +1,15 @@ package org.kiwiproject.test.junit.jupiter; +import ch.qos.logback.classic.ClassicConstants; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; - import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; import org.kiwiproject.test.logback.LogbackTestHelpers; -import ch.qos.logback.classic.ClassicConstants; - /** * A JUnit Jupiter {@link org.junit.jupiter.api.extension.Extension Extension} to reset * the Logback logging system after all tests have completed. @@ -64,7 +62,7 @@ public class ResetLogbackLoggingExtension implements AfterAllCallback { private String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; @Override - public void afterAll(ExtensionContext context) throws Exception { + public void afterAll(ExtensionContext context) { LogbackTestHelpers.resetLogback(logbackConfigFilePath); LOG.info("Logback was reset using configuration: {}", logbackConfigFilePath); } diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index fe73a66b..073e1452 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -5,13 +5,9 @@ import ch.qos.logback.classic.ClassicConstants; import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.Appender; -import ch.qos.logback.core.joran.spi.JoranException; import com.google.common.annotations.Beta; -import com.google.common.io.Resources; import lombok.Getter; import lombok.experimental.Accessors; import org.checkerframework.checker.nullness.qual.Nullable; @@ -153,7 +149,7 @@ public InMemoryAppenderExtension withLogbackConfigFilePath(String logbackConfigF * @param context the current extension context; never {@code null} */ @Override - public void beforeEach(ExtensionContext context) throws Exception { + public void beforeEach(ExtensionContext context) { var logbackLogger = (Logger) LoggerFactory.getLogger(loggerClass); var rawAppender = getAppender(logbackLogger); @@ -166,7 +162,7 @@ public void beforeEach(ExtensionContext context) throws Exception { @Nullable @SuppressWarnings("java:S106") - private Appender getAppender(Logger logbackLogger) throws JoranException { + private Appender getAppender(Logger logbackLogger) { var rawAppender = logbackLogger.getAppender(appenderName); if (nonNull(rawAppender)) { @@ -180,14 +176,7 @@ private Appender getAppender(Logger logbackLogger) throws JoranEx System.out.println("You can customize the logging configuration using #withLogbackConfigFilePath"); // Reset the Logback logging system - var loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); - loggerContext.stop(); - - var joranConfigurator = new JoranConfigurator(); - joranConfigurator.setContext(loggerContext); - var logbackConfigUrl = Resources.getResource(logbackConfigFilePath); - joranConfigurator.doConfigure(logbackConfigUrl); - loggerContext.start(); + LogbackTestHelpers.resetLogback(logbackConfigFilePath); // Try again and return whatever we get. It should not be null after resetting, unless // the reset failed, or the appender was not configured correctly. diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java index 1ae59edd..b724156a 100644 --- a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java @@ -1,14 +1,11 @@ package org.kiwiproject.test.logback; -import com.google.common.io.Resources; - -import lombok.experimental.UtilityClass; - import ch.qos.logback.classic.ClassicConstants; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.core.joran.spi.JoranException; - +import com.google.common.io.Resources; +import lombok.experimental.UtilityClass; import org.slf4j.LoggerFactory; /** diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java index 2463b066..a2ca64fc 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionConfigOverridesTest.java @@ -28,7 +28,7 @@ class PostgresAppTestExtensionConfigOverridesTest { @Getter @Setter - static class Config extends Configuration { + public static class Config extends Configuration { @Valid @NotNull @JsonProperty("database") diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java index 53e51d0c..e0854313 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionCustomPropertyTest.java @@ -29,7 +29,7 @@ class PostgresAppTestExtensionCustomPropertyTest { @Getter @Setter - static class Config extends Configuration { + public static class Config extends Configuration { @Valid @NotNull @JsonProperty("db") diff --git a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java index c6092702..42170a59 100644 --- a/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/dropwizard/app/PostgresAppTestExtensionTest.java @@ -26,7 +26,7 @@ class PostgresAppTestExtensionTest { @Getter @Setter - static class Config extends Configuration { + public static class Config extends Configuration { @Valid @NotNull @JsonProperty("database") diff --git a/src/test/java/org/kiwiproject/test/junit/jupiter/Jdbi3DaoExtensionTest.java b/src/test/java/org/kiwiproject/test/junit/jupiter/Jdbi3DaoExtensionTest.java index d21b5f70..d6ed6820 100644 --- a/src/test/java/org/kiwiproject/test/junit/jupiter/Jdbi3DaoExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/junit/jupiter/Jdbi3DaoExtensionTest.java @@ -110,7 +110,7 @@ void shouldNotBeAffectedByPreviousFailure() { } @Value - private static class TestTableValue { + public static class TestTableValue { String col1; int col2; } diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java index 7f02e489..1a5e14ae 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java @@ -4,15 +4,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import ch.qos.logback.classic.Logger; import com.codahale.metrics.health.HealthCheck; - import io.dropwizard.core.Application; import io.dropwizard.core.Configuration; import io.dropwizard.core.setup.Environment; import io.dropwizard.testing.junit5.DropwizardAppExtension; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import lombok.extern.slf4j.Slf4j; - import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -24,8 +23,6 @@ import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; import org.slf4j.LoggerFactory; -import ch.qos.logback.classic.Logger; - /** * This integration test uses DropwizardAppExtension which resets Logback when it * starts the test application class. It first verifies that there is no appender @@ -60,12 +57,12 @@ public static class MyConfig extends Configuration { public static class MyApp extends Application { @Override - public void run(MyConfig config, Environment environment) throws Exception { + public void run(MyConfig config, Environment environment) { // does nothing at all environment.healthChecks().register("noop", new HealthCheck() { @Override - protected Result check() throws Exception { + protected Result check() { return Result.healthy("Everything's fine here, how are you?"); } }); diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java index ffaab058..04198f98 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java @@ -3,13 +3,12 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import ch.qos.logback.core.joran.spi.JoranException; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; -import ch.qos.logback.core.joran.spi.JoranException; - /** * Unit test for {@link LogbackTestHelpers}. This mainly tests invalid arguments * and invalid Logback configuration. {@link LogbackTestHelpersIntegrationTest} @@ -17,7 +16,7 @@ */ @DisplayName("LogbackTestHelpers") @ExtendWith(ResetLogbackLoggingExtension.class) -public class LogbackTestHelpersTest { +class LogbackTestHelpersTest { @Test void shouldThrowIllegalArgument_WhenInvalidLogbackConfigPath() { diff --git a/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java b/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java index 7ffc159b..ca0829f1 100644 --- a/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java +++ b/src/test/java/org/kiwiproject/test/logback/UncheckedJoranExceptionTest.java @@ -4,14 +4,14 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assertions.assertAll; +import ch.qos.logback.core.joran.spi.JoranException; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import ch.qos.logback.core.joran.spi.JoranException; - @DisplayName("UncheckedJoranException") class UncheckedJoranExceptionTest { + @SuppressWarnings("ThrowableNotThrown") @Test void shouldRequireCause() { assertAll( From 98613f633f18a33510d387367282b466f6fd4da5 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 14:00:04 -0500 Subject: [PATCH 03/16] Code cleanup * Remove public modifier from LogbackTestHelpersIntegrationTest * Fix javadoc typo and grammar in LogbackTestHelpersIntegrationTest --- .../test/logback/LogbackTestHelpersIntegrationTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java index 1a5e14ae..b24619e2 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java @@ -27,7 +27,7 @@ * This integration test uses DropwizardAppExtension which resets Logback when it * starts the test application class. It first verifies that there is no appender * for this class' Logger, and then uses {@link LogbackTestHelpers#resetLogback()} - * to reset Loback to the default logging configuration. Finally, it ensures that + * to reset Logback to the default logging configuration. Finally, it ensures that * the InMemoryAppender was reset and that it receives the expected messages. *

* The tests are designed to execute in a specific order and use Jupiter's @@ -40,14 +40,14 @@ * {@link LogbackTestHelpers}, it might not work if there is actually a bug and * is therefore a bit circular. But, since it uses {@link LogbackTestHelpers#resetLogback(String)} * with {@link ch.qos.logback.classic.ClassicConstants#TEST_AUTOCONFIG_FILE} as its - * argument, instead of the no-arg method, it is slightly different than here. + * argument, instead of the no-arg method, it is slightly different from here. */ @DisplayName("LogbackTestHelpers (Integration Test)") @ExtendWith(DropwizardExtensionsSupport.class) @ExtendWith(ResetLogbackLoggingExtension.class) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) @Slf4j -public class LogbackTestHelpersIntegrationTest { +class LogbackTestHelpersIntegrationTest { private static final String APPENDER_NAME = "LogbackTestHelpersIntegrationTestAppender"; @@ -69,6 +69,7 @@ protected Result check() { } } + @SuppressWarnings("unused") static final DropwizardAppExtension APP_EXTENSION = new DropwizardAppExtension<>(MyApp.class); private Logger logbackLogger; From 702a6d048ce58765378dbbc27af7a7db8054ee1d Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 14:06:05 -0500 Subject: [PATCH 04/16] Code cleanup * Sonar java:S3551 - make UncheckedJoranException#getCause synchronized to math the synchronization of Throwable#getCause. While I get what Sonar is saying, I'm not entirely convinced this is really an issue. For example, why does java.io.UncheckedIOException override getCause but NOT make it synchronized? But go ahead and make Sonar happy... --- .../org/kiwiproject/test/logback/UncheckedJoranException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java b/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java index e5a0acd9..b7b18d70 100644 --- a/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java +++ b/src/main/java/org/kiwiproject/test/logback/UncheckedJoranException.java @@ -36,7 +36,7 @@ public UncheckedJoranException(JoranException cause) { * @return the {@link JoranException} which is the cause of this exception */ @Override - public JoranException getCause() { + public synchronized JoranException getCause() { return (JoranException) super.getCause(); } } From 7259c43c05272f6c333f11984836a1d0d13a148e Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:09:29 -0500 Subject: [PATCH 05/16] Enhance LogbackTestHelpers * Make resetLogback() try logback-test.xml, then logback.xml as fallback * Add varargs argument to resetLogback so that it accepts a primary location, and optionally fallback locations --- .../test/logback/LogbackTestHelpers.java | 58 +++++++++++++++++-- .../test/logback/LogbackTestHelpersTest.java | 49 ++++++++++++++-- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java index b724156a..03e7ebaa 100644 --- a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java @@ -1,13 +1,24 @@ package org.kiwiproject.test.logback; +import static com.google.common.base.Preconditions.checkArgument; +import static org.apache.commons.lang3.StringUtils.isNoneBlank; +import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotBlank; +import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotNull; + import ch.qos.logback.classic.ClassicConstants; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.core.joran.spi.JoranException; import com.google.common.io.Resources; import lombok.experimental.UtilityClass; +import org.apache.commons.collections4.ListUtils; +import org.checkerframework.checker.nullness.qual.Nullable; import org.slf4j.LoggerFactory; +import java.net.URL; +import java.util.List; +import java.util.Objects; + /** * Static test utilities that provide Logback-related functionality. */ @@ -16,32 +27,67 @@ public class LogbackTestHelpers { /** * Reset the Logback logging system using the default test configuration file ({@code logback-test.xml}). + * If that doesn't exist, try to fall back to the default configuration file ({@code logback.xml}). + *

+ * If you need a custom location (or locations), use {@link #resetLogback(String, String...)}. * - * @see ClassicConstants#TEST_AUTOCONFIG_FILE * @throws UncheckedJoranException if an error occurs resetting Logback + * @see ClassicConstants#TEST_AUTOCONFIG_FILE + * @see ClassicConstants#AUTOCONFIG_FILE */ public static void resetLogback() { - resetLogback(ClassicConstants.TEST_AUTOCONFIG_FILE); + resetLogback(ClassicConstants.TEST_AUTOCONFIG_FILE, ClassicConstants.AUTOCONFIG_FILE); } /** - * Reset the Logback logging system using the given configuration file. + * Reset the Logback logging system using the given configuration file or fallback configuration files. + *

+ * The fallback configurations are tried in the order they are provided. * - * @param logbackConfigFilePath the location of the custom Logback configuration file + * @param logbackConfigFile the location of the custom Logback configuration file + * @param fallbackConfigFiles additional locations to check for Logback configuration files * @throws UncheckedJoranException if an error occurs resetting Logback + * @throws IllegalArgumentException if none of the Logback configuration files exist */ - public static void resetLogback(String logbackConfigFilePath) { + public static void resetLogback(String logbackConfigFile, String... fallbackConfigFiles) { + checkArgumentNotBlank(logbackConfigFile, "logbackConfigFile must not be blank"); + checkArgumentNotNull(fallbackConfigFiles, "fallback locations vararg parameter must not be null"); + checkArgument(isNoneBlank(fallbackConfigFiles), "fallbackConfigFiles must not contain blank locations"); + try { var loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); loggerContext.stop(); var joranConfigurator = new JoranConfigurator(); joranConfigurator.setContext(loggerContext); - var logbackConfigUrl = Resources.getResource(logbackConfigFilePath); + var logbackConfigUrl = getFirstLogbackConfigOrThrow(logbackConfigFile, fallbackConfigFiles); joranConfigurator.doConfigure(logbackConfigUrl); loggerContext.start(); } catch (JoranException e) { throw new UncheckedJoranException(e); } } + + private static URL getFirstLogbackConfigOrThrow(String logbackConfigFilePath, String... fallbackConfigFilePaths) { + var allConfigs = ListUtils.union( + List.of(logbackConfigFilePath), + List.of(fallbackConfigFilePaths) + ); + + return allConfigs.stream() + .map(LogbackTestHelpers::getResourceOrNull) + .filter(Objects::nonNull) + .findFirst() + .orElseThrow(() -> + new IllegalArgumentException("Did not find any of the Logback configurations: " + allConfigs)); + } + + @Nullable + private static URL getResourceOrNull(String resourceName) { + try { + return Resources.getResource(resourceName); + } catch (IllegalArgumentException e) { + return null; + } + } } diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java index 04198f98..803f1010 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersTest.java @@ -5,9 +5,12 @@ import ch.qos.logback.core.joran.spi.JoranException; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; import org.kiwiproject.test.junit.jupiter.ResetLogbackLoggingExtension; +import org.kiwiproject.test.junit.jupiter.params.provider.MinimalBlankStringSource; /** * Unit test for {@link LogbackTestHelpers}. This mainly tests invalid arguments @@ -18,16 +21,52 @@ @ExtendWith(ResetLogbackLoggingExtension.class) class LogbackTestHelpersTest { - @Test - void shouldThrowIllegalArgument_WhenInvalidLogbackConfigPath() { - assertThatIllegalArgumentException() - .isThrownBy(() -> LogbackTestHelpers.resetLogback("dne.xml")); + @Nested + class ThrowsIllegalArgumentException { + + @Test + void whenInvalidLogbackConfigPath() { + assertThatIllegalArgumentException() + .isThrownBy(() -> LogbackTestHelpers.resetLogback("dne.xml")) + .withMessage("Did not find any of the Logback configurations: [dne.xml]"); + } + + @Test + void whenInvalidLogbackConfigPaths() { + assertThatIllegalArgumentException().isThrownBy(() -> + LogbackTestHelpers.resetLogback("dne1.xml", "dne2.xml", "dne3.xml")) + .withMessage("Did not find any of the Logback configurations: [dne1.xml, dne2.xml, dne3.xml]"); + } + + @ParameterizedTest + @MinimalBlankStringSource + void whenGivenBlankConfigLocation(String location) { + assertThatIllegalArgumentException() + .isThrownBy(() -> LogbackTestHelpers.resetLogback(location)) + .withMessage("logbackConfigFile must not be blank"); + } + + @Test + void whenExplicitNullFallbackLocationsIsGiven() { + assertThatIllegalArgumentException() + .isThrownBy(() -> LogbackTestHelpers.resetLogback("acme-logback.xml", (String[]) null)) + .withMessage("fallback locations vararg parameter must not be null"); + } + + @ParameterizedTest + @MinimalBlankStringSource + void whenFallbackLocationIsBlank(String fallbackLocation) { + assertThatIllegalArgumentException() + .isThrownBy(() -> LogbackTestHelpers.resetLogback("acme-test-logback.xml", fallbackLocation)) + .withMessage("fallbackConfigFiles must not contain blank locations"); + } } @Test void shouldThrowUncheckedJoranException_WhenInvalidLogbackConfig() { assertThatExceptionOfType(UncheckedJoranException.class) - .isThrownBy(() -> LogbackTestHelpers.resetLogback("LogbackTestHelpersTest/invalid-logback-test.xml")) + .isThrownBy(() -> + LogbackTestHelpers.resetLogback("LogbackTestHelpersTest/invalid-logback-test.xml")) .withCauseExactlyInstanceOf(JoranException.class); } } From 4793c9fa47e0df809bf94d985ca51ee25ed33ca5 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:17:13 -0500 Subject: [PATCH 06/16] Code cleanup: add test for InMemoryAppenderExtension All it does right now is validate withLogbackConfigFilePath --- .../logback/InMemoryAppenderExtension.java | 1 + .../InMemoryAppenderExtensionTest.java | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index 073e1452..72fa3e13 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -34,6 +34,7 @@ public class InMemoryAppenderExtension implements BeforeEachCallback, AfterEachC private final String appenderName; // Use the default Logback test configuration file as our default value. + @Getter private String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; @Getter diff --git a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java new file mode 100644 index 00000000..076cd158 --- /dev/null +++ b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java @@ -0,0 +1,29 @@ +package org.kiwiproject.test.logback; + +import static org.assertj.core.api.Assertions.assertThat; + +import ch.qos.logback.classic.ClassicConstants; +import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +@DisplayName("InMemoryAppenderExtension") +@Slf4j +class InMemoryAppenderExtensionTest { + + @Test + void shouldUseLogbackTestFileAsDefaultConfigLocation() { + var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class); + assertThat(extension.getLogbackConfigFilePath()) + .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + } + + @Test + void shouldAcceptCustomConfigLocation() { + var customLocation = "acme-test-logback.xml"; + var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class) + .withLogbackConfigFilePath(customLocation); + + assertThat(extension.getLogbackConfigFilePath()).isEqualTo(customLocation); + } +} From c72d357bc52b96f757eaa5bfa050c0eeb7e18c83 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:20:40 -0500 Subject: [PATCH 07/16] Docs: remove confusing javadoc --- .../test/logback/LogbackTestHelpersIntegrationTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java index b24619e2..2a6e2af6 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelpersIntegrationTest.java @@ -38,9 +38,7 @@ * In case of failure, this test uses the ResetLogbackLoggingExtension to restore * the Logback logging configuration. However, since that extension simply uses * {@link LogbackTestHelpers}, it might not work if there is actually a bug and - * is therefore a bit circular. But, since it uses {@link LogbackTestHelpers#resetLogback(String)} - * with {@link ch.qos.logback.classic.ClassicConstants#TEST_AUTOCONFIG_FILE} as its - * argument, instead of the no-arg method, it is slightly different from here. + * is therefore a bit circular. */ @DisplayName("LogbackTestHelpers (Integration Test)") @ExtendWith(DropwizardExtensionsSupport.class) From 347166b2f8d48fdaae102163cf5695d611c127e4 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:26:36 -0500 Subject: [PATCH 08/16] Add test for ResetLogbackLoggingExtension * Make logbackConfigFilePath final in ResetLogbackLoggingExtension * Add test that verifies default and custom config locations --- .../jupiter/ResetLogbackLoggingExtension.java | 5 ++- .../ResetLogbackLoggingExtensionTest.java | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java index e6268bbe..63393288 100644 --- a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -4,6 +4,7 @@ import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; +import lombok.Getter; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.extension.AfterAllCallback; @@ -56,10 +57,12 @@ @NoArgsConstructor @AllArgsConstructor(access = AccessLevel.PACKAGE) @Slf4j +@SuppressWarnings("LombokGetterMayBeUsed") public class ResetLogbackLoggingExtension implements AfterAllCallback { + @Getter @Builder.Default - private String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; + private final String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; @Override public void afterAll(ExtensionContext context) { diff --git a/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java b/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java new file mode 100644 index 00000000..9c145b0d --- /dev/null +++ b/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java @@ -0,0 +1,36 @@ +package org.kiwiproject.test.junit.jupiter; + +import static org.assertj.core.api.Assertions.assertThat; + +import ch.qos.logback.classic.ClassicConstants; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +@DisplayName("ResetLogbackLoggingExtension") +class ResetLogbackLoggingExtensionTest { + + @Test + void shouldConstructWithLogbackTestFileAsDefaultConfigLocation() { + var extension = new ResetLogbackLoggingExtension(); + assertThat(extension.getLogbackConfigFilePath()) + .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + } + + @Test + void shouldBuildWithLogbackTestFileAsDefaultConfigLocation() { + var extension = ResetLogbackLoggingExtension.builder().build(); + assertThat(extension.getLogbackConfigFilePath()) + .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + } + + @Test + void shouldAllowCustomConfigLocation() { + var customLocation = "acme-test-logback.xml"; + + var extension = ResetLogbackLoggingExtension.builder() + .logbackConfigFilePath(customLocation) + .build(); + + assertThat(extension.getLogbackConfigFilePath()).isEqualTo(customLocation); + } +} From 6e9883b0f1c945c55d370b90506325be992a0cca Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:02:05 -0500 Subject: [PATCH 09/16] Change extensions to use default Logback configs The reason to do this is to provide more flexibility. Unless a custom Logback config file is provided, use the LogbackTestHelpers.resetLogback() method which attempts both logback-test.xml with logback.xml as a fallback. --- .../junit/jupiter/ResetLogbackLoggingExtension.java | 12 ++++++++---- .../test/logback/InMemoryAppenderExtension.java | 11 +++++++---- .../jupiter/ResetLogbackLoggingExtensionTest.java | 11 ++++------- .../test/logback/InMemoryAppenderExtensionTest.java | 4 +--- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java index 63393288..0f842573 100644 --- a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -1,6 +1,7 @@ package org.kiwiproject.test.junit.jupiter; -import ch.qos.logback.classic.ClassicConstants; +import static org.apache.commons.lang3.StringUtils.isBlank; + import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; @@ -61,12 +62,15 @@ public class ResetLogbackLoggingExtension implements AfterAllCallback { @Getter - @Builder.Default - private final String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; + private String logbackConfigFilePath; @Override public void afterAll(ExtensionContext context) { - LogbackTestHelpers.resetLogback(logbackConfigFilePath); + if (isBlank(logbackConfigFilePath)) { + LogbackTestHelpers.resetLogback(); + } else { + LogbackTestHelpers.resetLogback(logbackConfigFilePath); + } LOG.info("Logback was reset using configuration: {}", logbackConfigFilePath); } } diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index 72fa3e13..0f714ccf 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -1,9 +1,9 @@ package org.kiwiproject.test.logback; import static java.util.Objects.nonNull; +import static org.apache.commons.lang3.StringUtils.isBlank; import static org.assertj.core.api.Assertions.assertThat; -import ch.qos.logback.classic.ClassicConstants; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.Appender; @@ -33,9 +33,8 @@ public class InMemoryAppenderExtension implements BeforeEachCallback, AfterEachC private final Class loggerClass; private final String appenderName; - // Use the default Logback test configuration file as our default value. @Getter - private String logbackConfigFilePath = ClassicConstants.TEST_AUTOCONFIG_FILE; + private String logbackConfigFilePath; @Getter @Accessors(fluent = true) @@ -177,7 +176,11 @@ private Appender getAppender(Logger logbackLogger) { System.out.println("You can customize the logging configuration using #withLogbackConfigFilePath"); // Reset the Logback logging system - LogbackTestHelpers.resetLogback(logbackConfigFilePath); + if (isBlank(logbackConfigFilePath)) { + LogbackTestHelpers.resetLogback(); + } else { + LogbackTestHelpers.resetLogback(logbackConfigFilePath); + } // Try again and return whatever we get. It should not be null after resetting, unless // the reset failed, or the appender was not configured correctly. diff --git a/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java b/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java index 9c145b0d..cfa4b9a1 100644 --- a/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtensionTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import ch.qos.logback.classic.ClassicConstants; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -10,17 +9,15 @@ class ResetLogbackLoggingExtensionTest { @Test - void shouldConstructWithLogbackTestFileAsDefaultConfigLocation() { + void shouldConstructWithNullAsDefaultConfigLocation() { var extension = new ResetLogbackLoggingExtension(); - assertThat(extension.getLogbackConfigFilePath()) - .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + assertThat(extension.getLogbackConfigFilePath()).isNull(); } @Test - void shouldBuildWithLogbackTestFileAsDefaultConfigLocation() { + void shouldBuildWithNullAsDefaultConfigLocation() { var extension = ResetLogbackLoggingExtension.builder().build(); - assertThat(extension.getLogbackConfigFilePath()) - .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + assertThat(extension.getLogbackConfigFilePath()).isNull(); } @Test diff --git a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java index 076cd158..d817921f 100644 --- a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import ch.qos.logback.classic.ClassicConstants; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -14,8 +13,7 @@ class InMemoryAppenderExtensionTest { @Test void shouldUseLogbackTestFileAsDefaultConfigLocation() { var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class); - assertThat(extension.getLogbackConfigFilePath()) - .isEqualTo(ClassicConstants.TEST_AUTOCONFIG_FILE); + assertThat(extension.getLogbackConfigFilePath()).isNull(); } @Test From d7d82373915d073e848ab5cadaef6e27d79b4edb Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:30:46 -0500 Subject: [PATCH 10/16] Introduce instance-based LogbackTestHelper to help testing Add LogbackTestHelper which mainly delegates to LogbackTestHelpers. It adds a new method, resetLogbackWithDefaultOrConfig, which removes the duplicate conditional logic in the InMemoryAppenderExtension and the ResetLogbackLoggingExtension. It also allows for mocking in tests of the Logback reset mechanisms and extensions. --- .../jupiter/ResetLogbackLoggingExtension.java | 19 ++++---- .../logback/InMemoryAppenderExtension.java | 13 +++--- .../test/logback/LogbackTestHelper.java | 46 +++++++++++++++++++ .../test/logback/LogbackTestHelperTest.java | 42 +++++++++++++++++ 4 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 src/main/java/org/kiwiproject/test/logback/LogbackTestHelper.java create mode 100644 src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java index 0f842573..49603ca0 100644 --- a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -1,7 +1,6 @@ package org.kiwiproject.test.junit.jupiter; -import static org.apache.commons.lang3.StringUtils.isBlank; - +import com.google.common.annotations.VisibleForTesting; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; @@ -10,7 +9,7 @@ import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; -import org.kiwiproject.test.logback.LogbackTestHelpers; +import org.kiwiproject.test.logback.LogbackTestHelper; /** * A JUnit Jupiter {@link org.junit.jupiter.api.extension.Extension Extension} to reset @@ -66,11 +65,13 @@ public class ResetLogbackLoggingExtension implements AfterAllCallback { @Override public void afterAll(ExtensionContext context) { - if (isBlank(logbackConfigFilePath)) { - LogbackTestHelpers.resetLogback(); - } else { - LogbackTestHelpers.resetLogback(logbackConfigFilePath); - } - LOG.info("Logback was reset using configuration: {}", logbackConfigFilePath); + getLogbackTestHelper().resetLogbackWithDefaultOrConfig(logbackConfigFilePath); + LOG.debug("Logback was reset using configuration: {} (if null, the Logback defaults are used)", + logbackConfigFilePath); + } + + @VisibleForTesting + protected LogbackTestHelper getLogbackTestHelper() { + return new LogbackTestHelper(); } } diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index 0f714ccf..1b4e8c5d 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -1,13 +1,13 @@ package org.kiwiproject.test.logback; import static java.util.Objects.nonNull; -import static org.apache.commons.lang3.StringUtils.isBlank; import static org.assertj.core.api.Assertions.assertThat; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.Appender; import com.google.common.annotations.Beta; +import com.google.common.annotations.VisibleForTesting; import lombok.Getter; import lombok.experimental.Accessors; import org.checkerframework.checker.nullness.qual.Nullable; @@ -176,17 +176,18 @@ private Appender getAppender(Logger logbackLogger) { System.out.println("You can customize the logging configuration using #withLogbackConfigFilePath"); // Reset the Logback logging system - if (isBlank(logbackConfigFilePath)) { - LogbackTestHelpers.resetLogback(); - } else { - LogbackTestHelpers.resetLogback(logbackConfigFilePath); - } + getLogbackTestHelper().resetLogbackWithDefaultOrConfig(logbackConfigFilePath); // Try again and return whatever we get. It should not be null after resetting, unless // the reset failed, or the appender was not configured correctly. return logbackLogger.getAppender(appenderName); } + @VisibleForTesting + protected LogbackTestHelper getLogbackTestHelper() { + return new LogbackTestHelper(); + } + /** * Clears all logging events from the {@link InMemoryAppender} to ensure each * test starts with an empty appender. diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelper.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelper.java new file mode 100644 index 00000000..d25d1fc4 --- /dev/null +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelper.java @@ -0,0 +1,46 @@ +package org.kiwiproject.test.logback; + +import static org.apache.commons.lang3.StringUtils.isBlank; + +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Provides utilities for Logback-related functionality. + *

+ * This is an instance-based utility class, and is mainly useful if you need to mock + * its behavior. By default, it delegates to {@link LogbackTestHelpers} for methods + * that have the same signature. + */ +public class LogbackTestHelper { + + /** + * Resets Logback using either the given config file, or uses the defaults + * as provided by {@link LogbackTestHelpers#resetLogback()}. + * + * @param logbackConfigFile the Logback config file to use, or null + */ + public void resetLogbackWithDefaultOrConfig(@Nullable String logbackConfigFile) { + if (isBlank(logbackConfigFile)) { + resetLogback(); + } else { + resetLogback(logbackConfigFile); + } + } + + /** + * Delegates to {@link LogbackTestHelpers#resetLogback()}. + */ + public void resetLogback() { + LogbackTestHelpers.resetLogback(); + } + + /** + * Delegates to {@link LogbackTestHelpers#resetLogback(String, String...)}. + * + * @param logbackConfigFile the location of the custom Logback configuration file + * @param fallbackConfigFiles additional locations to check for Logback configuration files + */ + public void resetLogback(String logbackConfigFile, String... fallbackConfigFiles) { + LogbackTestHelpers.resetLogback(logbackConfigFile, fallbackConfigFiles); + } +} diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java new file mode 100644 index 00000000..2d46c04a --- /dev/null +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java @@ -0,0 +1,42 @@ +package org.kiwiproject.test.logback; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.kiwiproject.test.junit.jupiter.params.provider.MinimalBlankStringSource; + +@DisplayName("Logback") +class LogbackTestHelperTest { + + private LogbackTestHelper helper; + + @BeforeEach + void setUp() { + helper = spy(new LogbackTestHelper()); + + doNothing().when(helper).resetLogback(); + doNothing().when(helper).resetLogback(anyString()); + } + + @Test + void shouldResetLogbackWithCustomConfiguration() { + var customConfig = "test-acme-logback.xml"; + helper.resetLogbackWithDefaultOrConfig(customConfig); + + verify(helper).resetLogback(customConfig); + } + + @ParameterizedTest + @MinimalBlankStringSource + void shouldUseDefaultWhenCustomConfigurationIsBlank(String configFile) { + helper.resetLogbackWithDefaultOrConfig(configFile); + + verify(helper).resetLogback(); + } +} From 19792af83334dc19334a61f5696b25df26b655cf Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:52:25 -0500 Subject: [PATCH 11/16] Add "clear-box" tests for InMemoryAppenderExtension#getAppender --- .../logback/InMemoryAppenderExtension.java | 3 +- .../InMemoryAppenderExtensionTest.java | 60 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index 1b4e8c5d..d065c11e 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -161,8 +161,9 @@ public void beforeEach(ExtensionContext context) { } @Nullable + @VisibleForTesting @SuppressWarnings("java:S106") - private Appender getAppender(Logger logbackLogger) { + Appender getAppender(Logger logbackLogger) { var rawAppender = logbackLogger.getAppender(appenderName); if (nonNull(rawAppender)) { diff --git a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java index d817921f..1b5c88be 100644 --- a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java @@ -1,10 +1,19 @@ package org.kiwiproject.test.logback; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; +import ch.qos.logback.classic.Logger; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.kiwiproject.test.junit.jupiter.ClearBoxTest; @DisplayName("InMemoryAppenderExtension") @Slf4j @@ -24,4 +33,55 @@ void shouldAcceptCustomConfigLocation() { assertThat(extension.getLogbackConfigFilePath()).isEqualTo(customLocation); } + + @ClearBoxTest + void shouldGetAppenderWhenExists() { + var logbackLogger = mock(Logger.class); + var appender = new InMemoryAppender(); + when(logbackLogger.getAppender(anyString())).thenReturn(appender); + + var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class); + + var returnedAppender = extension.getAppender(logbackLogger); + assertThat(returnedAppender).isSameAs(appender); + + verify(logbackLogger, only()).getAppender(expectedAppenderName()); + } + + private static String expectedAppenderName() { + return InMemoryAppenderExtensionTest.class.getSimpleName() + "Appender"; + } + + @ClearBoxTest + void shouldResetLogbackWhenAppenderDoesNotExist() { + var logbackLogger = mock(Logger.class); + var appender = new InMemoryAppender(); + + // Simulate initially not getting the appender + // then getting an appender (once Logback has been reset) + when(logbackLogger.getAppender(anyString())) + .thenReturn(null) + .thenReturn(appender); + + var logbackTestHelper = mock(LogbackTestHelper.class); + + var appenderName = "MyAppender"; + var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class, appenderName) { + @Override + protected LogbackTestHelper getLogbackTestHelper() { + return logbackTestHelper; + } + }; + + var customConfig = "acme-logback-test.xml"; + extension.withLogbackConfigFilePath(customConfig); + + var returnedAppender = extension.getAppender(logbackLogger); + assertThat(returnedAppender).isSameAs(appender); + + verify(logbackLogger, times(2)).getAppender(appenderName); + verifyNoMoreInteractions(logbackLogger); + + verify(logbackTestHelper, only()).resetLogbackWithDefaultOrConfig(customConfig); + } } From 935f7c91cd595ce683fea23309dbf7a452602eb9 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 18:00:13 -0500 Subject: [PATCH 12/16] Add test to, um, well, cover the getLogbackTestHelper method Since it gets mocked, it's never tested. So, this tests it. And hey, it could theoretically break if someone changed it to return null! --- .../test/logback/InMemoryAppenderExtensionTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java index 1b5c88be..6f1cff48 100644 --- a/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java +++ b/src/test/java/org/kiwiproject/test/logback/InMemoryAppenderExtensionTest.java @@ -34,6 +34,18 @@ void shouldAcceptCustomConfigLocation() { assertThat(extension.getLogbackConfigFilePath()).isEqualTo(customLocation); } + @ClearBoxTest + void shouldReturnNewLogbackTestHelperInstances() { + var extension = new InMemoryAppenderExtension(InMemoryAppenderExtensionTest.class); + + var helper1 = extension.getLogbackTestHelper(); + var helper2 = extension.getLogbackTestHelper(); + var helper3 = extension.getLogbackTestHelper(); + + assertThat(helper1).isNotSameAs(helper2); + assertThat(helper2).isNotSameAs(helper3); + } + @ClearBoxTest void shouldGetAppenderWhenExists() { var logbackLogger = mock(Logger.class); From f0822ab2653ed42a50c00f602643aa7b4024318b Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 18:08:55 -0500 Subject: [PATCH 13/16] Add one more test to cover LogbackTestHelper#resetLogback with fallback This method is mocked in other tests, so is never exercised. Test the delegation to LogbackTestHelpers by passing arguments that don't exist, and validating the exception we get. Kind of invasive but gets the job done without actually resetting Logback, which we generally want to avoid. --- .../test/logback/LogbackTestHelperTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java index 2d46c04a..af2abe1a 100644 --- a/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java +++ b/src/test/java/org/kiwiproject/test/logback/LogbackTestHelperTest.java @@ -1,5 +1,6 @@ package org.kiwiproject.test.logback; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.spy; @@ -39,4 +40,15 @@ void shouldUseDefaultWhenCustomConfigurationIsBlank(String configFile) { verify(helper).resetLogback(); } + + @Test + void shouldDelegateToLogbackTestHelpersWithFallback() { + // Verify the delegation without actually resetting Logback + // by passing in config files that don't exist. + + assertThatIllegalArgumentException() + .isThrownBy(() -> + new LogbackTestHelper().resetLogback("acme-test-logback.xml", "acme-logback.xml")) + .withMessage("Did not find any of the Logback configurations: [acme-test-logback.xml, acme-logback.xml]"); + } } From d04977801b04c6fbe3c0495c89bf2ba4405afb22 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Fri, 9 Feb 2024 18:12:57 -0500 Subject: [PATCH 14/16] Make javadocs provide more information about the Logback configs --- .../test/junit/jupiter/ResetLogbackLoggingExtension.java | 6 ++++++ .../kiwiproject/test/logback/InMemoryAppenderExtension.java | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java index 49603ca0..2bbe4320 100644 --- a/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java +++ b/src/main/java/org/kiwiproject/test/junit/jupiter/ResetLogbackLoggingExtension.java @@ -60,6 +60,12 @@ @SuppressWarnings("LombokGetterMayBeUsed") public class ResetLogbackLoggingExtension implements AfterAllCallback { + /** + * A custom location for the Logback configuration. + *

+ * If this is not set, then the default Logback configuration files are used + * in the order {@code logback-test.xml} followed by {@code logback.xml}. + */ @Getter private String logbackConfigFilePath; diff --git a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java index d065c11e..befb383c 100644 --- a/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java +++ b/src/main/java/org/kiwiproject/test/logback/InMemoryAppenderExtension.java @@ -102,9 +102,10 @@ public InMemoryAppenderExtension(Class loggerClass, String appenderName) { } /** - * The Logback configuration to use if the logging system needs to be reset. + * The custom Logback configuration to use if the logging system needs to be reset. *

* For example: + * * 

      * {@literal @}RegisterExtension
      *  private final InMemoryAppenderExtension inMemoryAppenderExtension =
@@ -112,6 +113,9 @@ public InMemoryAppenderExtension(Class loggerClass, String appenderName) {
      *                  .withLogbackConfigFilePath("acme-logback-test.xml");
      *
* + * If this is not set, then the default Logback configuration files are used + * in the order {@code logback-test.xml} followed by {@code logback.xml}. + * * @param logbackConfigFilePath the location of the custom Logback configuration file * @return this extension, so this can be chained after the constructor * @see Tests failing because Logback appenders don't exist (#457) From b65b76217d43bfac0dc20ee4b50e7249b90037b1 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Sat, 10 Feb 2024 16:04:17 -0500 Subject: [PATCH 15/16] Don't stop the LoggerContext unless the config file exists --- .../java/org/kiwiproject/test/logback/LogbackTestHelpers.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java index 03e7ebaa..88e54a99 100644 --- a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java @@ -55,12 +55,13 @@ public static void resetLogback(String logbackConfigFile, String... fallbackConf checkArgument(isNoneBlank(fallbackConfigFiles), "fallbackConfigFiles must not contain blank locations"); try { + var logbackConfigUrl = getFirstLogbackConfigOrThrow(logbackConfigFile, fallbackConfigFiles); + var loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); loggerContext.stop(); var joranConfigurator = new JoranConfigurator(); joranConfigurator.setContext(loggerContext); - var logbackConfigUrl = getFirstLogbackConfigOrThrow(logbackConfigFile, fallbackConfigFiles); joranConfigurator.doConfigure(logbackConfigUrl); loggerContext.start(); } catch (JoranException e) { From 4b2ff78a7227229df574e9a7b2c97b2d72a8b5eb Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Sat, 10 Feb 2024 16:28:30 -0500 Subject: [PATCH 16/16] Clarify javadoc; we only try to reset using the first one that exists --- .../org/kiwiproject/test/logback/LogbackTestHelpers.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java index 88e54a99..35c18080 100644 --- a/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java +++ b/src/main/java/org/kiwiproject/test/logback/LogbackTestHelpers.java @@ -40,9 +40,11 @@ public static void resetLogback() { } /** - * Reset the Logback logging system using the given configuration file or fallback configuration files. + * Reset the Logback logging system using the given configuration file. + * If the primary file does not exist, use the first fallback configuration + * file that exists. If the reset fails, an exception is thrown immediately. *

- * The fallback configurations are tried in the order they are provided. + * The fallback configurations are searched in the order they are provided. * * @param logbackConfigFile the location of the custom Logback configuration file * @param fallbackConfigFiles additional locations to check for Logback configuration files