Repository: weld/weld-junit
Pull request #72 (2019-04-01 14:10:18)

Weld container should not startup for @Disabled methods
After quite some time digging I found no reasonable way to detect that we are post-processing test instance for a disabled method.

Instead, I decided to rework the extension a bit and drop `TestInstancePostProcessor` altogether.
We now start/stop Weld container during `[Before|After]EachCallback` or `[Before|After]AllCallback` depending on test lifecycle (per method versus per class).

This passes all tests for me, however I am aware that this solution is far from ideal as `TestInstancePostProcessor` phase is meant to be used exactly what we used it for. I am unsure how this can affect user-defined before/after methods in tests - in the one I added it all works and extension preceeds the interceptor on method but I found no guarantee on this in JUnit docs.

I also added a commit that bumps some versions in POMs.

mkouba: Looking at the `BeforeEachCallback` javadoc - it should be called before any `@BeforeEach` is invoked. So it should be fine.
manovotn: Fixes #71 

Patch:
diff --git a/junit-common/src/main/java/org/jboss/weld/junit/MockEjbInjectionServices.java b/junit-common/src/main/java/org/jboss/weld/junit/MockEjbInjectionServices.java
index 8c6aa81..be0027b 100644
--- a/junit-common/src/main/java/org/jboss/weld/junit/MockEjbInjectionServices.java
+++ b/junit-common/src/main/java/org/jboss/weld/junit/MockEjbInjectionServices.java
@@ -53,7 +53,7 @@ public class MockEjbInjectionServices implements EjbInjectionServices {
         };
     }
 
-    @Override
+    // only for Weld 2
     public Object resolveEjb(InjectionPoint injectionPoint) {
         return ejbFactory.apply(injectionPoint);
     }
diff --git a/junit-common/src/main/java/org/jboss/weld/junit/MockJpaInjectionServices.java b/junit-common/src/main/java/org/jboss/weld/junit/MockJpaInjectionServices.java
index 3b7d029..2167916 100644
--- a/junit-common/src/main/java/org/jboss/weld/junit/MockJpaInjectionServices.java
+++ b/junit-common/src/main/java/org/jboss/weld/junit/MockJpaInjectionServices.java
@@ -64,7 +64,7 @@ public class MockJpaInjectionServices implements JpaInjectionServices {
         };
     }
 
