Document scope of applicability for TestWatcher implementations

Closes #3234

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

@@ -58,6 +58,8 @@ JUnit repository on GitHub.
 * The <<../user-guide/index.adoc#extensions-RandomNumberExtension, User Guide>> now
   includes an example implementation of the `RandomNumberExtension` in order to improve
   the documentation for extension registration via `@ExtendWith` on fields.
+* The scope of applicability for `TestWatcher` implementations is now more extensively
+  documented in the User Guide and Javadoc.
 
 
 [[release-notes-5.10.0-RC1-junit-vintage]]

documentation/src/docs/asciidoc/user-guide/extensions.adoc

@@ -458,9 +458,34 @@ NOTE: In contrast to the definition of "test method" presented in
 <<writing-tests-classes-and-methods>>, in this context _test method_ refers to any `@Test`
 method or `@TestTemplate` method (for example, a `@RepeatedTest` or `@ParameterizedTest`).
 
-Extensions implementing this interface can be registered at the method level or at the
-class level. In the latter case they will be invoked for any contained _test method_
-including those in `@Nested` classes.
+Extensions implementing this interface can be registered at the class level, instance
+level, or method level. When registered at the class level, a `TestWatcher` will be
+invoked for any contained _test method_ including those in `@Nested` classes. When
+registered at the method level, a `TestWatcher` will only be invoked for the _test method_
+for which it was registered.
+
+[WARNING]
+====
+If a `TestWatcher` is registered via a non-static (instance) field – for example, using
+`@RegisterExtension` – and the test class is configured with
+`@TestInstance(Lifecycle.PER_METHOD)` semantics (which is the default lifecycle mode), the
+`TestWatcher` will **not** be invoked with events for `@TestTemplate` methods (for
+example, `@RepeatedTest` or `@ParameterizedTest`).
+
+To ensure that a `TestWatcher` is invoked for all _test methods_ in a given class, it is
+therefore recommended that the `TestWatcher` be registered at the class level with
+`@ExtendWith` or via a `static` field with `@RegisterExtension` or `@ExtendWith`.
+====
+
+If there is a failure at the class level — for example, an exception thrown by a
+`@BeforeAll` method — no test results will be reported. Similarly, if the test class is
+disabled via an `ExecutionCondition` — for example, `@Disabled` — no test results will be
+reported.
+
+In contrast to other Extension APIs, a `TestWatcher` is not permitted to adversely
+influence the execution of tests. Consequently, any exception thrown by a method in the
+`TestWatcher` API will be logged at `WARNING` level and will not be allowed to propagate
+or fail test execution.
 
 [WARNING]
 ====

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/TestWatcher.java

@@ -31,9 +31,29 @@
  * {@link org.junit.jupiter.api.TestTemplate @TestTemplate} methods (e.g.,
  * {@code @RepeatedTest} and {@code @ParameterizedTest}). Moreover, if there is a
  * failure at the class level — for example, an exception thrown by a
- * {@code @BeforeAll} method — no test results will be reported.
+ * {@code @BeforeAll} method — no test results will be reported. Similarly,
+ * if the test class is disabled via an {@link ExecutionCondition} — for
+ * example, {@code @Disabled} — no test results will be reported.
  *
- * <p>Extensions implementing this API can be registered at any level.
+ * <p>Extensions implementing this interface can be registered at the class level,
+ * instance level, or method level. When registered at the class level, a
+ * {@code TestWatcher} will be invoked for any contained test method including
+ * those in {@link org.junit.jupiter.api.Nested @Nested} classes. When registered
+ * at the method level, a {@code TestWatcher} will only be invoked for the test
+ * method for which it was registered.
+ *
+ * <p><strong>WARNING</strong>: If a {@code TestWatcher} is registered via a
+ * non-static (instance) field &mdash; for example, using
+ * {@link RegisterExtension @RegisterExtension} &mdash; and the test class is
+ * configured with
+ * {@link org.junit.jupiter.api.TestInstance @TestInstance(Lifecycle.PER_METHOD)}
+ * semantics (which is the default lifecycle mode), the {@code TestWatcher} will
+ * <strong>not</strong> be invoked with events for {@code @TestTemplate} methods
+ * (such as {@code @RepeatedTest} and {@code @ParameterizedTest}). To ensure that
+ * a {@code TestWatcher} is invoked for all test methods in a given class, it is
+ * therefore recommended that the {@code TestWatcher} be registered at the class
+ * level with {@link ExtendWith @ExtendWith} or via a {@code static} field with
+ * {@code @RegisterExtension} or {@code @ExtendWith}.
  *
  * <h2>Exception Handling</h2>
  *

junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestWatcherTests.java

@@ -33,12 +33,18 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.MethodOrderer.OrderAnnotation;
 import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.RepeatedTest;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.TestInstance.Lifecycle;
+import org.junit.jupiter.api.TestMethodOrder;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.api.extension.TestWatcher;
 import org.junit.jupiter.api.fixtures.TrackLogRecords;
 import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
