Fail fast when TempDirFactory::createTempDirectory returns null

junit-team · Jul 26, 2024 · b7e8586 · b7e8586
1 parent adf6d43
commit b7e8586
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 22 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
@@ -286,7 +286,8 @@ static class CloseablePath implements CloseableResource {
 
 		CloseablePath(TempDirFactory factory, CleanupMode cleanupMode, AnnotatedElementContext elementContext,
 				ExtensionContext extensionContext) throws Exception {
-			this.dir = factory.createTempDirectory(elementContext, extensionContext);
+			this.dir = Preconditions.notNull(factory.createTempDirectory(elementContext, extensionContext),
+				"temp directory must not be null");
 			this.factory = factory;
 			this.cleanupMode = cleanupMode;
 			this.extensionContext = extensionContext;

diff --git a/...engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerContextTests.java b/...engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerContextTests.java
@@ -54,10 +54,11 @@
 import org.junit.jupiter.api.extension.ParameterResolutionException;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.api.io.TempDirFactory;
+import org.junit.jupiter.api.io.TempDirFactory.Standard;
 import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
+import org.junit.platform.commons.PreconditionViolationException;
 import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
 import org.junit.platform.testkit.engine.EngineExecutionResults;
-import org.junit.platform.testkit.engine.Events;
 
 /**
  * Integration tests for the legacy behavior of the {@link TempDirectory}
@@ -74,6 +75,13 @@ protected EngineExecutionResults executeTestsForClass(Class<?> testClass) {
 		return executeTests(requestBuilder(testClass).build());
 	}
 
+	private EngineExecutionResults executeTestsForClassWithDefaultFactory(Class<?> testClass,
+			Class<? extends TempDirFactory> factoryClass) {
+		return executeTests(requestBuilder(testClass) //
+				.configurationParameter(TempDir.DEFAULT_FACTORY_PROPERTY_NAME, factoryClass.getName()) //
+				.build());
+	}
+
 	@SuppressWarnings("deprecation")
 	private static LauncherDiscoveryRequestBuilder requestBuilder(Class<?> testClass) {
 		return request() //
@@ -295,25 +303,20 @@ void resolvesSeparateTempDirWhenAnnotationIsUsedOnAfterAllMethodParameterOnly()
 	@TestMethodOrder(OrderAnnotation.class)
 	class DefaultFactory {
 
-		private Events executeTestsForClassWithDefaultFactory(Class<?> testClass,
-				Class<? extends TempDirFactory> factoryClass) {
-			return TempDirectoryPerContextTests.super.executeTests(requestBuilder(testClass) //
-					.configurationParameter(TempDir.DEFAULT_FACTORY_PROPERTY_NAME, factoryClass.getName()) //
-					.build()).testEvents();
-		}
-
 		@Test
 		@DisplayName("set to Jupiter's default")
 		void supportsStandardDefaultFactory() {
-			executeTestsForClassWithDefaultFactory(StandardDefaultFactoryTestCase.class, TempDirFactory.Standard.class) //
-					.assertStatistics(stats -> stats.started(1).succeeded(1));
+			var results = executeTestsForClassWithDefaultFactory(StandardDefaultFactoryTestCase.class, Standard.class);
+
+			results.testEvents().assertStatistics(stats -> stats.started(1).succeeded(1));
 		}
 
 		@Test
 		@DisplayName("set to custom factory")
 		void supportsCustomDefaultFactory() {
-			executeTestsForClassWithDefaultFactory(NonStandardDefaultFactoryTestCase.class, Factory.class) //
-					.assertStatistics(stats -> stats.started(1).succeeded(1));
+			var results = executeTestsForClassWithDefaultFactory(CustomDefaultFactoryTestCase.class, Factory.class);
+
+			results.testEvents().assertStatistics(stats -> stats.started(1).succeeded(1));
 		}
 
 		private static class Factory implements TempDirFactory {
@@ -359,8 +362,7 @@ void onlySupportsParametersOfTypePathAndFile() {
 			var results = executeTestsForClass(InvalidTestCase.class);
 
 			// @formatter:off
-			TempDirectoryPerContextTests.assertSingleFailedTest(results,
-				instanceOf(ParameterResolutionException.class),
+			assertSingleFailedTest(results, instanceOf(ParameterResolutionException.class),
 				message(m -> m.matches("Failed to resolve parameter \\[java.lang.String .+] in method \\[.+]: .+")),
 				cause(
 					instanceOf(ExtensionConfigurationException.class),
@@ -389,14 +391,13 @@ void doesNotSupportTempDirAnnotationOnConstructorParameterWithTestInstancePerCla
 		}
 
 		@Test
-		@DisplayName("when @TempDir factory is not Standard")
+		@DisplayName("when non-default @TempDir factory is set")
 		@Order(32)
-		void onlySupportsStandardTempDirFactory() {
-			var results = executeTestsForClass(NonStandardFactoryTestCase.class);
+		void doesNotSupportNonDefaultTempDirFactory() {
+			var results = executeTestsForClass(NonDefaultFactoryTestCase.class);
 
 			// @formatter:off
-			TempDirectoryPerContextTests.assertSingleFailedTest(results,
-					instanceOf(ParameterResolutionException.class),
+			assertSingleFailedTest(results, instanceOf(ParameterResolutionException.class),
 					message(m -> m.matches("Failed to resolve parameter \\[.+] in method \\[.+]: .+")),
 					cause(
 							instanceOf(ExtensionConfigurationException.class),
@@ -405,6 +406,35 @@ void onlySupportsStandardTempDirFactory() {
 			// @formatter:on
 		}
 
+		@Test
+		@DisplayName("when default @TempDir factory returns null")
+		@Order(33)
+		void doesNotSupportCustomDefaultTempDirFactoryReturningNull() {
+			var results = executeTestsForClassWithDefaultFactory(CustomDefaultFactoryReturningNullTestCase.class,
+				Factory.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 must not be null")
+							)
+					));
+			// @formatter:on
+		}
+
+		private static class Factory implements TempDirFactory {
+
+			@Override
+			public Path createTempDirectory(AnnotatedElementContext elementContext, ExtensionContext extensionContext) {
+				return null;
+			}
+		}
+
 	}
 
 	@Nested
@@ -685,7 +715,7 @@ static class AnnotationOnConstructorParameterWithTestInstancePerClassTestCase
 		}
 	}
 
-	static class NonStandardFactoryTestCase {
+	static class NonDefaultFactoryTestCase {
 
 		@Test
 		void test(@SuppressWarnings("unused") @TempDir(factory = Factory.class) Path tempDir) {
@@ -712,7 +742,7 @@ void test(@TempDir Path tempDir1, @TempDir Path tempDir2) {
 
 	}
 
-	static class NonStandardDefaultFactoryTestCase {
+	static class CustomDefaultFactoryTestCase {
 
 		@Test
 		void test(@TempDir Path tempDir1, @TempDir Path tempDir2) {
@@ -721,6 +751,15 @@ void test(@TempDir Path tempDir1, @TempDir Path tempDir2) {
 
 	}
 
+	static class CustomDefaultFactoryReturningNullTestCase {
+
+		@Test
+		void test(@SuppressWarnings("unused") @TempDir Path tempDir) {
+			// never called
+		}
+
+	}
+
 	static class AnnotationOnBeforeAllMethodParameterTestCase extends BaseSharedTempDirParameterInjectionTestCase {
 
 		@BeforeAll