-    @Override
+    // only for Weld 2
     public EntityManager resolvePersistenceContext(InjectionPoint injectionPoint) {
         if (persistenceContextFactory == null) {
             throw new IllegalStateException("Persistent context factory not set, cannot resolve injection point: " + injectionPoint);
@@ -76,7 +76,7 @@ public class MockJpaInjectionServices implements JpaInjectionServices {
         throw new IllegalStateException("Not an EntityManager instance: " + context);
     }
 
-    @Override
+    // only for Weld 2
     public EntityManagerFactory resolvePersistenceUnit(InjectionPoint injectionPoint) {
         if (persistenceUnitFactory == null) {
             throw new IllegalStateException("Persistent unit factory not set, cannot resolve injection point: " + injectionPoint);
diff --git a/junit-common/src/main/java/org/jboss/weld/junit/MockResourceInjectionServices.java b/junit-common/src/main/java/org/jboss/weld/junit/MockResourceInjectionServices.java
index 0fbbb0b..2b28456 100644
--- a/junit-common/src/main/java/org/jboss/weld/junit/MockResourceInjectionServices.java
+++ b/junit-common/src/main/java/org/jboss/weld/junit/MockResourceInjectionServices.java
@@ -49,7 +49,7 @@ public class MockResourceInjectionServices implements ResourceInjectionServices
         this.resources = ImmutableMap.copyOf(resources);
     }
 
-    @Override
+    // only for Weld 2
     public Object resolveResource(InjectionPoint injectionPoint) {
         Resource resource = getResourceAnnotation(injectionPoint);
         if (resource == null) {
@@ -83,7 +83,7 @@ public class MockResourceInjectionServices implements ResourceInjectionServices
         throw new UnsupportedOperationException();
     }
 
-    @Override
+    // only for Weld 2
     public Object resolveResource(String jndiName, String mappedName) {
         throw new UnsupportedOperationException();
     }
diff --git a/junit4/pom.xml b/junit4/pom.xml
index 7074969..b584bea 100644
--- a/junit4/pom.xml
+++ b/junit4/pom.xml
@@ -45,22 +45,6 @@
 
    </dependencies>
 
-   <build>
-      <plugins>
-         <plugin>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <configuration>
-               <systemProperties>
-                  <property>
-                     <name>java.util.logging.config.file</name>
-                     <value>${project.build.testOutputDirectory}/logging.properties</value>
-                  </property>
-               </systemProperties>
-            </configuration>
-         </plugin>
-      </plugins>
-   </build>
-
    <profiles>
 
       <profile>
diff --git a/junit5/pom.xml b/junit5/pom.xml
index ad4f8ea..05454c8 100644
--- a/junit5/pom.xml
+++ b/junit5/pom.xml
@@ -48,37 +48,9 @@
          <artifactId>hibernate-jpa-2.1-api</artifactId>
          <scope>test</scope>
       </dependency>
-
+      
    </dependencies>
 
-   <build>
-      <pluginManagement>
-         <plugins>
-            <plugin>
-               <groupId>org.apache.maven.plugins</groupId>
-               <artifactId>maven-surefire-plugin</artifactId>
-               <dependencies>
-                  <!-- Surefire doesn't know JUnit 5 provider yet, we need
-                     to add it here -->
-                  <dependency>
-                     <groupId>org.junit.platform</groupId>
-                     <artifactId>junit-platform-surefire-provider</artifactId>
-                     <version>${version.junit.platform}</version>
-                  </dependency>
-               </dependencies>
-               <configuration>
-                  <systemProperties>
-                     <property>
-                        <name>java.util.logging.config.file</name>
-                        <value>${project.build.testOutputDirectory}/logging.properties</value>
-                     </property>
-                  </systemProperties>
-               </configuration>
-            </plugin>
-         </plugins>
-      </pluginManagement>
-   </build>
-
    <profiles>
 
       <profile>
diff --git a/junit5/src/main/java/org/jboss/weld/junit5/WeldJunit5Extension.java b/junit5/src/main/java/org/jboss/weld/junit5/WeldJunit5Extension.java
index 63f33ac..3335f9f 100644
--- a/junit5/src/main/java/org/jboss/weld/junit5/WeldJunit5Extension.java
+++ b/junit5/src/main/java/org/jboss/weld/junit5/WeldJunit5Extension.java
@@ -20,13 +20,13 @@ import org.jboss.weld.environment.se.Weld;
 import org.jboss.weld.util.collections.ImmutableList;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.extension.AfterAllCallback;
-import org.junit.jupiter.api.extension.AfterTestExecutionCallback;
+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.ParameterContext;
 import org.junit.jupiter.api.extension.ParameterResolutionException;
 import org.junit.jupiter.api.extension.ParameterResolver;
-import org.junit.jupiter.api.extension.TestInstancePostProcessor;
 
 import javax.enterprise.inject.spi.BeanManager;
 import java.lang.annotation.Annotation;
@@ -52,7 +52,8 @@ import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
 import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_METHOD;
 
 /**
- * JUnit 5 extension allowing to bootstrap Weld SE container for each @Test method and tear it down afterwards. Also allows to
+ * JUnit 5 extension allowing to bootstrap Weld SE container for each @Test method (or per once per test class
+ * if running {@link org.junit.jupiter.api.TestInstance.Lifecycle#PER_CLASS}) and tear it down afterwards. Also allows to
  * inject CDI beans as parameters to @Test methods and resolves all @Inject fields in test class.
  *
  * <p>
@@ -80,11 +81,29 @@ import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_METHOD;
  * @see EnableWeld
  * @see WeldJunitEnricher
  */
-public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback, TestInstancePostProcessor, AfterTestExecutionCallback, ParameterResolver {
+public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback,
+        BeforeEachCallback, AfterEachCallback, ParameterResolver {
 
     // global system property
     public static final String GLOBAL_EXPLICIT_PARAM_INJECTION = "org.jboss.weld.junit5.explicitParamInjection";
 
+    private static void storeExplicitParamResolutionInformation(ExtensionContext ec) {
+        // check system property which may have set the global explicit param injection
+        Boolean globalSettings = Boolean.valueOf(System.getProperty(GLOBAL_EXPLICIT_PARAM_INJECTION, "false"));
+        if (globalSettings) {
+            setExplicitInjectionInfoToStore(ec, true);
+            return;
+        }
+        // check class-level annotation
+        for (Annotation annotation : ec.getRequiredTestClass().getAnnotations()) {
+            if (annotation.annotationType().equals(ExplicitParamInjection.class)) {
+                setExplicitInjectionInfoToStore(ec, true);
+                break;
+            }
+        }
+
+    }
+
     @Override
     public void afterAll(ExtensionContext context) throws Exception {
         if (determineTestLifecycle(context).equals(PER_CLASS)) {
@@ -100,81 +119,17 @@ public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback,
             ServiceLoader.load(WeldJunitEnricher.class).forEach(enrichers::add);
             setEnrichersToStore(context, enrichers.build());
         }
+        // if the lifecycle is per-class, then we want to start container here
+        starWeldContainerIfAppropriate(PER_CLASS, context);
     }
 
     @Override
-    public void afterTestExecution(ExtensionContext context) throws Exception {
+    public void afterEach(ExtensionContext context) throws Exception {
         if (determineTestLifecycle(context).equals(PER_METHOD)) {
             getInitiatorFromStore(context).shutdownWeld();
         }
     }
 
-    @Override
-    public void postProcessTestInstance(Object testInstance, ExtensionContext context) throws Exception {
-        // store info about explicit param injection, either from global settings or from annotation on the test class
-        storeExplicitParamResolutionInformation(context);
-
-        // all found fields which are WeldInitiator and have @WeldSetup annotation
-        List<Field> foundInitiatorFields = new ArrayList<>();
-        WeldInitiator initiator = null;
-        // We will go through class hierarchy in search of @WeldSetup field (even private)
-        for (Class<?> clazz = testInstance.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
-            // Find @WeldSetup field using getDeclaredFields() - this allows even for private fields
-            for (Field field : clazz.getDeclaredFields()) {
-                if (field.isAnnotationPresent(WeldSetup.class)) {
-                    Object fieldInstance;
-                    try {
-                        fieldInstance = field.get(testInstance);
-                    } catch (IllegalAccessException e) {
-                        // In case we cannot get to the field, we need to set accessibility as well
-                        AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
-                            field.setAccessible(true);
-                            return null;
-                        });
-                        fieldInstance = field.get(testInstance);
-                    }
-                    if (fieldInstance != null && fieldInstance instanceof WeldInitiator) {
-                        initiator = (WeldInitiator) fieldInstance;
-                        foundInitiatorFields.add(field);
-                    } else {
-                        // Field with other type than WeldInitiator was annotated with @WeldSetup
-                        throw new IllegalStateException("@WeldSetup annotation should only be used on a field of type"
-                            + " WeldInitiator but was found on a field of type " + field.getType() + " which is declared "
-                            + "in class " + field.getDeclaringClass());
-                    }
-                }
-            }
-        }
-        // Multiple occurrences of @WeldSetup in the hierarchy will lead to an exception
-        if (foundInitiatorFields.size() > 1) {
-            throw new IllegalStateException(foundInitiatorFields.stream().map(f -> "Field type - " + f.getType() + " which is "
-                + "in " + f.getDeclaringClass()).collect(Collectors.joining("\n", "Multiple @WeldSetup annotated fields found, "
-                        + "only one is allowed! Fields found:\n", "")));
-        }
-
-        // at this point we can be sure that either no or exactly one WeldInitiator was found
-        if (initiator == null) {
-            Weld weld = WeldInitiator.createWeld();
-            WeldInitiator.Builder builder = WeldInitiator.from(weld);
-
-            weldInit(testInstance, context, weld, builder);
-
-            // Apply discovered enrichers
-            for (WeldJunitEnricher enricher : getEnrichersFromStore(context)) {
-                String property = System.getProperty(enricher.getClass().getName());
-                if (property == null || Boolean.parseBoolean(property)) {
-                    enricher.enrich(testInstance, context, weld, builder);
-                }
-            }
-
-            initiator = builder.build();
-        }
-        setInitiatorToStore(context, initiator);
-
-        // and finally, init Weld
-        setContainerToStore(context, initiator.initWeld(testInstance));
-    }
-
     protected void weldInit(Object testInstance, ExtensionContext context, Weld weld, WeldInitiator.Builder weldInitiatorBuilder) {
         weld.addPackage(false, testInstance.getClass());
     }
@@ -185,7 +140,7 @@ public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback,
         if (getContainerFromStore(extensionContext) != null) {
             List<Annotation> qualifiers = resolveQualifiers(parameterContext, getContainerFromStore(extensionContext).getBeanManager());
             return getContainerFromStore(extensionContext)
-                .select(parameterContext.getParameter().getType(), qualifiers.toArray(new Annotation[qualifiers.size()])).get();
+                    .select(parameterContext.getParameter().getType(), qualifiers.toArray(new Annotation[qualifiers.size()])).get();
         }
         return null;
     }
@@ -202,7 +157,7 @@ public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback,
             return false;
         } else {
             return getContainerFromStore(extensionContext).select(parameterContext.getParameter().getType(), qualifiers.toArray(new Annotation[qualifiers.size()]))
-                .isResolvable();
+                    .isResolvable();
         }
     }
 
@@ -240,21 +195,81 @@ public class WeldJunit5Extension implements AfterAllCallback, BeforeAllCallback,
         }
     }
 
