Fix handling of CleanupMode.ON_SUCCESS

Prior to this commit, when a test failed all temp dirs created
subsequently with `CleanupMode.ON_SUCCESS` were not deleted even if
those tests passed.

Fixes #4464.

Author: marcphilipp

Diff for: documentation/src/docs/asciidoc/release-notes/release-notes-5.12.2.adoc

@@ -35,7 +35,9 @@ on GitHub.
 [[release-notes-5.12.2-junit-jupiter-bug-fixes]]
 ==== Bug Fixes
 
-*
+* Fix handling of `CleanupMode.ON_SUCCESS` with `@TempDir` that caused no temporary
+  directories (using that mode) to be deleted after the first failure even if the
+  corresponding tests passed.
 
 [[release-notes-5.12.2-junit-jupiter-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes

Diff for: junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java

@@ -129,12 +129,17 @@ public void beforeEach(ExtensionContext context) {
 	}
 
 	private static void installFailureTracker(ExtensionContext context) {
-		context.getStore(NAMESPACE).put(FAILURE_TRACKER, (CloseableResource) () -> context.getParent() //
-				.ifPresent(parentContext -> {
-					if (selfOrChildFailed(context)) {
-						parentContext.getStore(NAMESPACE).put(CHILD_FAILED, true);
-					}
-				}));
+		context.getParent() //
+				.filter(parentContext -> !context.getRoot().equals(parentContext)) //
+				.ifPresent(parentContext -> installFailureTracker(context, parentContext));
+	}
+
+	private static void installFailureTracker(ExtensionContext context, ExtensionContext parentContext) {
+		context.getStore(NAMESPACE).put(FAILURE_TRACKER, (CloseableResource) () -> {
+			if (selfOrChildFailed(context)) {
+				getContextSpecificStore(parentContext).put(CHILD_FAILED, true);
+			}
+		});
 	}
 
 	private void injectStaticFields(ExtensionContext context, Class<?> testClass) {
@@ -286,7 +291,11 @@ static CloseablePath createTempDir(TempDirFactory factory, CleanupMode cleanupMo
 
 	private static boolean selfOrChildFailed(ExtensionContext context) {
 		return context.getExecutionException().isPresent() //
-				|| context.getStore(NAMESPACE).getOrDefault(CHILD_FAILED, Boolean.class, false);
+				|| getContextSpecificStore(context).getOrDefault(CHILD_FAILED, Boolean.class, false);
+	}
+
+	private static ExtensionContext.Store getContextSpecificStore(ExtensionContext context) {
+		return context.getStore(NAMESPACE.append(context));
 	}
 
 	static class CloseablePath implements CloseableResource {

Diff for: jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java

@@ -27,7 +27,7 @@
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 
-import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Order;
@@ -60,6 +60,7 @@ class TempDirFieldTests {
 		private static Path alwaysFieldDir;
 		private static Path onSuccessFailingFieldDir;
 		private static Path onSuccessPassingFieldDir;
+		private static Path onSuccessPassingParameterDir;
 
 		/**
 		 * Ensure the cleanup mode defaults to ALWAYS for fields.
@@ -152,6 +153,14 @@ void cleanupModeOnSuccessFailingField() {
 			assertThat(onSuccessFailingFieldDir).exists();
 		}
 
+		@Test
+		void cleanupModeOnSuccessFailingThenPassingField() {
+			executeTests(selectClass(OnSuccessFailingFieldCase.class), selectClass(OnSuccessPassingFieldCase.class));
+
+			assertThat(onSuccessFailingFieldDir).exists();
+			assertThat(onSuccessPassingFieldDir).doesNotExist();
+		}
+
 		/**
 		 * Ensure that ON_SUCCESS cleanup modes are obeyed for static fields when tests are failing.
 		 * <p/>
@@ -174,21 +183,20 @@ void cleanupModeOnSuccessFailingStaticField() {
 		 */
 		@Test
 		void cleanupModeOnSuccessFailingStaticFieldWithNesting() {
-			LauncherDiscoveryRequest request = request()//
-					.selectors(selectClass(OnSuccessFailingStaticFieldWithNestingCase.class))//
-					.build();
-			executeTests(request);
+			executeTestsForClass(OnSuccessFailingStaticFieldWithNestingCase.class);
 
 			assertThat(onSuccessFailingFieldDir).exists();
+			assertThat(onSuccessPassingParameterDir).doesNotExist();
 		}
 
-		@AfterAll
-		static void afterAll() throws IOException {
+		@AfterEach
+		void deleteTempDirs() throws IOException {
 			deleteIfNotNullAndExists(defaultFieldDir);
 			deleteIfNotNullAndExists(neverFieldDir);
 			deleteIfNotNullAndExists(alwaysFieldDir);
 			deleteIfNotNullAndExists(onSuccessFailingFieldDir);
 			deleteIfNotNullAndExists(onSuccessPassingFieldDir);
+			deleteIfNotNullAndExists(onSuccessPassingParameterDir);
 		}
 
 		static void deleteIfNotNullAndExists(Path dir) throws IOException {
@@ -286,13 +294,21 @@ static class OnSuccessFailingStaticFieldWithNestingCase {
 			static Path onSuccessFailingFieldDir;
 
 			@Nested
+			@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 			class NestedTestCase {
 
 				@Test
-				void test() {
+				@Order(1)
+				void failingTest() {
 					TempDirFieldTests.onSuccessFailingFieldDir = onSuccessFailingFieldDir;
 					fail();
 				}
+
+				@Test
+				@Order(2)
+				void passingTest(@TempDir(cleanup = ON_SUCCESS) Path tempDir) {
+					TempDirFieldTests.onSuccessPassingParameterDir = tempDir;
+				}
 			}
 		}
 
@@ -400,8 +416,16 @@ void cleanupModeOnSuccessFailingParameter() {
 			assertThat(onSuccessFailingParameterDir).exists();
 		}
 
-		@AfterAll
-		static void afterAll() throws IOException {
+		@Test
+		void cleanupModeOnSuccessFailingThenPassingParameter() {
+			executeTestsForClass(OnSuccessFailingThenPassingParameterCase.class);
+
+			assertThat(onSuccessFailingParameterDir).exists();
+			assertThat(onSuccessPassingParameterDir).doesNotExist();
+		}
+
+		@AfterEach
+		void deleteTempDirs() throws IOException {
 			TempDirFieldTests.deleteIfNotNullAndExists(defaultParameterDir);
 			TempDirFieldTests.deleteIfNotNullAndExists(neverParameterDir);
 			TempDirFieldTests.deleteIfNotNullAndExists(alwaysParameterDir);
@@ -457,6 +481,24 @@ void testOnSuccessFailingParameter(@TempDir(cleanup = ON_SUCCESS) Path onSuccess
 			}
 		}
 
+		@SuppressWarnings({ "JUnitMalformedDeclaration", "NewClassNamingConvention" })
+		@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+		static class OnSuccessFailingThenPassingParameterCase {
+
+			@Test
+			@Order(1)
+			void testOnSuccessFailingParameter(@TempDir(cleanup = ON_SUCCESS) Path onSuccessFailingParameterDir) {
+				TempDirParameterTests.onSuccessFailingParameterDir = onSuccessFailingParameterDir;
+				fail();
+			}
+
+			@Test
+			@Order(2)
+			void testOnSuccessPassingParameter(@TempDir(cleanup = ON_SUCCESS) Path onSuccessPassingParameterDir) {
+				TempDirParameterTests.onSuccessPassingParameterDir = onSuccessPassingParameterDir;
+			}
+		}
+
 	}
 
 	@Nested