Make ArchUnit JUnit 5 support Java Modules compatible #827

So far, even though we added `Automatic-Module-Name` to the `MANIFEST` to make it compatible with Java Modules, ArchUnit's JUnit 5 support could not be used on the modulepath. The reason is that all three modules `archunit-junit5-api`, `archunit-junit5-engine` and `archunit-junit5-engine-api` export the same package `com.tngtech.archunit.junit`.

We now fix this by splitting the three modules into separate packages. This is a little tricky since there is also code that is shared with ArchUnit JUnit 4 support and we want to keep as many internals package private as possible. I tried to find a good compromise between backwards compatibility and public surface, the result is

* API stays the same, i.e. toplevel `c.t.archunit.junit`
* Engine and shared internal infrastructure are moved to `c.t.archunit.junit.internal` (this causes no problem, because engine dependencies were always only transparent runtime dependencies via `ServiceLoader`, never compile time dependencies)
* Engine-API is moved to `c.t.archunit.junit.engine_api`. This is a breaking change, however the `engine-api` is a very specific artifact that is only needed for frameworks to integrate with JUnit Platform. So it should only affect a very limited number of users.

Unfortunately the JUnit 4 design makes it very hard to cleanly split API and implementation. I.e. by making the `Runner` type public API (having to explicitly declare it as `@RunWith(..)`) tends to cause a chain reaction pulling more and more into the public scope. In particular if now the classes that are used internally (like `ClassCache`) don't reside in the same package anymore. I tried to find some compromise by creating an internal delegate that is loaded via Reflection. This way we can keep the things inside of `internal` conceiled with package-private scope. Hopefully this will not cause any problem in the long term.

Note that it will still not possible to put both JUnit 4 and JUnit 5 support on the modulepath together, but I also don't see any reason why anybody should want to do this, since they serve exactly the same purpose and JUnit 5 support is just more future proof.

Resolves: #206

archunit-integration-test/src/test/java/com/tngtech/archunit/integration/CustomClassLoaderIntegrationTest.java

@@ -21,7 +21,7 @@
 import org.junit.platform.launcher.core.LauncherFactory;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.tngtech.archunit.junit.FieldSelector.selectField;
+import static com.tngtech.archunit.junit.engine_api.FieldSelector.selectField;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
 

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchUnitRunner.java

@@ -15,32 +15,20 @@
  */
 package com.tngtech.archunit.junit;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
-import com.google.common.collect.ImmutableSet;
 import com.tngtech.archunit.Internal;
 import com.tngtech.archunit.PublicAPI;
-import com.tngtech.archunit.core.domain.JavaClasses;
-import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.base.MayResolveTypesViaReflection;
+import com.tngtech.archunit.base.ReflectionUtils;
 import com.tngtech.archunit.lang.ArchRule;
 import org.junit.runner.Description;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.ParentRunner;
-import org.junit.runners.model.FrameworkField;
-import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.InitializationError;
 import org.junit.runners.model.Statement;
 
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
-import static com.tngtech.archunit.junit.ArchRuleDeclaration.elementShouldBeIgnored;
-import static com.tngtech.archunit.junit.ArchRuleDeclaration.toDeclarations;
-import static com.tngtech.archunit.junit.ArchTestExecution.getValue;
 
 /**
  * Evaluates {@link ArchRule ArchRules} against the classes inside of the packages specified via
@@ -58,169 +46,61 @@
  * }
  * </code></pre>
  *
- * The runner will cache classes between test runs, for details please refer to {@link ClassCache}.
+ * The runner will cache classes between test runs, for details please refer to {@link com.tngtech.archunit.junit.internal.ClassCache}.
  */
 @PublicAPI(usage = ACCESS)
