-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Let @TempDir
fail fast if createTempDirectory
does not return a directory
#3901
Changes from all commits
e5815e0
fee5988
00072a7
3589a9c
c123964
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -137,7 +137,7 @@ static class JimfsTempDirFactory implements TempDirFactory { | |
@Override | ||
public Path createTempDirectory(AnnotatedElementContext elementContext, ExtensionContext extensionContext) | ||
throws IOException { | ||
return Files.createTempDirectory(fileSystem.getPath("/"), "junit"); | ||
return Files.createTempDirectory(fileSystem.getPath("/"), "junit-"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -284,14 +284,19 @@ static class CloseablePath implements CloseableResource { | |
private final CleanupMode cleanupMode; | ||
private final ExtensionContext extensionContext; | ||
|
||
CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, AnnotatedElementContext elementContext, | ||
private CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, AnnotatedElementContext elementContext, | ||
scordio marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ExtensionContext extensionContext) throws Exception { | ||
this.dir = factory.createTempDirectory(elementContext, extensionContext); | ||
this.dir = validateTempDirectory(factory.createTempDirectory(elementContext, extensionContext)); | ||
this.factory = factory; | ||
this.cleanupMode = cleanupMode; | ||
this.extensionContext = extensionContext; | ||
} | ||
Comment on lines
+287
to
293
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marcphilipp I just realized a Should we wrap it in a try/catch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I raised #3911 for evaluation. |
||
|
||
private static Path validateTempDirectory(Path dir) { | ||
Preconditions.condition(dir != null && Files.isDirectory(dir), "temp directory must be a directory"); | ||
return dir; | ||
} | ||
|
||
Path get() { | ||
return dir; | ||
} | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,236 @@ | ||
/* | ||
* Copyright 2015-2024 the original author or authors. | ||
* | ||
* All rights reserved. This program and the accompanying materials are | ||
* made available under the terms of the Eclipse Public License v2.0 which | ||
* accompanies this distribution and is available at | ||
* | ||
* https://www.eclipse.org/legal/epl-v20.html | ||
*/ | ||
|
||
package org.junit.jupiter.engine.extension; | ||
|
||
import static java.nio.file.Files.createDirectory; | ||
import static java.nio.file.Files.createFile; | ||
import static java.nio.file.Files.createSymbolicLink; | ||
import static java.nio.file.Files.createTempDirectory; | ||
import static java.nio.file.Files.delete; | ||
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.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.spy; | ||
import static org.mockito.Mockito.verify; | ||
import static org.mockito.Mockito.when; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
|
||
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.extension.AnnotatedElementContext; | ||
import org.junit.jupiter.api.extension.ExtensionConfigurationException; | ||
import org.junit.jupiter.api.extension.ExtensionContext; | ||
import org.junit.jupiter.api.extension.ExtensionContext.Namespace; | ||
import org.junit.jupiter.api.io.CleanupMode; | ||
import org.junit.jupiter.api.io.TempDir; | ||
import org.junit.jupiter.api.io.TempDirFactory; | ||
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests; | ||
import org.junit.jupiter.engine.execution.NamespaceAwareStore; | ||
import org.junit.platform.commons.PreconditionViolationException; | ||
import org.junit.platform.engine.support.store.NamespacedHierarchicalStore; | ||
|
||
/** | ||
* Integration tests for the creation and cleanup of the {@link TempDirectory}. | ||
* | ||
* @since 5.9 | ||
*/ | ||
@DisplayName("Temporary directory") | ||
class CloseablePathTests extends AbstractJupiterTestEngineTests { | ||
|
||
private final AnnotatedElementContext elementContext = mock(); | ||
private final ExtensionContext extensionContext = mock(); | ||
|
||
private TempDirectory.CloseablePath closeablePath; | ||
|
||
@BeforeEach | ||
void setUpExtensionContext() { | ||
var store = new NamespaceAwareStore(new NamespacedHierarchicalStore<>(null), Namespace.GLOBAL); | ||
when(extensionContext.getStore(any())).thenReturn(store); | ||
} | ||
|
||
/** | ||
* Integration tests for the creation of the {@link TempDirectory} based on the different result | ||
* that {@link TempDirFactory#createTempDirectory(AnnotatedElementContext, ExtensionContext)} may provide. | ||
* | ||
* @since 5.11 | ||
* | ||
* @see TempDirFactory | ||
*/ | ||
@Nested | ||
@DisplayName("creation") | ||
class Creation { | ||
|
||
private Path root; | ||
|
||
@BeforeEach | ||
void setUpRootFolder() throws IOException { | ||
root = createTempDirectory("root"); | ||
} | ||
|
||
@AfterEach | ||
void cleanupRoot() throws IOException { | ||
delete(root); | ||
} | ||
|
||
@Test | ||
@DisplayName("succeeds if the factory returns a directory") | ||
void factoryReturnsDirectory() throws Exception { | ||
TempDirFactory factory = (elementContext, extensionContext) -> createDirectory(root.resolve("directory")); | ||
|
||
closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
delete(closeablePath.get()); | ||
} | ||
|
||
@Test | ||
@DisplayName("succeeds if the factory returns a symbolic link to a directory") | ||
void factoryReturnsSymbolicLinkToDirectory() throws Exception { | ||
Path directory = createDirectory(root.resolve("directory")); | ||
TempDirFactory factory = (elementContext, | ||
extensionContext) -> createSymbolicLink(root.resolve("symbolicLink"), directory); | ||
|
||
closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
delete(closeablePath.get()); | ||
delete(directory); | ||
} | ||
|
||
@Test | ||
@DisplayName("fails if the factory returns null") | ||
void factoryReturnsNull() { | ||
TempDirFactory factory = (elementContext, extensionContext) -> null; | ||
|
||
assertThatExtensionConfigurationExceptionIsThrownBy( | ||
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); | ||
} | ||
|
||
@Test | ||
@DisplayName("fails if the factory returns a file") | ||
void factoryReturnsFile() throws IOException { | ||
Path file = createFile(root.resolve("file")); | ||
TempDirFactory factory = (elementContext, extensionContext) -> file; | ||
|
||
assertThatExtensionConfigurationExceptionIsThrownBy( | ||
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); | ||
|
||
delete(file); | ||
} | ||
|
||
@Test | ||
@DisplayName("fails if the factory returns a symbolic link to a file") | ||
void factoryReturnsSymbolicLinkToFile() throws IOException { | ||
Path file = createFile(root.resolve("file")); | ||
Path symbolicLink = createSymbolicLink(root.resolve("symbolicLink"), file); | ||
TempDirFactory factory = (elementContext, extensionContext) -> symbolicLink; | ||
|
||
assertThatExtensionConfigurationExceptionIsThrownBy( | ||
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext)); | ||
|
||
delete(symbolicLink); | ||
delete(file); | ||
} | ||
|
||
private static void assertThatExtensionConfigurationExceptionIsThrownBy(ThrowingCallable callable) { | ||
assertThatExceptionOfType(ExtensionConfigurationException.class)// | ||
.isThrownBy(callable)// | ||
.withMessage("Failed to create default temp directory")// | ||
.withCauseInstanceOf(PreconditionViolationException.class)// | ||
.havingCause().withMessage("temp directory must be a directory"); | ||
} | ||
|
||
} | ||
|
||
/** | ||
* Integration tests for cleanup of the {@link TempDirectory} when the {@link CleanupMode} is | ||
* set to {@link CleanupMode#ALWAYS}, {@link CleanupMode#NEVER}, or {@link CleanupMode#ON_SUCCESS}. | ||
* | ||
* @since 5.9 | ||
* | ||
* @see TempDir | ||
* @see CleanupMode | ||
*/ | ||
@Nested | ||
@DisplayName("cleanup") | ||
class Cleanup { | ||
|
||
private final TempDirFactory factory = spy(TempDirFactory.Standard.INSTANCE); | ||
|
||
@AfterEach | ||
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); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
closeablePath.close(); | ||
assertThat(closeablePath.get()).doesNotExist(); | ||
verify(factory).close(); | ||
} | ||
|
||
@Test | ||
@DisplayName("is not done for a cleanup mode of NEVER") | ||
void never() throws IOException { | ||
closeablePath = TempDirectory.createTempDir(factory, NEVER, elementContext, extensionContext); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
closeablePath.close(); | ||
assertThat(closeablePath.get()).exists(); | ||
verify(factory).close(); | ||
} | ||
|
||
@Test | ||
@DisplayName("is not done for a cleanup mode of ON_SUCCESS, if there is an exception") | ||
void onSuccessWithException() throws IOException { | ||
when(extensionContext.getExecutionException()).thenReturn(Optional.of(new Exception())); | ||
|
||
closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementContext, extensionContext); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
closeablePath.close(); | ||
assertThat(closeablePath.get()).exists(); | ||
verify(factory).close(); | ||
} | ||
|
||
@Test | ||
@DisplayName("is done for a cleanup mode of ON_SUCCESS, if there is no exception") | ||
void onSuccessWithNoException() throws IOException { | ||
when(extensionContext.getExecutionException()).thenReturn(Optional.empty()); | ||
|
||
closeablePath = TempDirectory.createTempDir(factory, ON_SUCCESS, elementContext, extensionContext); | ||
assertThat(closeablePath.get()).isDirectory(); | ||
|
||
closeablePath.close(); | ||
assertThat(closeablePath.get()).doesNotExist(); | ||
verify(factory).close(); | ||
} | ||
|
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing my typo! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thanks for catching the release notes gap!