Revert disallowing private lifecycle methods

Document using private as being strongly discouraged.

Resolves #3242.

(cherry picked from commit 594f7e4)

Author: marcphilipp

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

@@ -33,6 +33,8 @@ JUnit repository on GitHub.
 
 * Exceptions thrown for undeletable files when cleaning up a temporary directory created
   via `@TempDir` now include the root cause.
+* Allow lifecycle methods to be declared as `private` again for backwards compatibility
+  but document it as a discouraged practice.
 
 ==== Deprecations and Breaking Changes
 

Diff for: junit-jupiter-api/src/main/java/org/junit/jupiter/api/AfterAll.java

@@ -29,17 +29,20 @@
  *
  * <h2>Method Signatures</h2>
  *
- * <p>{@code @AfterAll} methods must have a {@code void} return type, must not
- * be {@code private}, and must be {@code static} by default. Consequently,
- * {@code @AfterAll} methods are not supported in {@link Nested @Nested} test
- * classes or as <em>interface default methods</em> unless the test class is
- * annotated with {@link TestInstance @TestInstance(Lifecycle.PER_CLASS)}.
+ * <p>{@code @AfterAll} methods must have a {@code void} return type and must be
+ * {@code static} by default. Consequently, {@code @AfterAll} methods are not
+ * supported in {@link Nested @Nested} test classes or as <em>interface default
+ * methods</em> unless the test class is annotated with
+ * {@link TestInstance @TestInstance(Lifecycle.PER_CLASS)}.
  * However, beginning with Java 16 {@code @AfterAll} methods may be declared as
  * {@code static} in {@link Nested @Nested} test classes, and the
  * {@code Lifecycle.PER_CLASS} restriction no longer applies. {@code @AfterAll}
  * methods may optionally declare parameters to be resolved by
  * {@link org.junit.jupiter.api.extension.ParameterResolver ParameterResolvers}.
  *
+ * <p>Using {@code private} visibility for {@code @BeforeAll} methods is
+ * strongly discouraged and will be disallowed in a future release.
+ *
  * <h2>Inheritance and Execution Order</h2>
  *
  * <p>{@code @AfterAll} methods are inherited from superclasses as long as

Diff for: junit-jupiter-api/src/main/java/org/junit/jupiter/api/AfterEach.java

@@ -28,8 +28,9 @@
  *
  * <h2>Method Signatures</h2>
  *
- * <p>{@code @AfterEach} methods must have a {@code void} return type,
- * must not be {@code private}, and must not be {@code static}.
+ * <p>{@code @AfterEach} methods must have a {@code void} return type and must
+ * not be {@code static}. Using {@code private} visibility is strongly
+ * discouraged and will be disallowed in a future release.
  * They may optionally declare parameters to be resolved by
  * {@link org.junit.jupiter.api.extension.ParameterResolver ParameterResolvers}.
  *

Diff for: junit-jupiter-api/src/main/java/org/junit/jupiter/api/BeforeAll.java

@@ -29,17 +29,20 @@
  *
  * <h2>Method Signatures</h2>
  *
- * <p>{@code @BeforeAll} methods must have a {@code void} return type, must not
- * be {@code private}, and must be {@code static} by default. Consequently,
- * {@code @BeforeAll} methods are not supported in {@link Nested @Nested} test
- * classes or as <em>interface default methods</em> unless the test class is
- * annotated with {@link TestInstance @TestInstance(Lifecycle.PER_CLASS)}.
+ * <p>{@code @BeforeAll} methods must have a {@code void} return type and must
+ * be {@code static} by default. Consequently, {@code @BeforeAll} methods are
+ * not supported in {@link Nested @Nested} test classes or as <em>interface
+ * default methods</em> unless the test class is annotated with
+ * {@link TestInstance @TestInstance(Lifecycle.PER_CLASS)}.
  * However, beginning with Java 16 {@code @BeforeAll} methods may be declared as
  * {@code static} in {@link Nested @Nested} test classes, and the
  * {@code Lifecycle.PER_CLASS} restriction no longer applies. {@code @BeforeAll}
  * methods may optionally declare parameters to be resolved by
  * {@link org.junit.jupiter.api.extension.ParameterResolver ParameterResolvers}.
  *
+ * <p>Using {@code private} visibility for {@code @BeforeAll} methods is
+ * strongly discouraged and will be disallowed in a future release.
+ *
  * <h2>Inheritance and Execution Order</h2>
  *
  * <p>{@code @BeforeAll} methods are inherited from superclasses as long as

Diff for: junit-jupiter-api/src/main/java/org/junit/jupiter/api/BeforeEach.java

@@ -28,8 +28,9 @@
  *
  * <h2>Method Signatures</h2>
  *
- * <p>{@code @BeforeEach} methods must have a {@code void} return type,
- * must not be {@code private}, and must not be {@code static}.
+ * <p>{@code @BeforeEach} methods must have a {@code void} return type and must
+ * not be {@code static}. Using {@code private} visibility is strongly
+ * discouraged and will be disallowed in a future release.
  * They may optionally declare parameters to be resolved by
  * {@link org.junit.jupiter.api.extension.ParameterResolver ParameterResolvers}.
  *