-    private static void storeExplicitParamResolutionInformation(ExtensionContext ec) {
-        // check system property which may have set the global explicit param injection
-        Boolean globalSettings = Boolean.valueOf(System.getProperty(GLOBAL_EXPLICIT_PARAM_INJECTION, "false"));
-        if (globalSettings) {
-            setExplicitInjectionInfoToStore(ec, true);
-            return;
-        }
-        // check class-level annotation
-        for (Annotation annotation : ec.getRequiredTestClass().getAnnotations()) {
-            if (annotation.annotationType().equals(ExplicitParamInjection.class)) {
-                setExplicitInjectionInfoToStore(ec, true);
-                break;
+    @Override
+    public void beforeEach(ExtensionContext extensionContext) throws Exception {
+        starWeldContainerIfAppropriate(PER_METHOD, extensionContext);
+    }
+
+    private void starWeldContainerIfAppropriate(TestInstance.Lifecycle expectedLifecycle, ExtensionContext context) throws Exception {
+        // is the lifecycle is what we expect it to be, start Weld container
+        if (determineTestLifecycle(context).equals(expectedLifecycle)) {
+            Object testInstance = context.getTestInstance().orElseGet(null);
+            if (testInstance == null) {
+                throw new IllegalStateException("ExtensionContext.getTestInstance() returned empty Optional!");
             }
-        }
 
-    }
+            // store info about explicit param injection, either from global settings or from annotation on the test class
+            storeExplicitParamResolutionInformation(context);
+
+            // all found fields which are WeldInitiator and have @WeldSetup annotation
+            List<Field> foundInitiatorFields = new ArrayList<>();
+            WeldInitiator initiator = null;
+            // We will go through class hierarchy in search of @WeldSetup field (even private)
+            for (Class<?> clazz = testInstance.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
+                // Find @WeldSetup field using getDeclaredFields() - this allows even for private fields
+                for (Field field : clazz.getDeclaredFields()) {
+                    if (field.isAnnotationPresent(WeldSetup.class)) {
+                        Object fieldInstance;
+                        try {
+                            fieldInstance = field.get(testInstance);
+                        } catch (IllegalAccessException e) {
+                            // In case we cannot get to the field, we need to set accessibility as well
+                            AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
+                                field.setAccessible(true);
+                                return null;
+                            });
+                            fieldInstance = field.get(testInstance);
+                        }
+                        if (fieldInstance != null && fieldInstance instanceof WeldInitiator) {
+                            initiator = (WeldInitiator) fieldInstance;
+                            foundInitiatorFields.add(field);
+                        } else {
+                            // Field with other type than WeldInitiator was annotated with @WeldSetup
+                            throw new IllegalStateException("@WeldSetup annotation should only be used on a field of type"
+                                    + " WeldInitiator but was found on a field of type " + field.getType() + " which is declared "
+                                    + "in class " + field.getDeclaringClass());
+                        }
+                    }
+                }
+            }
+            // Multiple occurrences of @WeldSetup in the hierarchy will lead to an exception
+            if (foundInitiatorFields.size() > 1) {
+                throw new IllegalStateException(foundInitiatorFields.stream().map(f -> "Field type - " + f.getType() + " which is "
+                        + "in " + f.getDeclaringClass()).collect(Collectors.joining("\n", "Multiple @WeldSetup annotated fields found, "
+                        + "only one is allowed! Fields found:\n", "")));
+            }
+
+            // at this point we can be sure that either no or exactly one WeldInitiator was found
+            if (initiator == null) {
+                Weld weld = WeldInitiator.createWeld();
+                WeldInitiator.Builder builder = WeldInitiator.from(weld);
 
+                weldInit(testInstance, context, weld, builder);
+
+                // Apply discovered enrichers
+                for (WeldJunitEnricher enricher : getEnrichersFromStore(context)) {
+                    String property = System.getProperty(enricher.getClass().getName());
+                    if (property == null || Boolean.parseBoolean(property)) {
+                        enricher.enrich(testInstance, context, weld, builder);
+                    }
+                }
+
+                initiator = builder.build();
+            }
+            setInitiatorToStore(context, initiator);
+
+            // and finally, init Weld
+            setContainerToStore(context, initiator.initWeld(testInstance));
+        }
+    }
 }
