Skip to content

Commit 3ce6d7e

Browse files
committed
Forbid TempDir on constructor parameters
Because `TempDir` on constructor parameters would create a shared temp dir which is unintuitive (see discussion in #1731), it is now forbidden and fails with a `ParameterResolutionException` that hints toward using field injection instead. Resolves #1746.
1 parent 614e46b commit 3ce6d7e

File tree

3 files changed

+36
-63
lines changed

3 files changed

+36
-63
lines changed

documentation/src/docs/asciidoc/release-notes/release-notes-5.4.0-RC1.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ repository on GitHub.
6767

6868
* The `ParentDirProvider` strategy in the `TempDirectory` extension introduced in 5.4 M1
6969
now accepts a `TempDirContext` instead of `ParameterContext`.
70+
* `@TempDir` (introduced in 5.4 M1) is no longer supported on constructor parameters.
71+
Please use field injection instead.
7072

7173
==== New Features and Improvements
7274

junit-jupiter-api/src/main/java/org/junit/jupiter/api/support/io/TempDirectory.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.lang.annotation.RetentionPolicy;
2727
import java.lang.annotation.Target;
2828
import java.lang.reflect.AnnotatedElement;
29+
import java.lang.reflect.Constructor;
2930
import java.lang.reflect.Field;
3031
import java.lang.reflect.Parameter;
3132
import java.nio.file.FileVisitResult;
@@ -393,6 +394,10 @@ private void assertValidFieldCandidate(Field field) {
393394
*/
394395
@Override
395396
public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
397+
if (parameterContext.getDeclaringExecutable() instanceof Constructor) {
398+
throw new ParameterResolutionException(
399+
"@TempDir is not supported on constructor parameters. Please use field injection instead.");
400+
}
396401
return parameterContext.isAnnotated(TempDir.class);
397402
}
398403

junit-jupiter-engine/src/test/java/org/junit/jupiter/api/support/io/TempDirectoryTests.java

Lines changed: 29 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import static org.junit.jupiter.api.Assertions.assertEquals;
1515
import static org.junit.jupiter.api.Assertions.assertNotEquals;
1616
import static org.junit.jupiter.api.Assertions.assertNotNull;
17-
import static org.junit.jupiter.api.Assertions.assertNull;
1817
import static org.junit.jupiter.api.Assertions.assertSame;
1918
import static org.junit.jupiter.api.Assertions.assertTrue;
2019
import static org.junit.jupiter.api.Assertions.fail;
@@ -102,21 +101,6 @@ void resolvesSharedTempDirWhenAnnotationIsUsedOnStaticFieldAndBeforeAllMethodPar
102101
assertSharedTempDirForFieldInjection(AnnotationOnStaticFieldAndBeforeAllMethodParameterTestCase.class);
103102
}
104103

105-
@Test
106-
@DisplayName("when @TempDir is used on instance field and constructor parameter")
107-
@Order(12)
108-
void resolvesSharedTempDirWhenAnnotationIsUsedOnInstanceFieldAndConstructorParameter() {
109-
assertSharedTempDirForFieldInjection(AnnotationOnInstanceFieldAndConstructorParameterTestCase.class);
110-
}
111-
112-
@Test
113-
@DisplayName("when @TempDir is used on instance field and constructor parameter with @TestInstance(PER_CLASS)")
114-
@Order(13)
115-
void resolvesSharedTempDirWhenAnnotationIsUsedOnInstanceFieldAndConstructorParameterWithTestInstancePerClass() {
116-
assertSharedTempDirForFieldInjection(
117-
AnnotationOnInstanceFieldAndConstructorParameterWithTestInstancePerClassTestCase.class);
118-
}
119-
120104
@Test
121105
@DisplayName("when @TempDir is used on instance field and @BeforeAll method parameter")
122106
@Order(14)
@@ -132,21 +116,6 @@ void resolvesSharedTempDirWhenAnnotationIsUsedOnInstanceFieldAndBeforeAllMethodP
132116
AnnotationOnInstanceFieldAndBeforeAllMethodParameterWithTestInstancePerClassTestCase.class);
133117
}
134118

