Fix bug with @tempdir custom default cleanup mode

Prior to this commit, a custom default cleanup mode was ignored when determining the cleanup mode for a parameter annotated with @tempdir. See #2729
junit-team · Dec 31, 2021 · 498cf7f · 498cf7f
1 parent 01353cd
commit 498cf7f
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 25 deletions.
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
@@ -12,7 +12,6 @@
 
 import static java.nio.file.FileVisitResult.CONTINUE;
 import static java.util.stream.Collectors.joining;
-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;
@@ -35,7 +34,6 @@
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Collections;
-import java.util.Optional;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.function.Predicate;
@@ -117,8 +115,8 @@ private void injectFields(ExtensionContext context, Object testInstance, Class<?
 			assertSupportedType("field", field.getType());
 
 			try {
-				CleanupMode mode = findCleanupModeForField(field);
-				makeAccessible(field).set(testInstance, getPathOrFile(field, field.getType(), mode, context));
+				CleanupMode cleanupMode = determineCleanupModeForField(field);
+				makeAccessible(field).set(testInstance, getPathOrFile(field, field.getType(), cleanupMode, context));
 			}
 			catch (Throwable t) {
 				ExceptionUtils.throwAsUncheckedException(t);
@@ -148,35 +146,29 @@ public boolean supportsParameter(ParameterContext parameterContext, ExtensionCon
 	public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
 		Class<?> parameterType = parameterContext.getParameter().getType();
 		assertSupportedType("parameter", parameterType);
-		CleanupMode cleanupMode = findCleanupModeForParameter(parameterContext);
+		CleanupMode cleanupMode = determineCleanupModeForParameter(parameterContext);
 		return getPathOrFile(parameterContext.getParameter(), parameterType, cleanupMode, extensionContext);
 	}
 
-	private CleanupMode findCleanupModeForParameter(ParameterContext parameterContext) {
-		CleanupMode cleanupMode = null;
-		Optional<TempDir> optional = parameterContext.findAnnotation(TempDir.class);
-		if (optional.isPresent()) {
-			cleanupMode = optional.get().cleanup();
-		}
-		if (cleanupMode == null || cleanupMode == DEFAULT) {
-			cleanupMode = ALWAYS;
-		}
-		return cleanupMode;
+	private CleanupMode determineCleanupModeForField(Field field) {
+		TempDir tempDir = findAnnotation(field, TempDir.class).orElseThrow(
+			() -> new IllegalStateException("Field " + field + " must be annotated with @TempDir"));
+		return determineCleanupMode(tempDir);
 	}
 
-	private CleanupMode findCleanupModeForField(Field field) {
-		CleanupMode cleanupMode = null;
-		Optional<TempDir> optional = findAnnotation(field, TempDir.class);
-		if (optional.isPresent()) {
-			cleanupMode = optional.get().cleanup();
-		}
-		if (cleanupMode == null || cleanupMode == DEFAULT) {
-			cleanupMode = this.configuration.getDefaultTempDirCleanupMode();
-		}
-		return cleanupMode;
+	private CleanupMode determineCleanupModeForParameter(ParameterContext parameterContext) {
+		TempDir tempDir = parameterContext.findAnnotation(TempDir.class).orElseThrow(() -> new IllegalStateException(
+			"Parameter " + parameterContext.getParameter() + " must be annotated with @TempDir"));
+		return determineCleanupMode(tempDir);
+	}
+
+	private CleanupMode determineCleanupMode(TempDir tempDir) {
+		CleanupMode cleanupMode = tempDir.cleanup();
+		return cleanupMode == DEFAULT ? this.configuration.getDefaultTempDirCleanupMode() : cleanupMode;
 	}
 
 	private void assertSupportedType(String target, Class<?> type) {
+
 		if (type != Path.class && type != File.class) {
 			throw new ExtensionConfigurationException("Can only resolve @TempDir " + target + " of type "
 					+ Path.class.getName() + " or " + File.class.getName() + " but was: " + type.getName());

diff --git a/...er-engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java b/...er-engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java
@@ -65,6 +65,22 @@ void cleanupModeDefaultField() {
 			assertThat(defaultFieldDir).doesNotExist();
 		}
 
+		/**
+		 * Ensure that a custom, global cleanup mode is used for fields.
+		 * <p/>
+		 * Expect the TempDir NOT to be cleaned up if set to NEVER.
+		 */
+		@Test
+		void cleanupModeCustomDefaultField() {
+			LauncherDiscoveryRequest request = request()//
+					.configurationParameter(TempDir.DEFAULT_CLEANUP_MODE_PROPERTY_NAME, "never")//
+					.selectors(selectMethod(DefaultFieldCase.class, "testDefaultField"))//
+					.build();
+			executeTests(request);
+
+			assertThat(defaultFieldDir).exists();
+		}
+
 		/**
 		 * Ensure that NEVER cleanup modes are obeyed for fields.
 		 * <p/>
@@ -218,6 +234,22 @@ void cleanupModeDefaultParameter() {
 			assertThat(defaultParameterDir).doesNotExist();
 		}
 
+		/**
+		 * Ensure that a custom, global cleanup mode is used for parameters.
+		 * <p/>
+		 * Expect the TempDir NOT to be cleaned up if set to NEVER.
+		 */
+		@Test
+		void cleanupModeCustomDefaultParameter() {
+			LauncherDiscoveryRequest request = request()//
+					.configurationParameter(TempDir.DEFAULT_CLEANUP_MODE_PROPERTY_NAME, "never")//
+					.selectors(selectMethod(DefaultParameterCase.class, "testDefaultParameter", "java.nio.file.Path"))//
+					.build();
+			executeTests(request);
+
+			assertThat(defaultParameterDir).exists();
+		}
+
 		/**
 		 * Ensure that NEVER cleanup modes are obeyed for parameters.
 		 * <p/>