diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc index 56c448b69304..08edb8126cd6 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc @@ -183,6 +183,9 @@ on GitHub. tests, in particular in recent versions of Java that support records. * `@TempDir` now fails fast in case `TempDirFactory::createTempDirectory` returns `null`, a file, or a symbolic link to a file. +* `@TempDir` now fails fast in case the annotated target is of type `File` and + `TempDirFactory::createTempDirectory` returns a `Path` that does not belong to the + default file system. [[release-notes-5.11.0-junit-vintage]] diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/io/TempDir.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/io/TempDir.java index de5fd182d6a8..55d9bbfcf56b 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/io/TempDir.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/io/TempDir.java @@ -40,11 +40,24 @@ * *

The temporary directory is only created if a field in a test class or a * parameter in a lifecycle method or test method is annotated with - * {@code @TempDir}. If the field type or parameter type is neither {@link Path} - * nor {@link File}, if a field is declared as {@code final}, or if the temporary - * directory cannot be created, an {@link ExtensionConfigurationException} or a - * {@link ParameterResolutionException} will be thrown as appropriate. In - * addition, a {@code ParameterResolutionException} will be thrown for a + * {@code @TempDir}. + * An {@link ExtensionConfigurationException} or a + * {@link ParameterResolutionException} will be thrown in one of the following + * cases: + * + *

+ * + * In addition, a {@code ParameterResolutionException} will be thrown for a * constructor parameter annotated with {@code @TempDir}. * *

