Skip to content

Let @TempDir fail fast with File annotated element and non-default file system temp directory #3921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcphilipp merged 4 commits into from
Aug 12, 2024
Merged

Let @TempDir fail fast with File annotated element and non-default file system temp directory #3921

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1591388
Let `@TempDir` fail fast with `File` annotated element and non-defaul…
scordio Aug 10, 2024
38744ab
Remove unnecessary spy
scordio Aug 12, 2024
37df841
Update `TempDir` Javadoc
scordio Aug 12, 2024
99f8f1e
Polisj Javadoc
marcphilipp Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
27 changes: 18 additions & 9 deletions junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -145,7 +146,7 @@ private void injectFields(ExtensionContext context, Object testInstance, Class<?
CleanupMode cleanupMode = determineCleanupModeForField(field);
TempDirFactory factory = determineTempDirFactoryForField(field, scope);
makeAccessible(field).set(testInstance,
getPathOrFile(new FieldContext(field), field.getType(), factory, cleanupMode, scope, context));
getPathOrFile(field.getType(), new FieldContext(field), factory, cleanupMode, scope, context));
}
catch (Throwable t) {
throw ExceptionUtils.throwAsUncheckedException(t);
Expand Down Expand Up @@ -178,7 +179,7 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte
CleanupMode cleanupMode = determineCleanupModeForParameter(parameterContext);
Scope scope = getScope(extensionContext);
TempDirFactory factory = determineTempDirFactoryForParameter(parameterContext, scope);
return getPathOrFile(parameterContext, parameterType, factory, cleanupMode, scope, extensionContext);
return getPathOrFile(parameterType, parameterContext, factory, cleanupMode, scope, extensionContext);
}

private CleanupMode determineCleanupModeForField(Field field) {
Expand Down Expand Up @@ -248,23 +249,24 @@ private void assertSupportedType(String target, 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);
Expand All @@ -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;
Expand All @@ -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() {
Expand Down
145 changes: 114 additions & 31 deletions jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/CloseablePathTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,28 +21,35 @@
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;
import org.junit.jupiter.api.DisplayName;
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;
Expand All @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -95,72 +113,107 @@ 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 = spy(new JimfsFactory());
Comment thread
scordio marked this conversation as resolved.
Outdated

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();

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 {

Expand All @@ -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)//
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
Loading