-public class ArchUnitRunner extends ParentRunner<ArchTestExecution> {
-    private SharedCache cache = new SharedCache(); // NOTE: We want to change this in tests -> no static/final reference
+public class ArchUnitRunner<T> extends ParentRunner<T> {
+    private final InternalRunner<T> runnerDelegate;
 
     @Internal
     public ArchUnitRunner(Class<?> testClass) throws InitializationError {
         super(testClass);
-        checkAnnotation(testClass);
+        runnerDelegate = createInternalRunner(testClass);
     }
 
-    private static AnalyzeClasses checkAnnotation(Class<?> testClass) {
-        AnalyzeClasses analyzeClasses = testClass.getAnnotation(AnalyzeClasses.class);
-        ArchTestInitializationException.check(analyzeClasses != null,
-                "Class %s must be annotated with @%s",
-                testClass.getSimpleName(), AnalyzeClasses.class.getSimpleName());
-        return analyzeClasses;
+    private InternalRunner<T> createInternalRunner(Class<?> testClass) {
+        Class<InternalRunner<T>> runnerClass = classForName("com.tngtech.archunit.junit.internal.ArchUnitRunnerInternal");
+        return ReflectionUtils.newInstanceOf(runnerClass, testClass);
     }
 
-    @Override
-    protected Statement classBlock(RunNotifier notifier) {
-        final Statement statement = super.classBlock(notifier);
-        return new Statement() {
-            @Override
-            public void evaluate() throws Throwable {
-                try {
-                    statement.evaluate();
-                } finally {
-                    cache.clear(getTestClass().getJavaClass());
-                }
-            }
-        };
-    }
-
-    @Override
-    protected List<ArchTestExecution> getChildren() {
-        List<ArchTestExecution> children = new ArrayList<>();
-        children.addAll(findArchRuleFields());
-        children.addAll(findArchRuleMethods());
-        return children;
-    }
-
-    private Collection<ArchTestExecution> findArchRuleFields() {
-        List<ArchTestExecution> result = new ArrayList<>();
-        for (FrameworkField ruleField : getTestClass().getAnnotatedFields(ArchTest.class)) {
-            result.addAll(findArchRulesIn(ruleField));
-        }
-        return result;
-    }
-
-    private Set<ArchTestExecution> findArchRulesIn(FrameworkField ruleField) {
-        boolean ignore = elementShouldBeIgnored(ruleField.getField());
-        if (ruleField.getType() == ArchTests.class) {
-            return asTestExecutions(getArchRules(ruleField.getField()), ignore);
-        }
-        return Collections.<ArchTestExecution>singleton(new ArchRuleExecution(getTestClass().getJavaClass(), ruleField.getField(), ignore));
-    }
-
-    private Set<ArchTestExecution> asTestExecutions(ArchTests archTests, boolean forceIgnore) {
-        ExecutionTransformer executionTransformer = new ExecutionTransformer();
-        for (ArchRuleDeclaration<?> declaration : toDeclarations(archTests, getTestClass().getJavaClass(), ArchTest.class, forceIgnore)) {
-            declaration.handleWith(executionTransformer);
+    @SuppressWarnings("unchecked")
+    @MayResolveTypesViaReflection(reason = "Only used to load internal ArchUnit class")
+    private static <T> Class<T> classForName(String typeName) {
+        try {
+            return (Class<T>) Class.forName(typeName);
+        } catch (ClassNotFoundException e) {
+            throw new RuntimeException(e);
         }
-        return executionTransformer.getExecutions();
-    }
-
-    private ArchTests getArchRules(Field field) {
-        return getValue(field, field.getDeclaringClass());
-    }
-
-    private Collection<ArchTestExecution> findArchRuleMethods() {
-        List<ArchTestExecution> result = new ArrayList<>();
-        for (FrameworkMethod testMethod : getTestClass().getAnnotatedMethods(ArchTest.class)) {
-            boolean ignore = elementShouldBeIgnored(testMethod.getMethod());
-            result.add(new ArchTestMethodExecution(getTestClass().getJavaClass(), testMethod.getMethod(), ignore));
-        }
-        return result;
     }
 
     @Override
-    protected Description describeChild(ArchTestExecution child) {
-        return child.describeSelf();
+    protected Statement classBlock(RunNotifier notifier) {
+        return runnerDelegate.classBlock(notifier);
     }
 
     @Override
-    protected void runChild(ArchTestExecution child, RunNotifier notifier) {
-        if (child.ignore()) {
-            notifier.fireTestIgnored(describeChild(child));
-        } else {
-            notifier.fireTestStarted(describeChild(child));
-            Class<?> testClass = getTestClass().getJavaClass();
-            JavaClasses classes = cache.get().getClassesToAnalyzeFor(testClass, new JUnit4ClassAnalysisRequest(testClass));
-            child.evaluateOn(classes).notify(notifier);
-            notifier.fireTestFinished(describeChild(child));
-        }
+    protected List<T> getChildren() {
+        return runnerDelegate.getChildren();
     }
 
-    static class SharedCache {
-        private static final ClassCache cache = new ClassCache();
-
-        ClassCache get() {
-            return cache;
-        }
-
-        void clear(Class<?> testClass) {
-            cache.clear(testClass);
-        }
+    @Override
+    protected Description describeChild(T child) {
+        return runnerDelegate.describeChild(child);
     }
 
-    private static class ExecutionTransformer implements ArchRuleDeclaration.Handler {
-        private final ImmutableSet.Builder<ArchTestExecution> executions = ImmutableSet.builder();
-
-        @Override
-        public void handleFieldDeclaration(Field field, Class<?> fieldOwner, boolean ignore) {
-            executions.add(new ArchRuleExecution(fieldOwner, field, ignore));
-        }
-
-        @Override
-        public void handleMethodDeclaration(Method method, Class<?> methodOwner, boolean ignore) {
-            executions.add(new ArchTestMethodExecution(methodOwner, method, ignore));
-        }
-
-        Set<ArchTestExecution> getExecutions() {
-            return executions.build();
-        }
+    @Override
+    protected void runChild(T child, RunNotifier notifier) {
+        runnerDelegate.runChild(child, notifier);
     }
 
-    private static class JUnit4ClassAnalysisRequest implements ClassAnalysisRequest {
-        private final AnalyzeClasses analyzeClasses;
-
-        JUnit4ClassAnalysisRequest(Class<?> testClass) {
-            analyzeClasses = checkAnnotation(testClass);
-        }
-
-        @Override
-        public String[] getPackageNames() {
-            return analyzeClasses.packages();
-        }
-
-        @Override
-        public Class<?>[] getPackageRoots() {
-            return analyzeClasses.packagesOf();
-        }
-
-        @Override
-        public Class<? extends LocationProvider>[] getLocationProviders() {
-            return analyzeClasses.locations();
-        }
+    @Internal
+    public interface InternalRunner<T> {
+        Statement classBlock(RunNotifier notifier);
 
-        @Override
-        public Class<? extends ImportOption>[] getImportOptions() {
-            return analyzeClasses.importOptions();
-        }
+        List<T> getChildren();
 
-        @Override
-        public CacheMode getCacheMode() {
-            return analyzeClasses.cacheMode();
-        }
+        Description describeChild(T child);
 
-        @Override
-        public boolean scanWholeClasspath() {
-            return analyzeClasses.wholeClasspath();
-        }
+        void runChild(T child, RunNotifier notifier);
     }
 }

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchRuleDeclaration.java → archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.tngtech.archunit.junit;
+package com.tngtech.archunit.junit.internal;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
@@ -24,12 +24,14 @@
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
+import com.tngtech.archunit.junit.ArchIgnore;
+import com.tngtech.archunit.junit.ArchTests;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.tngtech.archunit.junit.ArchTestExecution.getValue;
-import static com.tngtech.archunit.junit.ReflectionUtils.getAllFields;
-import static com.tngtech.archunit.junit.ReflectionUtils.getAllMethods;
-import static com.tngtech.archunit.junit.ReflectionUtils.withAnnotation;
+import static com.tngtech.archunit.junit.internal.ArchTestExecution.getValue;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllFields;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllMethods;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.withAnnotation;
 
 abstract class ArchRuleDeclaration<T extends AnnotatedElement> {
     private final Class<?> testClass;

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchRuleExecution.java → archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleExecution.java

@@ -13,15 +13,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.tngtech.archunit.junit;
+package com.tngtech.archunit.junit.internal;
 
 import java.lang.reflect.Field;
 
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.lang.ArchRule;
 import org.junit.runner.Description;
 
-import static com.tngtech.archunit.junit.DisplayNameResolver.determineDisplayName;
+import static com.tngtech.archunit.junit.internal.DisplayNameResolver.determineDisplayName;
 
 class ArchRuleExecution extends ArchTestExecution {
     private final Field ruleField;

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchTestExecution.java → archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchTestExecution.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.tngtech.archunit.junit;
+package com.tngtech.archunit.junit.internal;
 
 import java.lang.reflect.Field;
 
@@ -22,8 +22,8 @@
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
 
-import static com.tngtech.archunit.junit.ArchTestInitializationException.WRAP_CAUSE;
-import static com.tngtech.archunit.junit.ReflectionUtils.getValueOrThrowException;
+import static com.tngtech.archunit.junit.internal.ArchTestInitializationException.WRAP_CAUSE;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.getValueOrThrowException;
 
 abstract class ArchTestExecution {
     final Class<?> testClass;

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchTestMethodExecution.java → archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchTestMethodExecution.java

@@ -13,16 +13,17 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.tngtech.archunit.junit;
+package com.tngtech.archunit.junit.internal;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
 
 import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.junit.ArchTest;
 import org.junit.runner.Description;
 
-import static com.tngtech.archunit.junit.DisplayNameResolver.determineDisplayName;
-import static com.tngtech.archunit.junit.ReflectionUtils.invokeMethod;
+import static com.tngtech.archunit.junit.internal.DisplayNameResolver.determineDisplayName;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.invokeMethod;
 
 class ArchTestMethodExecution extends ArchTestExecution {
     private final Method testMethod;