Align use of extension point

CONTRIBUTING.md

@@ -135,6 +135,35 @@ Classes implementing an extension's functionality should reflect that in their n
 
 Note _should_, not _must_ - there can be exceptions if well argued.
 
+#### Extension Scopes
+
+Consider the following:
+
+```java
+@YourExtension
+class MyTests {
+
+	@Test
+	void testFoo() { /* ... */ }
+
+	@Test
+	void testBar() { /* ... */ }
+
+}
+```
+
+You might ask yourself: does `@YourExtension` run
+
+1. once before/after all tests (meaning it "brackets" the test class) or
+2. once before/after each test (meaning it "brackets" each test method)?
+
+We decided to _default_ to option 2 as we believe this is less error-prone and covers more common use cases.
+Furthermore, we want to guarantee consistent behavior across different extensions.
+(Examples are e.g. `DefaultLocaleExtension` or `DefaultTimezoneExtension`.)
+
+This, however, is just a default.
+`@YourExtension` is free to diverge if it makes sense.
+
 #### Namespaces
 
 Interacting with [Jupiter's extension `Store`](https://junit.org/junit5/docs/current/user-guide/#extensions-keeping-state) requires a `Namespace` instance.

docs/default-locale-timezone.adoc

@@ -78,6 +78,8 @@ class MyLocaleTests {
 }
 ----
 
+Note that class level configurations mean that the corresponding locale is set before every test inside the annotated class.
+
 == `@DefaultTimeZone`
 
 The default `TimeZone` is specified according to the https://docs.oracle.com/javase/8/docs/api/java/util/TimeZone.html#getTimeZone(java.lang.String)[TimeZone.getTimeZone(String)] method.
@@ -121,6 +123,8 @@ class MyTimeZoneTests {
 }
 ----
 
+Note that class level configurations mean that the corresponding time zone is set before every test inside the annotated class.
+
 == Thread-Safety
 
 Since default locale and time zone are global state, reading and writing them during https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution[parallel test execution] can lead to unpredictable results and flaky tests.

docs/environment-variables.adoc

@@ -72,6 +72,8 @@ class MyEnvironmentVariableTest {
 }
 ----
 
+Also note that class level configurations mean that the corresponding environment variables are cleared/set before every test inside the annotated class.
+
 == Warnings for Reflective Access
 
 As explained above, this extension uses reflective access to change the otherwise immutable environment variables.

docs/system-properties.adoc

@@ -66,6 +66,8 @@ class MySystemPropertyTest {
 }
 ----
 
+Also note that class level configurations mean that the corresponding system properties are cleared/set before every test inside the annotated class.
+
 Sometimes, you might also need to set a system property to a value that is not a constant expression, which is required for annotation values.
 In this case, you can still leverage the restore mechanism:
 

src/main/java/org/junitpioneer/internal/PioneerUtils.java

@@ -24,6 +24,8 @@
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
 
+import org.junit.jupiter.api.extension.ExtensionContext;
+
 /**
  * Pioneer-internal utility class.
  * DO NOT USE THIS CLASS - IT MAY CHANGE SIGNIFICANTLY IN ANY MINOR UPDATE.
@@ -82,6 +84,23 @@ public static Optional<Method> findMethodCurrentOrEnclosing(Class<?> clazz, Stri
 		return method;
 	}
 
+	/**
+	 * Find all (parent) {@code ExtensionContext}s via {@link ExtensionContext#getParent()}.
+	 *
+	 * @param context the context for which to find all (parent) contexts; never {@code null}
+	 * @return a list of all contexts, beginning with the given context; never {@code null} or empty
+	 */
+	public static List<ExtensionContext> findAllContexts(ExtensionContext context) {
+		List<ExtensionContext> allContexts = new ArrayList<>();
+		allContexts.add(context);
+		List<ExtensionContext> parentContexts = context
+				.getParent()
+				.map(PioneerUtils::findAllContexts)
+				.orElse(Collections.emptyList());
+		allContexts.addAll(parentContexts);
+		return allContexts;
+	}
+
 	public static String nullSafeToString(Object object) {
 		if (object == null) {
 			return "null";

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

@@ -10,21 +10,23 @@
 
 package org.junitpioneer.jupiter;
 
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toMap;
 
 import java.lang.annotation.Annotation;
+import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.ParameterizedType;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 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;
@@ -43,60 +45,52 @@
  * @param <S> The set annotation type.
  */
 abstract class AbstractEntryBasedExtension<K, V, C extends Annotation, S extends Annotation>
-		implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback, AfterEachCallback {
-
-	@Override
-	public void beforeAll(ExtensionContext context) {
-		clearAndSetEntries(context);
-	}
+		implements BeforeEachCallback, AfterEachCallback {
 
 	@Override
 	public void beforeEach(ExtensionContext context) {
-		clearAndSetEntries(context);
+		List<ExtensionContext> contexts = PioneerUtils.findAllContexts(context);
+		// apply from outermost to innermost
+		Collections.reverse(contexts);
+		contexts.forEach(this::clearAndSetEntries);
 	}
 
 	private void clearAndSetEntries(ExtensionContext context) {
-		Set<K> entriesToClear;
-		Map<K, V> entriesToSet;
-
-		try {
-			entriesToClear = findEntriesToClear(context);
-			entriesToSet = findEntriesToSet(context);
-			preventClearAndSetSameEntries(entriesToClear, entriesToSet.keySet());
-		}
-		catch (IllegalStateException ex) {
-			throw new ExtensionConfigurationException("Don't clear/set the same entry more than once.", ex);
-		}
-
-		if (entriesToClear.isEmpty() && entriesToSet.isEmpty())
-			return;
-
-		reportWarning(context);
-		storeOriginalEntries(context, entriesToClear, entriesToSet.keySet());
-		clearEntries(entriesToClear);
-		setEntries(entriesToSet);
-	}
-
-	private Set<K> findEntriesToClear(ExtensionContext context) {
-		return findAnnotations(context, getClearAnnotationType())
+		context.getElement().ifPresent(element -> {
+			Set<K> entriesToClear;
+			Map<K, V> entriesToSet;
+
+			try {
+				entriesToClear = findEntriesToClear(element);
+				entriesToSet = findEntriesToSet(element);
+				preventClearAndSetSameEntries(entriesToClear, entriesToSet.keySet());
+			}
+			catch (IllegalStateException ex) {
+				throw new ExtensionConfigurationException("Don't clear/set the same entry more than once.", ex);
+			}
+
+			if (entriesToClear.isEmpty() && entriesToSet.isEmpty())
+				return;
+
+			reportWarning(context);
+			storeOriginalEntries(context, entriesToClear, entriesToSet.keySet());
+			clearEntries(entriesToClear);
+			setEntries(entriesToSet);
+		});
+	}
+
+	private Set<K> findEntriesToClear(AnnotatedElement element) {
+		return findAnnotations(element, getClearAnnotationType())
 				.map(clearKeyMapper())
 				.collect(PioneerUtils.distinctToSet());
 	}
 
-	private Map<K, V> findEntriesToSet(ExtensionContext context) {
-		return findAnnotations(context, getSetAnnotationType()).collect(toMap(setKeyMapper(), setValueMapper()));
+	private Map<K, V> findEntriesToSet(AnnotatedElement element) {
+		return findAnnotations(element, getSetAnnotationType()).collect(toMap(setKeyMapper(), setValueMapper()));
 	}
 
-	private <A extends Annotation> Stream<A> findAnnotations(ExtensionContext context, Class<A> clazz) {
-		/*
-		 * Implementation notes:
-		 *
-		 * This extension implements `BeforeAllCallback` and `BeforeEachCallback` and so if an outer class (i.e. a
-		 * class that the test class is @Nested within) uses this extension, this method will be called on those
-		 * extension points and discover the variables to set/clear. That means we don't need to search for
-		 * enclosing annotations here.
-		 */
-		return AnnotationSupport.findRepeatableAnnotations(context.getElement(), clazz).stream();
+	private <A extends Annotation> Stream<A> findAnnotations(AnnotatedElement element, Class<A> clazz) {
+		return AnnotationSupport.findRepeatableAnnotations(element, clazz).stream();
 	}
 
 	private Class<C> getClearAnnotationType() {
@@ -113,15 +107,14 @@ private <T> Class<T> getActualTypeArgumentAt(int index) {
 	}
 
 	private void preventClearAndSetSameEntries(Collection<K> entriesToClear, Collection<K> entriesToSet) {
-		entriesToClear
+		String duplicateEntries = entriesToClear
 				.stream()
 				.filter(entriesToSet::contains)
 				.map(Object::toString)
-				.reduce((e0, e1) -> e0 + ", " + e1)
-				.ifPresent(duplicateEntries -> {
-					throw new IllegalStateException(
-						"Cannot clear and set the following entries at the same time: " + duplicateEntries);
-				});
+				.collect(joining(", "));
+		if (!duplicateEntries.isEmpty())
+			throw new IllegalStateException(
+				"Cannot clear and set the following entries at the same time: " + duplicateEntries);
 	}
 
 	private void storeOriginalEntries(ExtensionContext context, Collection<K> entriesToClear,
@@ -139,12 +132,8 @@ private void setEntries(Map<K, V> entriesToSet) {
 
 	@Override
 	public void afterEach(ExtensionContext context) throws Exception {
-		restoreOriginalEntries(context);
-	}
-
-	@Override
-	public void afterAll(ExtensionContext context) throws Exception {
-		restoreOriginalEntries(context);
+		// apply from innermost to outermost
+		PioneerUtils.findAllContexts(context).forEach(this::restoreOriginalEntries);
 	}
 
 	private void restoreOriginalEntries(ExtensionContext context) {

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

@@ -28,7 +28,7 @@
  *
  * <p>{@code ClearEnvironmentVariable} is repeatable and can be used on the method and
  * on the class level. If a class is annotated, the configured variable will be
- * cleared for all tests inside that class.</p>
+ * cleared before every test inside that class.</p>
  *
  * <p>WARNING: Java considers environment variables to be immutable, so this extension
  * uses reflection to change them. This requires that the {@link SecurityManager}

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

@@ -28,7 +28,7 @@
  *
  * <p>{@code ClearSystemProperty} is repeatable and can be used on the method and
  * on the class level. If a class is annotated, the configured property will be
- * cleared for all tests inside that class.</p>
+ * cleared before every test inside that class.</p>
  *
  * <p>During
  * <a href="https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution" target="_top">parallel test execution</a>,

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

@@ -29,7 +29,7 @@
  *
  * <p>{@code SetEnvironmentVariable} is repeatable and can be used on the method and on
  * the class level. If a class is annotated, the configured variable will be set
- * for all tests inside that class. Any method level configurations will
+ * before every test inside that class. Any method level configurations will
  * override the class level configurations.</p>
  *
  * <p>WARNING: Java considers environment variables to be immutable, so this extension

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

@@ -29,7 +29,7 @@
  *
  * <p>{@code SetSystemProperty} is repeatable and can be used on the method and on
  * the class level. If a class is annotated, the configured property will be set
- * for all tests inside that class. Any method level configurations will
+ * before every test inside that class. Any method level configurations will
  * override the class level configurations.</p>
  *
  * <p>During

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

@@ -48,6 +48,8 @@ void tearDown() {
 
 	@Test
 	@Issue("432")
+	@WritesSystemProperty
+	@WritesEnvironmentVariable
 	@DisplayName("should not mix backups of different extensions on clear environment variable and clear system property")
 	void shouldNotMixBackupsOfDifferentExtensionsOnClearEnvironmentVariableAndClearSystemProperty() {
 		PioneerTestKit.executeTestMethod(MixBackupsTestCases.class, "clearEnvironmentVariableAndClearSystemProperty");
@@ -58,6 +60,8 @@ void shouldNotMixBackupsOfDifferentExtensionsOnClearEnvironmentVariableAndClearS
 
 	@Test
 	@Issue("432")
+	@WritesSystemProperty
+	@WritesEnvironmentVariable
 	@DisplayName("should not mix backups of different extensions on set environment variable and set system property")
 	void shouldNotMixBackupsOfDifferentExtensionsOnSetEnvironmentVariableAndSetSystemProperty() {
 		PioneerTestKit.executeTestMethod(MixBackupsTestCases.class, "setEnvironmentVariableAndSetSystemProperty");
@@ -68,6 +72,8 @@ void shouldNotMixBackupsOfDifferentExtensionsOnSetEnvironmentVariableAndSetSyste
 
 	@Test
 	@Issue("432")
+	@WritesSystemProperty
+	@WritesEnvironmentVariable
 	@DisplayName("should not mix backups of different extensions on clear environment variable and set system property")
 	void shouldNotMixBackupsOfDifferentExtensionsOnClearEnvironmentVariableAndSetSystemProperty() {
 		PioneerTestKit.executeTestMethod(MixBackupsTestCases.class, "clearEnvironmentVariableAndSetSystemProperty");
@@ -78,6 +84,8 @@ void shouldNotMixBackupsOfDifferentExtensionsOnClearEnvironmentVariableAndSetSys
 
 	@Test
 	@Issue("432")
+	@WritesSystemProperty
+	@WritesEnvironmentVariable
 	@DisplayName("should not mix backups of different extensions on set environment variable and clear system property")
 	void shouldNotMixBackupsOfDifferentExtensionsOnSetEnvironmentVariableAndClearSystemProperty() {
 		PioneerTestKit.executeTestMethod(MixBackupsTestCases.class, "setEnvironmentVariableAndClearSystemProperty");