Scope

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 a4bb8662cb4c..ac5831d88329 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 @@ -26,6 +26,7 @@ import java.lang.reflect.Field; import java.lang.reflect.Parameter; import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileSystems; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.NoSuchFileException; @@ -145,7 +146,7 @@ private void injectFields(ExtensionContext context, Object testInstance, Class type) { } } - private Object getPathOrFile(AnnotatedElementContext elementContext, Class type, TempDirFactory factory, + private Object getPathOrFile(Class elementType, AnnotatedElementContext elementContext, TempDirFactory factory, CleanupMode cleanupMode, Scope scope, ExtensionContext extensionContext) { Namespace namespace = scope == Scope.PER_DECLARATION // ? NAMESPACE.append(elementContext) // : NAMESPACE; Path path = extensionContext.getStore(namespace) // - .getOrComputeIfAbsent(KEY, __ -> createTempDir(factory, cleanupMode, elementContext, extensionContext), + .getOrComputeIfAbsent(KEY, + __ -> createTempDir(factory, cleanupMode, elementType, elementContext, extensionContext), CloseablePath.class) // .get(); - return (type == Path.class) ? path : path.toFile(); + return (elementType == Path.class) ? path : path.toFile(); } - static CloseablePath createTempDir(TempDirFactory factory, CleanupMode cleanupMode, + static CloseablePath createTempDir(TempDirFactory factory, CleanupMode cleanupMode, Class elementType, AnnotatedElementContext elementContext, ExtensionContext extensionContext) { try { - return new CloseablePath(factory, cleanupMode, elementContext, extensionContext); + return new CloseablePath(factory, cleanupMode, elementType, elementContext, extensionContext); } catch (Exception ex) { throw new ExtensionConfigurationException("Failed to create default temp directory", ex); @@ -285,8 +287,8 @@ static class CloseablePath implements CloseableResource { private final CleanupMode cleanupMode; private final ExtensionContext extensionContext; - private CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, AnnotatedElementContext elementContext, - ExtensionContext extensionContext) throws Exception { + private CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, Class elementType, + AnnotatedElementContext elementContext, ExtensionContext extensionContext) throws Exception { this.dir = factory.createTempDirectory(elementContext, extensionContext); this.factory = factory; this.cleanupMode = cleanupMode; @@ -296,6 +298,13 @@ private CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, Annotated close(); throw new PreconditionViolationException("temp directory must be a directory"); } + + if (elementType == File.class && !dir.getFileSystem().equals(FileSystems.getDefault())) { + close(); + throw new PreconditionViolationException( + "temp directory with non-default file system cannot be injected into " + File.class.getName() + + " target"); + } } Path get() { diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/CloseablePathTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/CloseablePathTests.java index 3f76a49c77b0..6a0645975813 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/CloseablePathTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/CloseablePathTests.java @@ -10,6 +10,9 @@ package org.junit.jupiter.engine.extension; +import static com.google.common.jimfs.Configuration.unix; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; import static java.nio.file.Files.createDirectory; import static java.nio.file.Files.createFile; import static java.nio.file.Files.createSymbolicLink; @@ -18,20 +21,28 @@ import static java.nio.file.Files.deleteIfExists; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +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.DEFAULT; import static org.junit.jupiter.api.io.CleanupMode.NEVER; import static org.junit.jupiter.api.io.CleanupMode.ON_SUCCESS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.File; import java.io.IOException; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.nio.file.FileSystem; import java.nio.file.Path; import java.util.Optional; +import com.google.common.jimfs.Jimfs; + import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -39,7 +50,6 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; -import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.extension.AnnotatedElementContext; import org.junit.jupiter.api.extension.ExtensionConfigurationException; import org.junit.jupiter.api.extension.ExtensionContext; @@ -49,6 +59,8 @@ import org.junit.jupiter.api.io.TempDirFactory; import org.junit.jupiter.engine.AbstractJupiterTestEngineTests; import org.junit.jupiter.engine.execution.NamespaceAwareStore; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.PreconditionViolationException; import org.junit.platform.engine.support.store.NamespacedHierarchicalStore; @@ -65,6 +77,12 @@ class CloseablePathTests extends AbstractJupiterTestEngineTests { private TempDirectory.CloseablePath closeablePath; + @Target(METHOD) + @Retention(RUNTIME) + @ValueSource(classes = { File.class, Path.class }) + private @interface ElementTypeSource { + } + @BeforeEach void setUpExtensionContext() { var store = new NamespaceAwareStore(new NamespacedHierarchicalStore<>(null), Namespace.GLOBAL); @@ -95,65 +113,84 @@ void cleanupRoot() throws IOException { delete(root); } - @Test @DisplayName("succeeds if the factory returns a directory") - void factoryReturnsDirectory() throws Exception { - TempDirFactory factory = spy(new Factory(createDirectory(root.resolve("directory")))); + @ParameterizedTest + @ElementTypeSource + void factoryReturnsDirectoryDynamic(Class elementType) throws IOException { + TempDirFactory factory = (elementContext, extensionContext) -> createDirectory(root.resolve("directory")); - closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext); + closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementType, elementContext, + extensionContext); assertThat(closeablePath.get()).isDirectory(); delete(closeablePath.get()); } - @Test @DisplayName("succeeds if the factory returns a symbolic link to a directory") - @DisabledOnOs(OS.WINDOWS) - void factoryReturnsSymbolicLinkToDirectory() throws Exception { + @ParameterizedTest + @ElementTypeSource + @DisabledOnOs(WINDOWS) + void factoryReturnsSymbolicLinkToDirectory(Class elementType) throws IOException { Path directory = createDirectory(root.resolve("directory")); - TempDirFactory factory = spy(new Factory(createSymbolicLink(root.resolve("symbolicLink"), directory))); + TempDirFactory factory = (elementContext, + extensionContext) -> createSymbolicLink(root.resolve("symbolicLink"), directory); - closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext); + closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementType, elementContext, + extensionContext); assertThat(closeablePath.get()).isDirectory(); delete(closeablePath.get()); delete(directory); } + @DisplayName("succeeds if the factory returns a directory on a non-default file system for a Path annotated element") @Test + void factoryReturnsDirectoryOnNonDefaultFileSystemWithPath() throws IOException { + TempDirFactory factory = new JimfsFactory(); + + closeablePath = TempDirectory.createTempDir(factory, DEFAULT, Path.class, elementContext, extensionContext); + assertThat(closeablePath.get()).isDirectory(); + + delete(closeablePath.get()); + } + @DisplayName("fails if the factory returns null") - void factoryReturnsNull() throws IOException { + @ParameterizedTest + @ElementTypeSource + void factoryReturnsNull(Class elementType) throws IOException { TempDirFactory factory = spy(new Factory(null)); assertThatExtensionConfigurationExceptionIsThrownBy( - () -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); + () -> TempDirectory.createTempDir(factory, DEFAULT, elementType, elementContext, extensionContext)); verify(factory).close(); } - @Test @DisplayName("fails if the factory returns a file") - void factoryReturnsFile() throws IOException { + @ParameterizedTest + @ElementTypeSource + void factoryReturnsFile(Class elementType) throws IOException { Path file = createFile(root.resolve("file")); TempDirFactory factory = spy(new Factory(file)); assertThatExtensionConfigurationExceptionIsThrownBy( - () -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); + () -> TempDirectory.createTempDir(factory, DEFAULT, elementType, elementContext, extensionContext)); verify(factory).close(); assertThat(file).doesNotExist(); } - @Test @DisplayName("fails if the factory returns a symbolic link to a file") - @DisabledOnOs(OS.WINDOWS) - void factoryReturnsSymbolicLinkToFile() throws IOException { + @ParameterizedTest + @ElementTypeSource + @DisabledOnOs(WINDOWS) + void factoryReturnsSymbolicLinkToFile(Class elementType) throws IOException { Path file = createFile(root.resolve("file")); Path symbolicLink = createSymbolicLink(root.resolve("symbolicLink"), file); TempDirFactory factory = spy(new Factory(symbolicLink)); assertThatExtensionConfigurationExceptionIsThrownBy( - () -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); + () -> TempDirectory.createTempDir(factory, DEFAULT, elementType, elementContext, extensionContext)); verify(factory).close(); assertThat(symbolicLink).doesNotExist(); @@ -161,6 +198,22 @@ void factoryReturnsSymbolicLinkToFile() throws IOException { delete(file); } + @DisplayName("fails if the factory returns a directory on a non-default file system for a File annotated element") + @Test + void factoryReturnsDirectoryOnNonDefaultFileSystemWithFile() throws IOException { + TempDirFactory factory = spy(new JimfsFactory()); + + assertThatExceptionOfType(ExtensionConfigurationException.class)// + .isThrownBy(() -> TempDirectory.createTempDir(factory, DEFAULT, File.class, elementContext, + extensionContext))// + .withMessage("Failed to create default temp directory")// + .withCauseInstanceOf(PreconditionViolationException.class)// + .havingCause().withMessage("temp directory with non-default file system cannot be injected into " + + File.class.getName() + " target"); + + verify(factory).close(); + } + // Mockito spying a lambda fails with: VM does not support modification of given type private record Factory(Path path) implements TempDirFactory { @@ -171,6 +224,22 @@ public Path createTempDirectory(AnnotatedElementContext elementContext, Extensio } + private static class JimfsFactory implements TempDirFactory { + + private final FileSystem fileSystem = Jimfs.newFileSystem(unix()); + + @Override + public Path createTempDirectory(AnnotatedElementContext elementContext, ExtensionContext extensionContext) + throws Exception { + return createDirectory(fileSystem.getPath("/").resolve("directory")); + } + + @Override + public void close() throws IOException { + TempDirFactory.super.close(); + } + } + private static void assertThatExtensionConfigurationExceptionIsThrownBy(ThrowingCallable callable) { assertThatExceptionOfType(ExtensionConfigurationException.class)// .isThrownBy(callable)// @@ -201,10 +270,13 @@ void cleanupTempDirectory() throws IOException { deleteIfExists(closeablePath.get()); } - @Test @DisplayName("is done for a cleanup mode of ALWAYS") - void always() throws IOException { - closeablePath = TempDirectory.createTempDir(factory, ALWAYS, elementContext, extensionContext); + @ParameterizedTest + @ElementTypeSource + void always(Class elementType) throws IOException { + reset(factory); + + closeablePath = TempDirectory.createTempDir(factory, ALWAYS, elementType, elementContext, extensionContext); assertThat(closeablePath.get()).isDirectory(); closeablePath.close(); @@ -213,10 +285,13 @@ void always() throws IOException { assertThat(closeablePath.get()).doesNotExist(); } - @Test @DisplayName("is not done for a cleanup mode of NEVER") - void never() throws IOException { - closeablePath = TempDirectory.createTempDir(factory, NEVER, elementContext, extensionContext); + @ParameterizedTest + @ElementTypeSource + void never(Class elementType) throws IOException { + reset(factory); + + closeablePath = TempDirectory.createTempDir(factory, NEVER, elementType, elementContext, extensionContext); assertThat(closeablePath.get()).isDirectory(); closeablePath.close(); @@ -225,12 +300,16 @@ void never() throws IOException { assertThat(closeablePath.get()).exists(); } - @Test @DisplayName("is not done for a cleanup mode of ON_SUCCESS, if there is an exception") - void onSuccessWithException() throws IOException { + @ParameterizedTest + @ElementTypeSource + void onSuccessWithException(Class elementType) throws IOException { + reset(factory); + when(extensionContext.getExecutionException()).thenReturn(Optional.of(new Exception())); - closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementContext, extensionContext); + closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementType, elementContext, + extensionContext); assertThat(closeablePath.get()).isDirectory(); closeablePath.close(); @@ -239,12 +318,16 @@ void onSuccessWithException() throws IOException { assertThat(closeablePath.get()).exists(); } - @Test @DisplayName("is done for a cleanup mode of ON_SUCCESS, if there is no exception") - void onSuccessWithNoException() throws IOException { + @ParameterizedTest + @ElementTypeSource + void onSuccessWithNoException(Class elementType) throws IOException { + reset(factory); + when(extensionContext.getExecutionException()).thenReturn(Optional.empty()); - closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementContext, extensionContext); + closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementType, elementContext, + extensionContext); assertThat(closeablePath.get()).isDirectory(); closeablePath.close(); diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java index 8c9e455e0041..cfd8fbc452af 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java @@ -321,7 +321,7 @@ void doesNotSupportTempDirFactoryNotReturningDirectory() { @Test @DisplayName("when default @TempDir factory does not return directory") @Order(33) - void doesNotSupportCustomDefaultTempDirFactoryReturningNull() { + void doesNotSupportCustomDefaultTempDirFactoryNotReturningDirectory() { var results = executeTestsForClassWithDefaultFactory( CustomDefaultFactoryNotReturningDirectoryTestCase.class, FactoryNotReturningDirectory.class); @@ -347,6 +347,28 @@ public Path createTempDirectory(AnnotatedElementContext elementContext, Extensio } } + @Test + @DisplayName("when @TempDir factory returns a non-default file system path for a File annotated element") + @Order(34) + void doesNotSupportNonDefaultFileSystemTempDirFactoryOnFileAnnotatedElement() { + var results = executeTestsForClass( + FactoryReturningNonDefaultFileSystemPathForFileAnnotatedElementTestCase.class); + + // @formatter:off + assertSingleFailedTest(results, instanceOf(ParameterResolutionException.class), + message(m -> m.matches("Failed to resolve parameter \\[.+] in method \\[.+]: .+")), + cause( + instanceOf(ExtensionConfigurationException.class), + message("Failed to create default temp directory"), + cause( + instanceOf(PreconditionViolationException.class), + message("temp directory with non-default file system cannot be injected into " + + File.class.getName() + " target") + ) + )); + // @formatter:on + } + } @Nested @@ -1424,6 +1446,31 @@ public Path createTempDirectory(AnnotatedElementContext elementContext, Extensio } + static class FactoryReturningNonDefaultFileSystemPathForFileAnnotatedElementTestCase { + + @Test + void test(@SuppressWarnings("unused") @TempDir(factory = Factory.class) File tempDir) { + // never called + } + + private static class Factory implements TempDirFactory { + + private final FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix()); + + @Override + public Path createTempDirectory(AnnotatedElementContext elementContext, ExtensionContext extensionContext) + throws Exception { + return Files.createTempDirectory(fileSystem.getPath("/"), "prefix"); + } + + @Override + public void close() throws IOException { + fileSystem.close(); + } + } + + } + static class StandardDefaultFactoryTestCase { @Test