Diff for: junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java

@@ -61,18 +61,14 @@ private static List<Method> findMethodsAndAssertStaticAndNonPrivate(Class<?> tes
 		if (requireStatic) {
 			methods.forEach(method -> assertStatic(annotationType, method));
 		}
-		methods.forEach(method -> assertNonPrivate(annotationType, method));
 		return methods;
 	}
 
 	private static List<Method> findMethodsAndAssertNonStaticAndNonPrivate(Class<?> testClass,
 			Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode) {
 
 		List<Method> methods = findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode);
-		methods.forEach(method -> {
-			assertNonStatic(annotationType, method);
-			assertNonPrivate(annotationType, method);
-		});
+		methods.forEach(method -> assertNonStatic(annotationType, method));
 		return methods;
 	}
 
@@ -99,13 +95,6 @@ private static void assertNonStatic(Class<? extends Annotation> annotationType,
 		}
 	}
 
-	private static void assertNonPrivate(Class<? extends Annotation> annotationType, Method method) {
-		if (ReflectionUtils.isPrivate(method)) {
-			throw new JUnitException(String.format("@%s method '%s' must not be private.",
-				annotationType.getSimpleName(), method.toGenericString()));
-		}
-	}
-
 	private static void assertVoid(Class<? extends Annotation> annotationType, Method method) {
 		if (!returnsVoid(method)) {
 			throw new JUnitException(String.format("@%s method '%s' must not return a value.",

Diff for: junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java

@@ -35,45 +35,25 @@ class InvalidLifecycleMethodConfigurationTests extends AbstractJupiterTestEngine
 
 	@Test
 	void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticBeforeAllDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidNonStaticBeforeAllMethod.class);
-	}
-
-	@Test
-	void executeValidTestCaseAlongsideTestCaseWithInvalidPrivateBeforeAllDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidPrivateBeforeAllMethod.class);
+		assertContainerFailed(TestCaseWithInvalidNonStaticBeforeAllMethod.class);
 	}
 
 	@Test
 	void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticAfterAllDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidNonStaticAfterAllMethod.class);
-	}
-
-	@Test
-	void executeValidTestCaseAlongsideTestCaseWithInvalidPrivateAfterAllDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidPrivateAfterAllMethod.class);
+		assertContainerFailed(TestCaseWithInvalidNonStaticAfterAllMethod.class);
 	}
 
 	@Test
 	void executeValidTestCaseAlongsideTestCaseWithInvalidStaticBeforeEachDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidStaticBeforeEachMethod.class);
-	}
-
-	@Test
-	void executeValidTestCaseAlongsideTestCaseWithInvalidPrivateBeforeEachDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidPrivateBeforeEachMethod.class);
+		assertContainerFailed(TestCaseWithInvalidStaticBeforeEachMethod.class);
 	}
 
 	@Test
 	void executeValidTestCaseAlongsideTestCaseWithInvalidStaticAfterEachDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidStaticAfterEachMethod.class);
-	}
-
-	@Test
-	void executeValidTestCaseAlongsideTestCaseWithInvalidPrivateAfterEachDeclaration() {
-		assertExecutionResults(TestCaseWithInvalidPrivateAfterEachMethod.class);
+		assertContainerFailed(TestCaseWithInvalidStaticAfterEachMethod.class);
 	}
 
-	private void assertExecutionResults(Class<?> invalidTestClass) {
+	private void assertContainerFailed(Class<?> invalidTestClass) {
 		EngineExecutionResults executionResults = executeTests(selectClass(TestCase.class),
 			selectClass(invalidTestClass));
 		Events containers = executionResults.containerEvents();
@@ -112,18 +92,6 @@ void test() {
 		}
 	}
 
-	static class TestCaseWithInvalidPrivateBeforeAllMethod {
-
-		// must not be private
-		@BeforeAll
-		private static void beforeAll() {
-		}
-
-		@Test
-		void test() {
-		}
-	}
-
 	static class TestCaseWithInvalidNonStaticAfterAllMethod {
 
 		// must be static
@@ -136,18 +104,6 @@ void test() {
 		}
 	}
 
-	static class TestCaseWithInvalidPrivateAfterAllMethod {
-
-		// must not be private
-		@AfterAll
-		private static void afterAll() {
-		}
-
-		@Test
-		void test() {
-		}
-	}
-
 	static class TestCaseWithInvalidStaticBeforeEachMethod {
 
 		// must NOT be static
@@ -160,18 +116,6 @@ void test() {
 		}
 	}
 
-	static class TestCaseWithInvalidPrivateBeforeEachMethod {
-
-		// must NOT be private
-		@BeforeEach
-		private void beforeEach() {
-		}
-
-		@Test
-		void test() {
-		}
-	}
-
 	static class TestCaseWithInvalidStaticAfterEachMethod {
 
 		// must NOT be static
@@ -184,16 +128,4 @@ void test() {
 		}
 	}
 
-	static class TestCaseWithInvalidPrivateAfterEachMethod {
-
-		// must NOT be private
-		@AfterEach
-		private void afterEach() {
-		}
-
-		@Test
-		void test() {
-		}
-	}
-
 }