@@ -125,11 +131,60 @@ void testWatcherIsInvokedForTestMethodsInTestCaseWithProblematicConstructor() {
 		assertThat(TrackingTestWatcher.results.get("testFailed")).hasSize(8);
 	}
 
+	@Test
+	void testWatcherSemanticsWhenRegisteredAtClassLevel() {
+		Class<?> testClass = ClassLevelTestWatcherTestCase.class;
+		assertStatsForAbstractDisabledMethodsTestCase(testClass);
+
+		// We get "testDisabled" events for the @Test method and the @RepeatedTest container.
+		assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test", "repeatedTest");
+	}
+
+	@Test
+	void testWatcherSemanticsWhenRegisteredAtInstanceLevelWithTestInstanceLifecyclePerClass() {
+		Class<?> testClass = TestInstancePerClassInstanceLevelTestWatcherTestCase.class;
+		assertStatsForAbstractDisabledMethodsTestCase(testClass);
+
+		// We get "testDisabled" events for the @Test method and the @RepeatedTest container.
+		assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test", "repeatedTest");
+	}
+
+	@Test
+	void testWatcherSemanticsWhenRegisteredAtInstanceLevelWithTestInstanceLifecyclePerMethod() {
+		Class<?> testClass = TestInstancePerMethodInstanceLevelTestWatcherTestCase.class;
+		assertStatsForAbstractDisabledMethodsTestCase(testClass);
+
+		// Since the TestWatcher is registered at the instance level with test instance
+		// lifecycle per-method semantics, we get a "testDisabled" event only for the @Test
+		// method and NOT for the @RepeatedTest container.
+		assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test");
+	}
+
+	@Test
+	void testWatcherSemanticsWhenRegisteredAtMethodLevel() {
+		Class<?> testClass = MethodLevelTestWatcherTestCase.class;
+		assertStatsForAbstractDisabledMethodsTestCase(testClass);
+
+		// We get "testDisabled" events for the @Test method and the @RepeatedTest container.
+		assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test", "repeatedTest");
+	}
+
 	private void assertCommonStatistics(EngineExecutionResults results) {
 		results.containerEvents().assertStatistics(stats -> stats.started(3).succeeded(3).failed(0));
 		results.testEvents().assertStatistics(stats -> stats.skipped(2).started(6).succeeded(2).aborted(2).failed(2));
 	}
 
+	private void assertStatsForAbstractDisabledMethodsTestCase(Class<?> testClass) {
+		EngineExecutionResults results = executeTestsForClass(testClass);
+
+		results.containerEvents().assertStatistics(//
+			stats -> stats.skipped(1).started(2).succeeded(2).aborted(0).failed(0));
+		results.testEvents().assertStatistics(//
+			stats -> stats.skipped(1).started(0).succeeded(0).aborted(0).failed(0));
+
+		assertThat(TrackingTestWatcher.results.keySet()).containsExactly("testDisabled");
+	}
+
 	// -------------------------------------------------------------------------
 
 	private static abstract class AbstractTestCase {
@@ -251,6 +306,61 @@ static class ProblematicConstructorTestCase extends AbstractTestCase {
 		}
 	}
 
+	@TestMethodOrder(OrderAnnotation.class)
+	private static abstract class AbstractDisabledMethodsTestCase {
+
+		@Disabled
+		@Test
+		@Order(1)
+		void test() {
+		}
+
+		@Disabled
+		@RepeatedTest(2)
+		@Order(2)
+		void repeatedTest() {
+		}
+	}
+
+	static class ClassLevelTestWatcherTestCase extends AbstractDisabledMethodsTestCase {
+
+		@RegisterExtension
+		static TestWatcher watcher = new TrackingTestWatcher();
+	}
+
+	@TestInstance(Lifecycle.PER_CLASS)
+	static class TestInstancePerClassInstanceLevelTestWatcherTestCase extends AbstractDisabledMethodsTestCase {
+
+		@RegisterExtension
+		TestWatcher watcher = new TrackingTestWatcher();
+	}
+
+	@TestInstance(Lifecycle.PER_METHOD)
+	static class TestInstancePerMethodInstanceLevelTestWatcherTestCase extends AbstractDisabledMethodsTestCase {
+
+		@RegisterExtension
+		TestWatcher watcher = new TrackingTestWatcher();
+	}
+
+	static class MethodLevelTestWatcherTestCase extends AbstractDisabledMethodsTestCase {
+
+		@Override
+		@Disabled
+		@Test
+		@Order(1)
+		@ExtendWith(TrackingTestWatcher.class)
+		void test() {
+		}
+
+		@Override
+		@Disabled
+		@RepeatedTest(1)
+		@Order(2)
+		@ExtendWith(TrackingTestWatcher.class)
+		void repeatedTest() {
+		}
+	}
+
 	private static class TrackingTestWatcher implements TestWatcher {
 
 		private static final Map<String, List<String>> results = new HashMap<>();