Add ExpectedToFail test extension (#551 / #676)

docs/docs-nav.yml

@@ -16,7 +16,7 @@
         url: /docs/disable-if-test-fails/
       - title: "Disable Parameterized Tests"
         url: /docs/disable-parameterized-tests/
-      - title: "Expected to Fail Tests"
+      - title: "Expected-to-Fail Tests"
         url: /docs/expected-to-fail-tests/
       - title: "Issue information"
         url: /docs/issue/

docs/expected-to-fail-tests.adoc

@@ -3,16 +3,16 @@
 :xp-demo-dir: ../src/demo/java
 :demo: {xp-demo-dir}/org/junitpioneer/jupiter/ExpectedToFailExtensionDemo.java
 
-Often tests fail due to a bug in the tested application or in used dependencies.
+Often tests fail due to a bug in the tested application or in dependencies.
 Traditionally such a test method would be annotated with JUnit's `@Disabled`.
-However, this has the following disadvantages when the bug causing the test failure is fixed:
+However, this has disadvantages when the bug that causes the test failure is fixed:
 
 * the developer might not notice the existing test method and create a new one
 * the existing test method might not be noticed and remains disabled for a long time after the bug has been fixed, adding no value for the project
 
-`@ExpectedToFail` tries to solve these issues.
+`@ExpectedToFail` solves these issues.
 Unlike `@Disabled` it still executes the annotated test method but aborts the test if a test failure or error occurs.
-However, if the test is executed successfully the `@ExpectedToFail` annotation will cause a test failure because the test _is working_.
+However, if the test is executed successfully the it will cause a test failure because the test _is working_.
 This lets the developer know that they have fixed the bug (possibly by accident) and that they can now remove the `@ExpectedToFail` annotation from the test method.
 
 The annotation can only be used on methods and as meta-annotation on other annotation types.
@@ -31,6 +31,9 @@ The test is aborted because the tested method `brokenMethod()` returns an incorr
 include::{demo}[tag=expected_to_fail]
 ----
 
+An aborted test is no failure and so the test suite passes (if all other tests pass, of course).
+Should `brokenMethod()` start returning the correct value, the test invocation passes, but `@ExpectedToFail` marks the test as failed to draw attention to that change in behavior.
+
 A custom message can be provided, explaining why the tested code is not working as intended at the moment.
 
 [source,java,indent=0]
@@ -40,4 +43,4 @@ include::{demo}[tag=expected_to_fail_message]
 
 == Thread-Safety
 
-This extension is thread-safe.
+This extension is safe to use during https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution[parallel test execution].

src/demo/java/org/junitpioneer/jupiter/ExpectedToFailExtensionDemo.java

@@ -17,17 +17,17 @@
 public class ExpectedToFailExtensionDemo {
 
 	// tag::expected_to_fail[]
-	@ExpectedToFail
 	@Test
+	@ExpectedToFail
 	void test() {
 		int actual = brokenMethod();
 		assertEquals(10, actual);
 	}
 	// end::expected_to_fail[]
 
 	// tag::expected_to_fail_message[]
-	@ExpectedToFail("Implementation bug in brokenMethod()")
 	@Test
+	@ExpectedToFail("Implementation bug in brokenMethod()")
 	void doSomething() {
 		int actual = brokenMethod();
 		assertEquals(10, actual);

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

@@ -22,43 +22,41 @@
 
 /**
  * {@code @ExpectedToFail} is a JUnit Jupiter extension to mark test methods as temporarily
- * 'expected to fail'.
- * Such test methods will still be executed but when they result in a test failure or error
- * the test will be aborted.
- * However, if the test method unexpectedly executes successfully, it is marked as failure
- * to let the developer know that the test is now successful and that the {@code @ExpectedToFail}
- * annotation can be removed.
+ * 'expected to fail'. Such test methods will still be executed but when they result in a test
+ * failure or error the test will be aborted. However, if the test method unexpectedly executes
+ * successfully, it is marked as failure to let the developer know that the test is now
+ * successful and that the {@code @ExpectedToFail} annotation can be removed.
  *
  * <p>The big difference compared to JUnit's {@link org.junit.jupiter.api.Disabled @Disabled}
  * annotation is that the developer is informed as soon as a test is successful again.
- * This helps avoiding writing duplicate tests by accident and counteracts the accumulation
+ * This helps to avoid creating duplicate tests by accident and counteracts the accumulation
  * of disabled tests over time.
  *
  * <p>The annotation can only be used on methods and as meta-annotation on other annotation types.
  * Similar to {@code @Disabled}, it has to be used in addition to a "testable" annotation, such
  * as {@link org.junit.jupiter.api.Test @Test}. Otherwise the annotation has no effect.
  *
  * <p><b>Important:</b> This annotation is <b>not</b> intended as a way to mark test methods
- * which intentionally cause exceptions.
- * Such test methods should use {@link org.junit.jupiter.api.Assertions#assertThrows(Class, org.junit.jupiter.api.function.Executable) assertThrows}
+ * which intentionally cause exceptions. Such test methods should use
+ * {@link org.junit.jupiter.api.Assertions#assertThrows(Class, org.junit.jupiter.api.function.Executable) assertThrows}
  * or similar means to explicitly test for a specific exception class being thrown by a
  * specific action.
  *
  * <p>For more details and examples, see
  * <a href="https://junit-pioneer.org/docs/expected-to-fail-tests/" target="_top">the documentation on <code>@ExpectedToFail</code></a>.
  * </p>
  *
+ * @since 1.8.0
  * @see org.junit.jupiter.api.Disabled
  */
+@Documented
+@Retention(RUNTIME)
 /*
- * Implementation note:
  * Only supports METHOD and ANNOTATION_TYPE as targets but not test classes because there
  * it is not clear what the 'correct' behavior would be when only a few test methods
  * execute successfully. Would the developer then have to remove the @ExpectedToFail annotation
  * from the test class and annotate methods individually?
  */
-@Documented
-@Retention(RUNTIME)
 @Target({ METHOD, ANNOTATION_TYPE })
 @ExtendWith(ExpectedToFailExtension.class)
 public @interface ExpectedToFail {

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

@@ -23,46 +23,26 @@
 
 class ExpectedToFailExtension implements Extension, InvocationInterceptor {
 
-	/**
-	 * No-arg constructor for JUnit to be able to create an instance.
-	 */
-	public ExpectedToFailExtension() {
-	}
-
-	private static ExpectedToFail getExpectedToFailAnnotation(ExtensionContext context) {
-		return AnnotationSupport
-				.findAnnotation(context.getRequiredTestMethod(), ExpectedToFail.class)
-				.orElseThrow(() -> new IllegalStateException("@ExpectedToFail is missing."));
-
-	}
-
-	/**
-	 * Returns whether the exception should be preserved and reported as is instead
-	 * of considering it an 'expected to fail' exception.
-	 *
-	 * <p>This method is used for exceptions which abort test execution and should
-	 * have higher precedence than aborted exceptions thrown by this extension.
-	 */
-	private static boolean shouldPreserveException(Throwable t) {
-		// Note: Ideally would use the same logic JUnit uses to determine if exception is aborting
-		// execution, see its class OpenTest4JAndJUnit4AwareThrowableCollector
-		return TestAbortedException.class.isInstance(t);
+	@Override
+	public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext,
+			ExtensionContext extensionContext) throws Throwable {
+		invokeAndInvertResult(invocation, extensionContext);
 	}
 
 	private static <T> T invokeAndInvertResult(Invocation<T> invocation, ExtensionContext extensionContext)
 			throws Throwable {
 		try {
 			invocation.proceed();
-			// if no exception was thrown fall through and call fail(...) eventually
+			// at this point, the invocation succeeded, so we'd want to call `fail(...)`,
+			// but that would get handled by the following `catch` and so it's easier
+			// to instead fall through to a `fail(...)` after the `catch` block
 		}
 		catch (Throwable t) {
 			if (shouldPreserveException(t)) {
 				throw t;
 			}
 
-			ExpectedToFail annotation = getExpectedToFailAnnotation(extensionContext);
-
-			String message = annotation.value();
+			String message = getExpectedToFailAnnotation(extensionContext).value();
 			if (message.isEmpty()) {
 				message = "Test marked as temporarily 'expected to fail' failed as expected";
 			}
@@ -73,10 +53,24 @@ private static <T> T invokeAndInvertResult(Invocation<T> invocation, ExtensionCo
 		return fail("Test marked as 'expected to fail' succeeded; remove @ExpectedToFail from it");
 	}
 
-	@Override
-	public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext,
-			ExtensionContext extensionContext) throws Throwable {
-		invokeAndInvertResult(invocation, extensionContext);
+	/**
+	 * Returns whether the exception should be preserved and reported as is instead
+	 * of considering it an 'expected to fail' exception.
+	 *
+	 * <p>This method is used for exceptions that abort test execution and should
+	 * have higher precedence than aborted exceptions thrown by this extension.
+	 */
+	private static boolean shouldPreserveException(Throwable t) {
+		// Note: Ideally would use the same logic JUnit uses to determine if exception is aborting
+		// execution, see its class OpenTest4JAndJUnit4AwareThrowableCollector
+		return TestAbortedException.class.isInstance(t);
+	}
+
+	private static ExpectedToFail getExpectedToFailAnnotation(ExtensionContext context) {
+		return AnnotationSupport
+				.findAnnotation(context.getRequiredTestMethod(), ExpectedToFail.class)
+				.orElseThrow(() -> new IllegalStateException("@ExpectedToFail is missing."));
+
 	}
 
 }

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

@@ -182,39 +182,39 @@ static class ExpectedToFailTestCases {
 		@interface ExpectedToFailMetaAnnotation {
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void failure() {
 			fail("failed");
 		}
 
-		@ExpectedToFailMetaAnnotation
 		@Test
+		@ExpectedToFailMetaAnnotation
 		void metaAnnotationFailure() {
 			fail("failed");
 		}
 
-		@ExpectedToFail("Custom message")
 		@Test
+		@ExpectedToFail("Custom message")
 		void failureWithMessage() {
 			fail("failed");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void exception() {
 			throw new RuntimeException("test");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void aborted() {
 			// Assumption should have higher precedence than @ExpectedToFail
 			Assumptions.assumeTrue(false, "custom assumption message");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void working() {
 			// Does not cause failure or error
 		}
@@ -232,8 +232,8 @@ void beforeEach() {
 			fail("before each");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 		}
 
@@ -250,8 +250,8 @@ void afterEach() {
 			fail("after each");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 		}
 
@@ -268,8 +268,8 @@ void afterEach() {
 			fail("after each");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 			Assumptions.assumeTrue(false, "custom assumption message");
 		}
@@ -283,8 +283,8 @@ void afterEach() {
 			Assumptions.assumeTrue(false, "custom assumption message");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 			fail("failed");
 		}
@@ -301,8 +301,8 @@ static void beforeAll() {
 			fail("before all");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 		}
 
@@ -318,8 +318,8 @@ static void afterAll() {
 			fail("after all");
 		}
 
-		@ExpectedToFail
 		@Test
+		@ExpectedToFail
 		void test() {
 		}