You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/09/16 09:51:40 UTC

(logging-log4j2) branch fix/2.x/use-random-logging-path created (now 102a1a31aa)

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a change to branch fix/2.x/use-random-logging-path
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


      at 102a1a31aa Use a random `${test:logging.path}`

This branch includes the following new commits:

     new 102a1a31aa Use a random `${test:logging.path}`

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(logging-log4j2) 01/01: Use a random `${test:logging.path}`

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch fix/2.x/use-random-logging-path
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 102a1a31aa7e1a64caf9612688b7cd38a140069a
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Sep 16 11:51:14 2024 +0200

    Use a random `${test:logging.path}`
    
    The `@TempLoggingDir` JUnit 5 extensions create a totally deterministic directory to hold the log files for tests.
    
    Since the directory is deleted only if all the tests succeed, this causes a problem with test reruns.
---
 .../log4j/test/junit/TempLoggingDirectory.java     | 44 ++++++++++++++++------
 .../log4j/test/junit/TempLoggingDirectoryTest.java |  6 ++-
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java
index d222f6dc17..ef97ff2378 100644
--- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java
+++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java
@@ -51,7 +51,8 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
         Path loggingPath = null;
         for (final Field field : fields) {
             if (loggingPath != null) {
-                StatusLogger.getLogger().warn("Multiple fields with @TempLoggingDir annotation are not supported.");
+                StatusLogger.getLogger()
+                        .warn("Multiple static fields with @TempLoggingDir annotation are not supported.");
             } else {
                 final CleanupMode cleanup = determineCleanupMode(field);
                 loggingPath = createLoggingPath(context, cleanup).getPath();
@@ -63,8 +64,8 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
 
     @Override
     public void beforeEach(ExtensionContext context) throws Exception {
-        // JUnit 5 does not set an error on the parent context if one of the children
-        // fail. We record the list of children.
+        // JUnit 5 does not set an error on the parent context if one of the children fails.
+        // We record the list of children.
         context.getParent().ifPresent(c -> {
             final PathHolder holder = ExtensionContextAnchor.getAttribute(PathHolder.class, PathHolder.class, c);
             if (holder != null) {
@@ -78,7 +79,8 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
         final Object instance = context.getRequiredTestInstance();
         for (final Field field : fields) {
             if (loggingPath != null) {
-                StatusLogger.getLogger().warn("Multiple fields with @TempLoggingDir annotation are not supported.");
+                StatusLogger.getLogger()
+                        .warn("Multiple instance fields with @TempLoggingDir annotation are not supported.");
             } else {
                 final CleanupMode cleanup = determineCleanupMode(field);
                 loggingPath = createLoggingPath(context, cleanup).getPath();
@@ -102,7 +104,7 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
             throws ParameterResolutionException {
         final TempLoggingDir annotation =
                 parameterContext.findAnnotation(TempLoggingDir.class).get();
-        // Get or create temporary directory
+        // Get or create a temporary directory
         PathHolder holder = ExtensionContextAnchor.getAttribute(PathHolder.class, PathHolder.class, extensionContext);
         if (holder == null || !extensionContext.equals(holder.getMainContext())) {
             final CleanupMode mode = determineCleanupMode(annotation);
@@ -111,14 +113,9 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
         return holder.getPath();
     }
 
-    private PathHolder createLoggingPath(final ExtensionContext context, final CleanupMode cleanup) {
+    private PathHolder createLoggingPath(ExtensionContext context, CleanupMode cleanup) {
         final TestProperties props = TestPropertySource.createProperties(context);
-        // Create temporary directory
-        final String baseDir = System.getProperty("basedir");
-        final Path basePath = (baseDir != null ? Paths.get(baseDir, "target") : Paths.get(".")).resolve("logs");
-        final Class<?> clazz = context.getRequiredTestClass();
-        final String dir = clazz.getName().replaceAll("[.$]", File.separatorChar == '\\' ? "\\\\" : File.separator);
-        final Path perClassPath = basePath.resolve(dir);
+        final Path perClassPath = determinePerClassPath(context);
         // Per test subfolder
         final Path loggingPath = context.getTestMethod()
                 .map(m -> perClassPath.resolve(m.getName()))
@@ -135,6 +132,29 @@ public class TempLoggingDirectory implements BeforeAllCallback, BeforeEachCallba
         return holder;
     }
 
+    private Path determinePerClassPath(ExtensionContext context) {
+        // Check if the parent context already created a folder
+        PathHolder holder = ExtensionContextAnchor.getAttribute(PathHolder.class, PathHolder.class, context);
+        if (holder == null) {
+            try {
+                // Create temporary per-class directory
+                final String baseDir = System.getProperty("basedir");
+                final Path basePath = (baseDir != null ? Paths.get(baseDir, "target") : Paths.get(".")).resolve("logs");
+                final Class<?> clazz = context.getRequiredTestClass();
+                final Package pkg = clazz.getPackage();
+                final String dir =
+                        pkg.getName().replaceAll("[.$]", File.separatorChar == '\\' ? "\\\\" : File.separator);
+                // Create a temporary directory that uses the simple class name as prefix
+                Path packagePath = basePath.resolve(dir);
+                Files.createDirectories(packagePath);
+                return Files.createTempDirectory(packagePath, clazz.getSimpleName());
+            } catch (final IOException e) {
+                throw new ExtensionContextException("Failed to create temporary directory.", e);
+            }
+        }
+        return holder.getPath();
+    }
+
     private CleanupMode determineCleanupMode(final TempLoggingDir annotation) {
         final CleanupMode mode = annotation.cleanup();
         // TODO: use JupiterConfiguration
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/test/junit/TempLoggingDirectoryTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/test/junit/TempLoggingDirectoryTest.java
index 846a044e79..0a1dd2f15f 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/test/junit/TempLoggingDirectoryTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/test/junit/TempLoggingDirectoryTest.java
@@ -20,13 +20,14 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.regex.Pattern;
 import org.apache.logging.log4j.test.TestProperties;
 import org.junit.jupiter.api.Test;
 
 @UsingTestProperties
 public class TempLoggingDirectoryTest {
 
-    private static final Path PER_CLASS_PATH = Paths.get("TempLoggingDirectoryTest");
+    private static final Pattern PER_CLASS_PATH = Pattern.compile("TempLoggingDirectoryTest\\d+");
     private static final Path PER_TEST_PATH = Paths.get("testInjectedFields");
 
     @TempLoggingDir
@@ -37,7 +38,8 @@ public class TempLoggingDirectoryTest {
 
     @Test
     void testInjectedFields(final @TempLoggingDir Path parameterLoggingPath, final TestProperties props) {
-        assertThat(staticLoggingPath).exists().endsWith(PER_CLASS_PATH);
+        assertThat(staticLoggingPath).exists();
+        assertThat(staticLoggingPath.getFileName().toString()).matches(PER_CLASS_PATH);
         assertThat(instanceLoggingPath).exists().startsWith(staticLoggingPath).endsWith(PER_TEST_PATH);
         assertThat(parameterLoggingPath).isEqualTo(instanceLoggingPath);
         assertThat(props.getProperty(TestProperties.LOGGING_PATH)).isEqualTo(instanceLoggingPath.toString());