Make entry-based extensions execute around classes

src/main/java/org/junitpioneer/jupiter/AbstractEntryBasedExtension.java

@@ -27,7 +27,9 @@
 import java.util.function.Function;
 import java.util.stream.Stream;
 
+import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionConfigurationException;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -46,22 +48,31 @@
  * @param <S> The set annotation type.
  */
 abstract class AbstractEntryBasedExtension<K, V, C extends Annotation, S extends Annotation>
-		implements BeforeEachCallback, AfterEachCallback {
+		implements BeforeEachCallback, AfterEachCallback, BeforeAllCallback, AfterAllCallback {
+
+	@Override
+	public void beforeAll(ExtensionContext context) {
+		applyForAllContexts(context);
+	}
 
 	@Override
 	public void beforeEach(ExtensionContext context) {
+		applyForAllContexts(context);
+	}
+
+	private void applyForAllContexts(ExtensionContext originalContext) {
 		/*
 		 * We cannot use PioneerAnnotationUtils#findAllEnclosingRepeatableAnnotations(ExtensionContext, Class) or the
 		 * like as clearing and setting might interfere. Therefore, we have to apply the extension from the outermost
 		 * to the innermost ExtensionContext.
 		 */
-		List<ExtensionContext> contexts = PioneerUtils.findAllContexts(context);
+		List<ExtensionContext> contexts = PioneerUtils.findAllContexts(originalContext);
 		Collections.reverse(contexts);
-		contexts.forEach(this::clearAndSetEntries);
+		contexts.forEach(currentContext -> clearAndSetEntries(currentContext, originalContext));
 	}
 
-	private void clearAndSetEntries(ExtensionContext context) {
-		context.getElement().ifPresent(element -> {
+	private void clearAndSetEntries(ExtensionContext currentContext, ExtensionContext originalContext) {
+		currentContext.getElement().ifPresent(element -> {
 			Set<K> entriesToClear;
 			Map<K, V> entriesToSet;
 
@@ -77,8 +88,8 @@ private void clearAndSetEntries(ExtensionContext context) {
 			if (entriesToClear.isEmpty() && entriesToSet.isEmpty())
 				return;
 
-			reportWarning(context);
-			storeOriginalEntries(context, entriesToClear, entriesToSet.keySet());
+			reportWarning(currentContext);
+			storeOriginalEntries(originalContext, entriesToClear, entriesToSet.keySet());
 			clearEntries(entriesToClear);
 			setEntries(entriesToSet);
 		});
@@ -138,13 +149,24 @@ private void setEntries(Map<K, V> entriesToSet) {
 	}
 
 	@Override
-	public void afterEach(ExtensionContext context) throws Exception {
-		// apply from innermost to outermost
-		PioneerUtils.findAllContexts(context).forEach(this::restoreOriginalEntries);
+	public void afterEach(ExtensionContext context) {
+		restoreForAllContexts(context);
+	}
+
+	@Override
+	public void afterAll(ExtensionContext context) {
+		restoreForAllContexts(context);
+	}
+
+	private void restoreForAllContexts(ExtensionContext originalContext) {
+		// restore from innermost to outermost
+		PioneerUtils.findAllContexts(originalContext).forEach(__ -> restoreOriginalEntries(originalContext));
 	}
 
-	private void restoreOriginalEntries(ExtensionContext context) {
-		getStore(context).getOrDefault(getStoreKey(context), EntriesBackup.class, new EntriesBackup()).restoreBackup();
+	private void restoreOriginalEntries(ExtensionContext originalContext) {
+		getStore(originalContext)
+				.getOrDefault(getStoreKey(originalContext), EntriesBackup.class, new EntriesBackup())
+				.restoreBackup();
 	}
 
 	private Store getStore(ExtensionContext context) {

src/test/java/org/junitpioneer/jupiter/EnvironmentVariableExtensionTests.java

@@ -18,6 +18,7 @@
 import static org.junitpioneer.testkit.assertion.PioneerAssert.assertThat;
 
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
@@ -47,11 +48,8 @@ static void globalSetUp() {
 
 	@AfterAll
 	static void globalTearDown() {
-		assertThat(systemEnvironmentVariable("set envvar A")).isEqualTo("old A");
 		EnvironmentVariableUtils.clear("set envvar A");
-		assertThat(systemEnvironmentVariable("set envvar B")).isEqualTo("old B");
 		EnvironmentVariableUtils.clear("set envvar B");
-		assertThat(systemEnvironmentVariable("set envvar C")).isEqualTo("old C");
 		EnvironmentVariableUtils.clear("set envvar C");
 
 		assertThat(systemEnvironmentVariable("clear envvar D")).isNull();
@@ -244,7 +242,42 @@ class ResettingEnvironmentVariableTests {
 		@Nested
 		@SetEnvironmentVariable(key = "set envvar A", value = "newer A")
 		@TestInstance(TestInstance.Lifecycle.PER_CLASS)
-		class ResettingEnvironmentVariableNestedTests {
+		class ResettingEnvironmentVariableAfterEachNestedTests {
+
+			@BeforeEach
+			@ReadsEnvironmentVariable
+			void changeShouldBeVisible() {
+				// we already see "newest A" because BeforeEachCallBack is invoked before @BeforeEach
+				// see https://junit.org/junit5/docs/current/user-guide/#extensions-execution-order-overview
+				assertThat(System.getenv("set envvar A")).isEqualTo("newest A");
+			}
+
+			@Test
+			@SetEnvironmentVariable(key = "set envvar A", value = "newest A")
+			void setForTestMethod() {
+				assertThat(System.getenv("set envvar A")).isEqualTo("newest A");
+			}
+
+			@AfterEach
+			@ReadsEnvironmentVariable
+			void resetAfterTestMethodExecution() {
+				// we still see "newest A" because AfterEachCallBack is invoked after @AfterEach
+				// see https://junit.org/junit5/docs/current/user-guide/#extensions-execution-order-overview
+				assertThat(System.getenv("set envvar A")).isEqualTo("newest A");
+			}
+
+		}
+
+		@Nested
+		@SetEnvironmentVariable(key = "set envvar A", value = "newer A")
+		@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+		class ResettingEnvironmentVariableAfterAllNestedTests {
+
+			@BeforeAll
+			@ReadsEnvironmentVariable
+			void changeShouldBeVisible() {
+				assertThat(System.getenv("set envvar A")).isEqualTo("newer A");
+			}
 
 			@Test
 			@SetEnvironmentVariable(key = "set envvar A", value = "newest A")
@@ -255,15 +288,15 @@ void setForTestMethod() {
 			@AfterAll
 			@ReadsEnvironmentVariable
 			void resetAfterTestMethodExecution() {
-				assertThat(System.getenv("set envvar A")).isEqualTo("old A");
+				assertThat(System.getenv("set envvar A")).isEqualTo("newer A");
 			}
 
 		}
 
 		@AfterAll
 		@ReadsEnvironmentVariable
 		void resetAfterTestContainerExecution() {
-			assertThat(System.getenv("set envvar A")).isEqualTo("old A");
+			assertThat(System.getenv("set envvar A")).isEqualTo("new A");
 		}
 
 	}

src/test/java/org/junitpioneer/jupiter/SystemPropertyExtensionTests.java

@@ -11,10 +11,13 @@
 package org.junitpioneer.jupiter;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junitpioneer.testkit.PioneerTestKit.executeTestMethod;
 import static org.junitpioneer.testkit.assertion.PioneerAssert.assertThat;
 
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.MethodOrderer.OrderAnnotation;
@@ -25,7 +28,6 @@
 import org.junit.jupiter.api.TestMethodOrder;
 import org.junit.jupiter.api.extension.ExtensionConfigurationException;
 import org.junitpioneer.testkit.ExecutionResults;
-import org.junitpioneer.testkit.PioneerTestKit;
 
 @DisplayName("SystemProperty extension")
 class SystemPropertyExtensionTests {
@@ -43,11 +45,8 @@ static void globalSetUp() {
 
 	@AfterAll
 	static void globalTearDown() {
-		assertThat(System.getProperty("A")).isEqualTo("old A");
 		System.clearProperty("A");
-		assertThat(System.getProperty("B")).isEqualTo("old B");
 		System.clearProperty("B");
-		assertThat(System.getProperty("C")).isEqualTo("old C");
 		System.clearProperty("C");
 
 		assertThat(System.getProperty("clear prop D")).isNull();
@@ -236,7 +235,40 @@ class ResettingSystemPropertyTests {
 		@Nested
 		@SetSystemProperty(key = "A", value = "newer A")
 		@TestInstance(TestInstance.Lifecycle.PER_CLASS)
-		class ResettingSystemPropertyNestedTests {
+		class ResettingSystemPropertyAfterEachNestedTests {
+
+			@BeforeEach
+			void changeShouldBeVisible() {
+				// we already see "newest A" because BeforeEachCallBack is invoked before @BeforeEach
+				// see https://junit.org/junit5/docs/current/user-guide/#extensions-execution-order-overview
+				assertThat(System.getProperty("A")).isEqualTo("newest A");
+			}
+
+			@Test
+			@SetSystemProperty(key = "A", value = "newest A")
+			void setForTestMethod() {
+				assertThat(System.getProperty("A")).isEqualTo("newest A");
+			}
+
+			@AfterEach
+			@ReadsSystemProperty
+			void resetAfterTestMethodExecution() {
+				// we still see "newest A" because AfterEachCallBack is invoked after @AfterEach
+				// see https://junit.org/junit5/docs/current/user-guide/#extensions-execution-order-overview
+				assertThat(System.getProperty("A")).isEqualTo("newest A");
+			}
+
+		}
+
+		@Nested
+		@SetSystemProperty(key = "A", value = "newer A")
+		@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+		class ResettingSystemPropertyAfterAllNestedTests {
+
+			@BeforeAll
+			void changeShouldBeVisible() {
+				assertThat(System.getProperty("A")).isEqualTo("newer A");
+			}
 
 			@Test
 			@SetSystemProperty(key = "A", value = "newest A")
@@ -247,15 +279,15 @@ void setForTestMethod() {
 			@AfterAll
 			@ReadsSystemProperty
 			void resetAfterTestMethodExecution() {
-				assertThat(System.getProperty("A")).isEqualTo("old A");
+				assertThat(System.getProperty("A")).isEqualTo("newer A");
 			}
 
 		}
 
 		@AfterAll
 		@ReadsSystemProperty
 		void resetAfterTestContainerExecution() {
-			assertThat(System.getProperty("A")).isEqualTo("old A");
+			assertThat(System.getProperty("A")).isEqualTo("new A");
 		}
 
 	}
@@ -267,9 +299,8 @@ class ConfigurationFailureTests {
 		@Test
 		@DisplayName("should fail when clear and set same system property")
 		void shouldFailWhenClearAndSetSameSystemProperty() {
-			ExecutionResults results = PioneerTestKit
-					.executeTestMethod(MethodLevelInitializationFailureTestCases.class,
-						"shouldFailWhenClearAndSetSameSystemProperty");
+			ExecutionResults results = executeTestMethod(MethodLevelInitializationFailureTestCases.class,
+				"shouldFailWhenClearAndSetSameSystemProperty");
 
 			assertThat(results).hasSingleFailedTest().withExceptionInstanceOf(ExtensionConfigurationException.class);
 		}
@@ -280,19 +311,17 @@ void shouldFailWhenClearAndSetSameSystemProperty() {
 				+ "deduplicates identical annotations like the ones required for this test: "
 				+ "https://github.com/junit-team/junit5/issues/2131")
 		void shouldFailWhenClearSameSystemPropertyTwice() {
-			ExecutionResults results = PioneerTestKit
-					.executeTestMethod(MethodLevelInitializationFailureTestCases.class,
-						"shouldFailWhenClearSameSystemPropertyTwice");
+			ExecutionResults results = executeTestMethod(MethodLevelInitializationFailureTestCases.class,
+				"shouldFailWhenClearSameSystemPropertyTwice");
 
 			assertThat(results).hasSingleFailedTest().withExceptionInstanceOf(ExtensionConfigurationException.class);
 		}
 
 		@Test
 		@DisplayName("should fail when set same system property twice")
 		void shouldFailWhenSetSameSystemPropertyTwice() {
-			ExecutionResults results = PioneerTestKit
-					.executeTestMethod(MethodLevelInitializationFailureTestCases.class,
-						"shouldFailWhenSetSameSystemPropertyTwice");
+			ExecutionResults results = executeTestMethod(MethodLevelInitializationFailureTestCases.class,
+				"shouldFailWhenSetSameSystemPropertyTwice");
 			assertThat(results).hasSingleFailedTest().withExceptionInstanceOf(ExtensionConfigurationException.class);
 		}