diff --git a/pom.xml b/pom.xml
index d505d38..770182c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -41,11 +41,10 @@
       <maven.compiler.target>1.8</maven.compiler.target>
       <!-- Versions -->
       <version.junit4>4.12</version.junit4>
-      <version.junit.platform>1.1.0</version.junit.platform>
-      <version.junit.jupiter>5.1.0</version.junit.jupiter>
+      <version.junit.jupiter>5.4.1</version.junit.jupiter>
       <version.weld>2.4.8.Final</version.weld>
-      <version.weld3>3.0.5.Final</version.weld3>
-      <version.mockito>2.7.19</version.mockito>
+      <version.weld3>3.1.0.Final</version.weld3>
+      <version.mockito>2.25.1</version.mockito>
       <version.jboss-ejb-api>1.0.0.Final</version.jboss-ejb-api>
       <version.hibernate-jpa-api>1.0.0.Final</version.hibernate-jpa-api>
       <!-- SpotBugs properties -->
@@ -174,4 +173,25 @@
        </profile>
    </profiles>
 
+    <build>
+        <pluginManagement>
+            <plugins>
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-surefire-plugin</artifactId>
+                    <!--Minimal version with Junit 5 engine support-->
+                    <version>2.22.1</version>
+                    <configuration>
+                        <systemProperties>
+                            <property>
+                                <name>java.util.logging.config.file</name>
+                                <value>${project.build.testOutputDirectory}/logging.properties</value>
+                            </property>
+                        </systemProperties>
+                    </configuration>
+                </plugin>
+            </plugins>
+        </pluginManagement>
+    </build>
+
 </project>


