Skip to content
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

Jump to bottom

Ensure TempDir cleanup is executed when validation fails #3911

Merged
merged 3 commits into from
Aug 7, 2024
Merged

Ensure TempDir cleanup is executed when validation fails #3911

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7eac468
Ensure closure of `TempDir` factory if temp directory validation fails
scordio Aug 1, 2024
14be79f
Ensure deletion of non-null temp directory if validation fails
scordio Aug 7, 2024
436eda4
Apply suggestion
scordio Aug 7, 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
15 changes: 11 additions & 4 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 @@ -57,6 +57,7 @@
import org.junit.jupiter.engine.config.EnumConfigurationParameterConverter;
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.logging.Logger;
import org.junit.platform.commons.logging.LoggerFactory;
import org.junit.platform.commons.util.ExceptionUtils;
Expand Down Expand Up @@ -286,10 +287,16 @@ static class CloseablePath implements CloseableResource {

private CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, AnnotatedElementContext elementContext,
ExtensionContext extensionContext) throws Exception {
this.dir = validateTempDirectory(factory.createTempDirectory(elementContext, extensionContext));
this.factory = factory;
this.cleanupMode = cleanupMode;
this.extensionContext = extensionContext;
try {
this.dir = validateTempDirectory(factory.createTempDirectory(elementContext, extensionContext));
this.factory = factory;
this.cleanupMode = cleanupMode;
this.extensionContext = extensionContext;
}
catch (PreconditionViolationException e) {
factory.close();
throw e;
scordio marked this conversation as resolved.
Show resolved Hide resolved
}
}

private static Path validateTempDirectory(Path dir) {
Expand Down
26 changes: 19 additions & 7 deletions ...t-jupiter-engine/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 @@ -96,7 +96,7 @@ void cleanupRoot() throws IOException {
@Test
@DisplayName("succeeds if the factory returns a directory")
void factoryReturnsDirectory() throws Exception {
TempDirFactory factory = (elementContext, extensionContext) -> createDirectory(root.resolve("directory"));
TempDirFactory factory = spy(new Factory(createDirectory(root.resolve("directory"))));

closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext);
assertThat(closeablePath.get()).isDirectory();
Expand All @@ -108,8 +108,7 @@ void factoryReturnsDirectory() throws Exception {
@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);
TempDirFactory factory = spy(new Factory(createSymbolicLink(root.resolve("symbolicLink"), directory)));

closeablePath = TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext);
assertThat(closeablePath.get()).isDirectory();
Expand All @@ -120,21 +119,23 @@ void factoryReturnsSymbolicLinkToDirectory() throws Exception {

@Test
@DisplayName("fails if the factory returns null")
void factoryReturnsNull() {
TempDirFactory factory = (elementContext, extensionContext) -> null;
void factoryReturnsNull() throws IOException {
TempDirFactory factory = spy(new Factory(null));

assertThatExtensionConfigurationExceptionIsThrownBy(
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext));
verify(factory).close();
}

@Test
@DisplayName("fails if the factory returns a file")
void factoryReturnsFile() throws IOException {
Path file = createFile(root.resolve("file"));
TempDirFactory factory = (elementContext, extensionContext) -> file;
TempDirFactory factory = spy(new Factory(file));

assertThatExtensionConfigurationExceptionIsThrownBy(
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext));
verify(factory).close();

delete(file);
}
Expand All @@ -144,15 +145,26 @@ void factoryReturnsFile() throws IOException {
void factoryReturnsSymbolicLinkToFile() throws IOException {
Path file = createFile(root.resolve("file"));
Path symbolicLink = createSymbolicLink(root.resolve("symbolicLink"), file);
TempDirFactory factory = (elementContext, extensionContext) -> symbolicLink;
TempDirFactory factory = spy(new Factory(symbolicLink));

assertThatExtensionConfigurationExceptionIsThrownBy(
() -> TempDirectory.createTempDir(factory, DEFAULT, elementContext, extensionContext));
verify(factory).close();

delete(symbolicLink);
delete(file);
}

// Mockito spying a lambda fails with: VM does not support modification of given type
private record Factory(Path path) implements TempDirFactory {

@Override
public Path createTempDirectory(AnnotatedElementContext elementContext, ExtensionContext extensionContext) {
return path;
}

}

private static void assertThatExtensionConfigurationExceptionIsThrownBy(ThrowingCallable callable) {
assertThatExceptionOfType(ExtensionConfigurationException.class)//
.isThrownBy(callable)//
Expand Down
Loading