ARQ-2231 The JUnit 5 container does not work with manual mode tests (#546)

* Run formatter on ArquillianExtension.

* Cleanup redundant modifiers in LifecycleMethodExecutor.

* ARQ-2231 Instead of implementing org.junit.jupiter.api.extension.BeforeEachCallback.beforeEach handle events within the interceptor

Author: rhusar

junit5/core/src/main/java/org/jboss/arquillian/junit5/ArquillianExtension.java

@@ -10,9 +10,7 @@
 import org.jboss.arquillian.test.spi.TestMethodExecutor;
 import org.jboss.arquillian.test.spi.TestResult;
 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.ExtensionContext;
 import org.junit.jupiter.api.extension.InvocationInterceptor;
 import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
@@ -23,7 +21,7 @@
 import static org.jboss.arquillian.junit5.ContextStore.getContextStore;
 import static org.jboss.arquillian.junit5.JUnitJupiterTestClassLifecycleManager.getManager;
 
-public class ArquillianExtension implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback, InvocationInterceptor, TestExecutionExceptionHandler {
+public class ArquillianExtension implements BeforeAllCallback, AfterAllCallback, InvocationInterceptor, TestExecutionExceptionHandler {
     public static final String RUNNING_INSIDE_ARQUILLIAN = "insideArquillian";
 
     private static final String CHAIN_EXCEPTION_MESSAGE_PREFIX = "Chain of InvocationInterceptors never called invocation";
@@ -44,22 +42,6 @@ public void afterAll(ExtensionContext context) throws Exception {
             LifecycleMethodExecutor.NO_OP);
     }
 
-    @Override
-    public void beforeEach(ExtensionContext context) throws Exception {
-        getManager(context).getAdaptor().before(
-                context.getRequiredTestInstance(),
-                context.getRequiredTestMethod(),
-                LifecycleMethodExecutor.NO_OP);
-    }
-
-    @Override
-    public void afterEach(ExtensionContext context) throws Exception {
-        getManager(context).getAdaptor().after(
-                context.getRequiredTestInstance(),
-                context.getRequiredTestMethod(),
-                LifecycleMethodExecutor.NO_OP);
-    }
-
     @Override
     public void interceptTestTemplateMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
         if (IS_INSIDE_ARQUILLIAN.test(extensionContext)) {
@@ -88,48 +70,68 @@ public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocatio
         } else {
             interceptInvocation(invocationContext, extensionContext);
             getContextStore(extensionContext).getResult(extensionContext.getUniqueId())
-                    .ifPresent(ExceptionUtils::throwAsUncheckedException);
+                .ifPresent(ExceptionUtils::throwAsUncheckedException);
         }
     }
 
     @Override
     public void interceptBeforeEachMethod(Invocation<Void> invocation,
-      ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
-      if (IS_INSIDE_ARQUILLIAN.test(extensionContext) || isRunAsClient(extensionContext)) {
-        invocation.proceed();
-      } else {
-        invocation.skip();
-      }
+                                          ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+        // Instead of implementing org.junit.jupiter.api.extension.BeforeEachCallback.beforeEach handle events within the interceptor
+        if (IS_INSIDE_ARQUILLIAN.test(extensionContext) || isRunAsClient(extensionContext)) {
+            // Since the invocation is going to proceed, the invocation must happen within the context of SPI before()
+            getManager(extensionContext).getAdaptor().before(
+                extensionContext.getRequiredTestInstance(),
+                extensionContext.getRequiredTestMethod(),
+                invocation::proceed);
+        } else {
+            // Ensure the SPI before() is called, but given that the execution is going to be skipped
+            getManager(extensionContext).getAdaptor().before(
+                extensionContext.getRequiredTestInstance(),
+                extensionContext.getRequiredTestMethod(),
+                LifecycleMethodExecutor.NO_OP);
+
+            // and ensure that the contract of the org.junit.jupiter.api.extension.InvocationInterceptor will be fulfilled.
+            invocation.skip();
+        }
     }
 
     @Override
     public void interceptAfterEachMethod(Invocation<Void> invocation,
-      ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
-      if (IS_INSIDE_ARQUILLIAN.test(extensionContext) || isRunAsClient(extensionContext)) {
-        invocation.proceed();
-      } else {
-        invocation.skip();
-      }
+                                         ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+        if (IS_INSIDE_ARQUILLIAN.test(extensionContext) || isRunAsClient(extensionContext)) {
+            getManager(extensionContext).getAdaptor().after(
+                extensionContext.getRequiredTestInstance(),
+                extensionContext.getRequiredTestMethod(),
+                invocation::proceed);
+        } else {
+            getManager(extensionContext).getAdaptor().after(
+                extensionContext.getRequiredTestInstance(),
+                extensionContext.getRequiredTestMethod(),
+                LifecycleMethodExecutor.NO_OP);
+
+            invocation.skip();
+        }
     }
 
     @Override
     public void interceptBeforeAllMethod(Invocation<Void> invocation,
-      ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
-      if (IS_INSIDE_ARQUILLIAN.test(extensionContext)) {
-        invocation.skip();
-      } else {
-        invocation.proceed();
-      }
+                                         ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+        if (IS_INSIDE_ARQUILLIAN.test(extensionContext)) {
+            invocation.skip();
+        } else {
+            invocation.proceed();
+        }
     }
 
     @Override
     public void interceptAfterAllMethod(Invocation<Void> invocation,
-      ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
-      if (IS_INSIDE_ARQUILLIAN.test(extensionContext)) {
-        invocation.skip();
-      } else {
-        invocation.proceed();
-      }
+                                        ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+        if (IS_INSIDE_ARQUILLIAN.test(extensionContext)) {
+            invocation.skip();
+        } else {
+            invocation.proceed();
+        }
     }
 
     @Override
@@ -172,7 +174,7 @@ private void populateResults(TestResult result, ExtensionContext context) {
             ContextStore contextStore = getContextStore(context);
             if (result.getThrowable() instanceof IdentifiedTestException) {
                 ((IdentifiedTestException) result.getThrowable()).getCollectedExceptions()
-                        .forEach(contextStore::storeResult);
+                    .forEach(contextStore::storeResult);
             } else {
                 contextStore.storeResult(context.getUniqueId(), result.getThrowable());
             }

test/spi/src/main/java/org/jboss/arquillian/test/spi/LifecycleMethodExecutor.java

@@ -25,10 +25,8 @@
  * @version $Revision: $
  */
 public interface LifecycleMethodExecutor {
-    public static final LifecycleMethodExecutor NO_OP = new LifecycleMethodExecutor() {
-        public void invoke() throws Throwable {
-        }
+    LifecycleMethodExecutor NO_OP = () -> {
     };
 
     void invoke() throws Throwable;
-}
+}