Test patch:
diff --git a/junit5/src/test/java/org/jboss/weld/junit5/basic/DisabledMethodsTest.java b/junit5/src/test/java/org/jboss/weld/junit5/basic/DisabledMethodsTest.java
new file mode 100644
index 0000000..d23dcd1
--- /dev/null
+++ b/junit5/src/test/java/org/jboss/weld/junit5/basic/DisabledMethodsTest.java
@@ -0,0 +1,55 @@
+package org.jboss.weld.junit5.basic;
+
+import org.jboss.weld.environment.se.WeldContainer;
+import org.jboss.weld.junit5.WeldInitiator;
+import org.jboss.weld.junit5.WeldJunit5Extension;
+import org.jboss.weld.junit5.WeldSetup;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests that disabled methods do not trigger Weld container startup.
+ * For this test to work, @TestMethodOrder is required.
+ * Failure of this test might cause failures of other tests because of multiple active containers!
+ *
+ * @author Matej Novotny
+ */
+@ExtendWith(WeldJunit5Extension.class)
+@TestMethodOrder(value = MethodOrderer.Alphanumeric.class) // enforces ordering of methods, required!
+public class DisabledMethodsTest {
+
+    @WeldSetup
+    public WeldInitiator weld = WeldInitiator.of(Foo.class);
+
+    @BeforeEach
+    public void tryUseWeldInBeforeEach() {
+        assertTrue(weld.isRunning());
+        assertTrue(weld.select(Foo.class).isResolvable());
+    }
+
+    @AfterEach
+    public void tryUseWeldAfterEachMethod() {
+        assertTrue(weld.isRunning());
+        assertTrue(weld.select(Foo.class).isResolvable());
+    }
+
+    @Test
+    @Disabled
+    public void executedFirst() {
+        // dummy disabled test method
+    }
+
+    @Test
+    public void testRunningContainers() {
+        // we assert that there is only one container running at this point
+        assertEquals(1, WeldContainer.getRunningContainerIds().size());
+    }
+}