diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-RC1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-RC1.adoc index 807cde03b72f..3c1c8115765c 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-RC1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-RC1.adoc @@ -36,7 +36,8 @@ repository on GitHub. [[release-notes-5.12.0-RC1-junit-jupiter-bug-fixes]] ==== Bug Fixes -* ❓ +* Fix handling of "junctions" on Windows during `@TempDir` cleanup: junctions will no + longer be followed when deleting directories and broken junctions will be deleted. [[release-notes-5.12.0-RC1-junit-jupiter-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java index eba68262c8e5..dbcb0a1c80c9 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java @@ -11,6 +11,7 @@ package org.junit.jupiter.engine.extension; import static java.nio.file.FileVisitResult.CONTINUE; +import static java.nio.file.FileVisitResult.SKIP_SUBTREE; import static java.util.stream.Collectors.joining; import static org.junit.jupiter.api.extension.TestInstantiationAwareExtension.ExtensionContextScope.TEST_METHOD; import static org.junit.jupiter.api.io.CleanupMode.DEFAULT; @@ -32,6 +33,7 @@ import java.nio.file.FileSystems; import java.nio.file.FileVisitResult; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; @@ -289,7 +291,7 @@ private static boolean selfOrChildFailed(ExtensionContext context) { static class CloseablePath implements CloseableResource { - private static final Logger logger = LoggerFactory.getLogger(CloseablePath.class); + private static final Logger LOGGER = LoggerFactory.getLogger(CloseablePath.class); private final Path dir; private final TempDirFactory factory; @@ -327,15 +329,27 @@ public void close() throws IOException { try { if (this.cleanupMode == NEVER || (this.cleanupMode == ON_SUCCESS && selfOrChildFailed(this.extensionContext))) { - logger.info(() -> String.format("Skipping cleanup of temp dir %s for %s due to CleanupMode.%s.", + LOGGER.info(() -> String.format("Skipping cleanup of temp dir %s for %s due to CleanupMode.%s.", this.dir, descriptionFor(this.annotatedElement), this.cleanupMode.name())); return; } FileOperations fileOperations = this.extensionContext.getStore(NAMESPACE) // .getOrDefault(FILE_OPERATIONS_KEY, FileOperations.class, FileOperations.DEFAULT); + FileOperations loggingFileOperations = file -> { + LOGGER.trace(() -> "Attempting to delete " + file); + try { + fileOperations.delete(file); + LOGGER.trace(() -> "Successfully deleted " + file); + } + catch (IOException e) { + LOGGER.trace(e, () -> "Failed to delete " + file); + throw e; + } + }; - SortedMap failures = deleteAllFilesAndDirectories(fileOperations); + LOGGER.trace(() -> "Cleaning up temp dir " + this.dir); + SortedMap failures = deleteAllFilesAndDirectories(loggingFileOperations); if (!failures.isEmpty()) { throw createIOExceptionWithAttachedFailures(failures); } @@ -375,26 +389,41 @@ private static String descriptionFor(Executable executable) { private SortedMap deleteAllFilesAndDirectories(FileOperations fileOperations) throws IOException { - if (this.dir == null || Files.notExists(this.dir)) { + Path rootDir = this.dir; + if (rootDir == null || Files.notExists(rootDir)) { return Collections.emptySortedMap(); } SortedMap failures = new TreeMap<>(); Set retriedPaths = new HashSet<>(); - tryToResetPermissions(this.dir); - Files.walkFileTree(this.dir, new SimpleFileVisitor() { + Path rootRealPath = rootDir.toRealPath(); + + tryToResetPermissions(rootDir); + Files.walkFileTree(rootDir, new SimpleFileVisitor() { @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { - if (!dir.equals(CloseablePath.this.dir)) { + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + LOGGER.trace(() -> "preVisitDirectory: " + dir); + if (isLink(dir)) { + delete(dir); + return SKIP_SUBTREE; + } + if (!dir.equals(rootDir)) { tryToResetPermissions(dir); } return CONTINUE; } + private boolean isLink(Path dir) throws IOException { + // While `Files.walkFileTree` does not follow symbolic links, it may follow other links + // such as "junctions" on Windows + return !dir.toRealPath().startsWith(rootRealPath); + } + @Override public FileVisitResult visitFileFailed(Path file, IOException exc) { - if (exc instanceof NoSuchFileException) { + LOGGER.trace(exc, () -> "visitFileFailed: " + file); + if (exc instanceof NoSuchFileException && !Files.exists(file, LinkOption.NOFOLLOW_LINKS)) { return CONTINUE; } // IOException includes `AccessDeniedException` thrown by non-readable or non-executable flags @@ -404,15 +433,19 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attributes) { - return deleteAndContinue(file); + LOGGER.trace(() -> "visitFile: " + file); + delete(file); + return CONTINUE; } @Override public FileVisitResult postVisitDirectory(Path dir, IOException exc) { - return deleteAndContinue(dir); + LOGGER.trace(exc, () -> "postVisitDirectory: " + dir); + delete(dir); + return CONTINUE; } - private FileVisitResult deleteAndContinue(Path path) { + private void delete(Path path) { try { fileOperations.delete(path); } @@ -426,7 +459,6 @@ private FileVisitResult deleteAndContinue(Path path) { // IOException includes `AccessDeniedException` thrown by non-readable or non-executable flags resetPermissionsAndTryToDeleteAgain(path, exception); } - return CONTINUE; } private void resetPermissionsAndTryToDeleteAgain(Path path, IOException exception) { diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java index 6a7d687fc1a3..4ace7a57d7b2 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java @@ -13,6 +13,7 @@ import static java.nio.file.Files.deleteIfExists; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.condition.OS.WINDOWS; import static org.junit.jupiter.api.io.CleanupMode.ALWAYS; import static org.junit.jupiter.api.io.CleanupMode.NEVER; import static org.junit.jupiter.api.io.CleanupMode.ON_SUCCESS; @@ -21,6 +22,7 @@ import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import org.junit.jupiter.api.AfterAll; @@ -29,6 +31,7 @@ import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.io.CleanupMode; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.engine.AbstractJupiterTestEngineTests; @@ -452,4 +455,57 @@ void testOnSuccessFailingParameter(@TempDir(cleanup = ON_SUCCESS) Path onSuccess } + @Nested + @EnabledOnOs(WINDOWS) + class WindowsTests { + + @Test + void deletesBrokenJunctions(@TempDir Path dir) throws Exception { + var test = Files.createDirectory(dir.resolve("test")); + createWindowsJunction(dir.resolve("link"), test); + // The error might also occur without the source folder being deleted + // but it depends on the order that the TempDir cleanup does its work, + // so the following line forces the error to occur always + Files.delete(test); + } + + @Test + void doesNotFollowJunctions(@TempDir Path tempDir) throws IOException { + var outsideDir = Files.createDirectory(tempDir.resolve("outside")); + var testFile = Files.writeString(outsideDir.resolve("test.txt"), "test"); + + JunctionTestCase.target = outsideDir; + try { + executeTestsForClass(JunctionTestCase.class).testEvents() // + .assertStatistics(stats -> stats.started(1).succeeded(1)); + } + finally { + JunctionTestCase.target = null; + } + + assertThat(outsideDir).exists(); + assertThat(testFile).exists(); + } + + @SuppressWarnings("JUnitMalformedDeclaration") + static class JunctionTestCase { + public static Path target; + + @Test + void createJunctionToTarget(@TempDir Path dir) throws Exception { + var link = createWindowsJunction(dir.resolve("link"), target); + try (var files = Files.list(link)) { + files.forEach(it -> System.out.println("- " + it)); + } + } + } + + private static Path createWindowsJunction(Path link, Path target) throws Exception { + // This creates a Windows "junction" which you can't do with Java code + String[] command = { "cmd.exe", "/c", "mklink", "/j", link.toString(), target.toString() }; + Runtime.getRuntime().exec(command).waitFor(); + return link; + } + } + } diff --git a/jupiter-tests/src/test/resources/junit-platform.properties b/jupiter-tests/src/test/resources/junit-platform.properties index 6efc0d5e85ce..0f5b21dec75e 100644 --- a/jupiter-tests/src/test/resources/junit-platform.properties +++ b/jupiter-tests/src/test/resources/junit-platform.properties @@ -1 +1,4 @@ junit.jupiter.extensions.autodetection.enabled=true + +junit.platform.output.capture.stdout=true +junit.platform.output.capture.stderr=true diff --git a/jupiter-tests/src/test/resources/log4j2-test.xml b/jupiter-tests/src/test/resources/log4j2-test.xml index d734a7fa7da2..509679b9abb2 100644 --- a/jupiter-tests/src/test/resources/log4j2-test.xml +++ b/jupiter-tests/src/test/resources/log4j2-test.xml @@ -11,8 +11,9 @@ + - \ No newline at end of file +