135-
@Test
136-
@DisplayName("when @TempDir is used on constructor parameter")
137-
@Order(21)
138-
void resolvesSharedTempDirWhenAnnotationIsUsedOnConstructorParameter() {
139-
assertSharedTempDirForParameterInjection(AnnotationOnConstructorParameterTestCase.class);
140-
}
141-
142-
@Test
143-
@DisplayName("when @TempDir is used on constructor parameter with @TestInstance(PER_CLASS)")
144-
@Order(22)
145-
void resolvesSharedTempDirWhenAnnotationIsUsedOnConstructorParameterWithTestInstancePerClass() {
146-
assertSharedTempDirForParameterInjection(
147-
AnnotationOnConstructorParameterWithTestInstancePerClassTestCase.class);
148-
}
149-
150119
@Test
151120
@DisplayName("when @TempDir is used on @BeforeAll method parameter")
152121
@Order(23)
@@ -374,6 +343,26 @@ void erroneousParentDirProviderMakesTestFail() {
374343
// @formatter:on
375344
}
376345

346+
@Test
347+
@DisplayName("when @TempDir is used on constructor parameter")
348+
@Order(8)
349+
void resolvesSharedTempDirWhenAnnotationIsUsedOnConstructorParameter() {
350+
var results = executeTestsForClass(AnnotationOnConstructorParameterTestCase.class);
351+
352+
assertSingleFailedTest(results, ParameterResolutionException.class,
353+
"@TempDir is not supported on constructor parameters. Please use field injection instead.");
354+
}
355+
356+
@Test
357+
@DisplayName("when @TempDir is used on constructor parameter with @TestInstance(PER_CLASS)")
358+
@Order(9)
359+
void resolvesSharedTempDirWhenAnnotationIsUsedOnConstructorParameterWithTestInstancePerClass() {
360+
var results = executeTestsForClass(AnnotationOnConstructorParameterWithTestInstancePerClassTestCase.class);
361+
362+
assertSingleFailedContainer(results, ParameterResolutionException.class,
363+
"@TempDir is not supported on constructor parameters. Please use field injection instead.");
364+
}
365+
377366
}
378367

379368
private static void assertSingleFailedContainer(EngineExecutionResults results, Class<? extends Throwable> clazz,
@@ -508,30 +497,6 @@ static void beforeAll(@TempDir Path tempDir) {
508497

509498
}
510499

511-
static class AnnotationOnInstanceFieldAndConstructorParameterTestCase
512-
extends BaseSharedTempDirFieldInjectionTestCase {
513-
514-
AnnotationOnInstanceFieldAndConstructorParameterTestCase(@TempDir Path tempDir) {
515-
if (BaseSharedTempDirFieldInjectionTestCase.staticTempDir != null) {
516-
assertSame(BaseSharedTempDirFieldInjectionTestCase.staticTempDir, tempDir);
517-
}
518-
else {
519-
BaseSharedTempDirFieldInjectionTestCase.staticTempDir = tempDir;
520-
}
521-
assertNull(this.tempDir);
522-
assertTrue(Files.exists(tempDir));
523-
}
524-
}
525-
526-
@TestInstance(PER_CLASS)
527-
static class AnnotationOnInstanceFieldAndConstructorParameterWithTestInstancePerClassTestCase
528-
extends AnnotationOnInstanceFieldAndConstructorParameterTestCase {
529-
530-
AnnotationOnInstanceFieldAndConstructorParameterWithTestInstancePerClassTestCase(@TempDir Path tempDir) {
531-
super(tempDir);
532-
}
533-
}
534-
535500
static class AnnotationOnInstanceFieldAndBeforeAllMethodParameterTestCase
536501
extends BaseSharedTempDirFieldInjectionTestCase {
537502

@@ -608,17 +573,18 @@ static void check(Path tempDir) {
608573

609574
}
610575

611-
static class AnnotationOnConstructorParameterTestCase extends BaseSharedTempDirParameterInjectionTestCase {
576+
@ExtendWith(TempDirectory.class)
577+
static class AnnotationOnConstructorParameterTestCase {
612578

613579
AnnotationOnConstructorParameterTestCase(@TempDir Path tempDir) {
614-
if (BaseSharedTempDirParameterInjectionTestCase.tempDir != null) {
615-
assertSame(BaseSharedTempDirParameterInjectionTestCase.tempDir, tempDir);
616-
}
617-
else {
618-
BaseSharedTempDirParameterInjectionTestCase.tempDir = tempDir;
619-
}
620-
check(tempDir);
580+
// never called
621581
}
582+
583+
@Test
584+
void test() {
585+
// never called
586+
}
587+
622588
}
623589

624590
@TestInstance(PER_CLASS)

0 commit comments

Comments
 (0)