JUnit 5 support now allows instance fields of all types of visibility…

… as well.

Issue: #77
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>

archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/CodingRulesTest.java

@@ -17,16 +17,16 @@
 public class CodingRulesTest {
 
     @ArchTest
-    static final ArchRule NO_ACCESS_TO_STANDARD_STREAMS = NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS;
+    private final ArchRule NO_ACCESS_TO_STANDARD_STREAMS = NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS;
 
     @ArchTest
-    static void no_access_to_standard_streams_as_method(JavaClasses classes) {
+    private void no_access_to_standard_streams_as_method(JavaClasses classes) {
         noClasses().should(ACCESS_STANDARD_STREAMS).check(classes);
     }
 
     @ArchTest
-    static final ArchRule NO_GENERIC_EXCEPTIONS = NO_CLASSES_SHOULD_THROW_GENERIC_EXCEPTIONS;
+    private final ArchRule NO_GENERIC_EXCEPTIONS = NO_CLASSES_SHOULD_THROW_GENERIC_EXCEPTIONS;
 
     @ArchTest
-    static final ArchRule NO_JAVA_UTIL_LOGGING = NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING;
+    private final ArchRule NO_JAVA_UTIL_LOGGING = NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING;
 }

archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/RuleSetsTest.java

@@ -9,8 +9,8 @@
 @AnalyzeClasses(packages = "com.tngtech.archunit.example")
 class RuleSetsTest {
     @ArchTest
-    static final ArchRules CODING_RULES = ArchRules.in(CodingRulesTest.class);
+    private final ArchRules CODING_RULES = ArchRules.in(CodingRulesTest.class);
 
     @ArchTest
-    static final ArchRules CYCLIC_DEPENDENCY_RULES = ArchRules.in(CyclicDependencyRulesTest.class);
+    private final ArchRules CYCLIC_DEPENDENCY_RULES = ArchRules.in(CyclicDependencyRulesTest.class);
 }

archunit-junit/build.gradle

@@ -85,4 +85,6 @@ ext.configureJUnitArchive = { configureDependencies ->
     }
 }
 
-this.with addCleanThirdPartyTask
+this.with addCleanThirdPartyTask
+
+javadoc.enabled = false

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

@@ -16,15 +16,14 @@
 package com.tngtech.archunit.junit;
 
 import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 
-import com.tngtech.archunit.base.ArchUnitException.ReflectionException;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import org.junit.runner.Description;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
 
-import static com.tngtech.archunit.junit.ReflectionUtils.newInstanceOf;
+import static com.tngtech.archunit.junit.ArchTestInitializationException.WRAP_CAUSE;
+import static com.tngtech.archunit.junit.ReflectionUtils.getValueOrThrowException;
 
 abstract class ArchTestExecution {
     final Class<?> testClass;
@@ -51,15 +50,7 @@ boolean ignore() {
     }
 
     static <T> T getValue(Field field) {
-        try {
-            if (Modifier.isStatic(field.getModifiers())) {
-                return ReflectionUtils.getValue(field, null);
-            } else {
-                return ReflectionUtils.getValue(field, newInstanceOf(field.getDeclaringClass()));
-            }
-        } catch (ReflectionException e) {
-            throw new ArchTestInitializationException(e.getCause());
-        }
+        return getValueOrThrowException(field, WRAP_CAUSE);
     }
 
     abstract static class Result {

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

@@ -20,9 +20,8 @@
 
 import com.tngtech.archunit.core.domain.JavaClasses;
 import org.junit.runner.Description;
-import org.junit.runners.model.FrameworkMethod;
 
-import static com.tngtech.archunit.junit.ReflectionUtils.newInstanceOf;
+import static com.tngtech.archunit.junit.ReflectionUtils.invokeMethod;
 
 class ArchTestMethodExecution extends ArchTestExecution {
     private final Method testMethod;
@@ -42,14 +41,13 @@ Result evaluateOn(JavaClasses classes) {
         }
     }
 
-    private void executeTestMethod(JavaClasses classes) throws Throwable {
+    private void executeTestMethod(JavaClasses classes) {
         ArchTestInitializationException.check(
                 Arrays.equals(testMethod.getParameterTypes(), new Class<?>[]{JavaClasses.class}),
                 "Methods annotated with @%s must have exactly one parameter of type %s",
                 ArchTest.class.getSimpleName(), JavaClasses.class.getSimpleName());
 
-        testMethod.setAccessible(true);
-        new FrameworkMethod(testMethod).invokeExplosively(newInstanceOf(testClass), classes);
+        invokeMethod(testMethod, classes);
     }
 
     @Override

archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/ArchUnitTestDescriptor.java

@@ -16,10 +16,7 @@
 package com.tngtech.archunit.junit;
 
 import java.lang.reflect.Field;
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.Optional;
-import java.util.concurrent.Callable;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
@@ -35,11 +32,12 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Suppliers.memoize;
+import static com.tngtech.archunit.junit.ArchTestInitializationException.WRAP_CAUSE;
 import static com.tngtech.archunit.junit.ReflectionUtils.getAllFields;
 import static com.tngtech.archunit.junit.ReflectionUtils.getAllMethods;
-import static com.tngtech.archunit.junit.ReflectionUtils.getValue;
+import static com.tngtech.archunit.junit.ReflectionUtils.getValueOrThrowException;
+import static com.tngtech.archunit.junit.ReflectionUtils.invokeMethod;
 import static com.tngtech.archunit.junit.ReflectionUtils.withAnnotation;
-import static java.lang.reflect.Modifier.isStatic;
 
 class ArchUnitTestDescriptor extends AbstractArchUnitTestDescriptor implements CreatesChildren {
     private static final Logger LOG = LoggerFactory.getLogger(ArchUnitTestDescriptor.class);
@@ -65,8 +63,8 @@ static void resolve(TestDescriptor parent, ElementResolver resolver, ClassCache
 
     private static void createTestDescriptor(TestDescriptor parent, ClassCache classCache, Class<?> clazz, ElementResolver childResolver) {
         if (clazz.getAnnotation(AnalyzeClasses.class) == null) {
-            LOG.warn("Class {} is not annotated with @{} and thus run as top level test. "
-                            + "This warning can be ignored, if {} is only used as part of a rules library, included via {}.in({}.class).",
+            LOG.warn("Class {} is not annotated with @{} and thus cannot run as a top level test. "
+                            + "This warning can be ignored if {} is only used as part of a rules library included via {}.in({}.class).",
                     clazz.getName(), AnalyzeClasses.class.getSimpleName(),
                     clazz.getSimpleName(), ArchRules.class.getSimpleName(), clazz.getSimpleName());
 
@@ -105,10 +103,14 @@ private static void resolveChildren(
         if (ArchRules.class.isAssignableFrom(field.getType())) {
             resolveArchRules(parent, resolver, field, classes);
         } else {
-            parent.addChild(new ArchUnitRuleDescriptor(resolver.getUniqueId(), getValue(field, null), classes, field));
+            parent.addChild(new ArchUnitRuleDescriptor(resolver.getUniqueId(), getValue(field), classes, field));
         }
     }
 
+    private static <T> T getValue(Field field) {
+        return getValueOrThrowException(field, WRAP_CAUSE);
+    }
+
     private static void resolveArchRules(
             TestDescriptor parent, ElementResolver resolver, Field field, Supplier<JavaClasses> classes) {
 
@@ -124,7 +126,7 @@ private static void resolveArchRules(
     }
 
     private static DeclaredArchRules getDeclaredRules(Field field) {
-        return new DeclaredArchRules(getValue(field, null));
+        return new DeclaredArchRules(getValue(field));
     }
 
     @Override
@@ -173,11 +175,6 @@ private static class ArchUnitMethodDescriptor extends AbstractArchUnitTestDescri
         }
 
         private void validate(Method method) {
-            ArchTestInitializationException.check(
-                    isStatic(method.getModifiers()),
-                    "@%s Method %s.%s must be static",
-                    ArchTest.class.getSimpleName(), method.getDeclaringClass().getSimpleName(), method.getName());
-
             ArchTestInitializationException.check(
                     method.getParameterCount() == 1 && method.getParameterTypes()[0].equals(JavaClasses.class),
                     "@%s Method %s.%s must have exactly one parameter of type %s",
@@ -191,30 +188,9 @@ public Type getType() {
 
         @Override
         public ArchUnitEngineExecutionContext execute(ArchUnitEngineExecutionContext context, DynamicTestExecutor dynamicTestExecutor) {
-            unwrapException(() -> method.invoke(null, classes.get()))
-                    .ifPresent(this::rethrowUnchecked);
+            invokeMethod(method, classes.get());
             return context;
         }
-
-        // Exceptions occurring during reflective calls are wrapped within an InvocationTargetException
-        private Optional<Throwable> unwrapException(Callable<?> callback) {
-            try {
-                callback.call();
-                return Optional.empty();
-            } catch (Exception e) {
-                Throwable throwable = e;
-                while (throwable instanceof InvocationTargetException) {
-                    throwable = ((InvocationTargetException) e).getTargetException();
-                }
-                return Optional.of(throwable);
-            }
-        }
-
-        // Certified Hack(TM) to rethrow any exception unchecked. Uses a hole in the JLS with respect to Generics.
-        @SuppressWarnings("unchecked")
-        private <T extends Throwable> void rethrowUnchecked(Throwable throwable) throws T {
-            throw (T) throwable;
-        }
     }
 
     private static class ArchUnitRulesDescriptor extends AbstractArchUnitTestDescriptor implements CreatesChildren {

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/ArchUnitTestEngineTest.java

@@ -11,11 +11,14 @@
 import java.util.Set;
 import java.util.stream.Stream;
 
+import com.google.common.collect.ImmutableSet;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.junit.ArchUnitTestEngine.SharedCache;
+import com.tngtech.archunit.junit.testexamples.ClassWithPrivateTests;
 import com.tngtech.archunit.junit.testexamples.ComplexRuleLibrary;
 import com.tngtech.archunit.junit.testexamples.ComplexTags;
 import com.tngtech.archunit.junit.testexamples.FullAnalyzeClassesSpec;
+import com.tngtech.archunit.junit.testexamples.LibraryWithPrivateTests;
 import com.tngtech.archunit.junit.testexamples.SimpleRuleLibrary;
 import com.tngtech.archunit.junit.testexamples.TestClassWithTags;
 import com.tngtech.archunit.junit.testexamples.TestFieldWithTags;
@@ -242,6 +245,28 @@ void a_class_with_complex_hierarchy() {
                     .containsOnlyElementsOf(getExpectedIdsForComplexRuleLibrary(engineId));
         }
 
+        @Test
+        void private_instance_members() {
+            EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(ClassWithPrivateTests.class);
+
+            TestDescriptor rootDescriptor = testEngine.discover(discoveryRequest, engineId);
+
+            assertThat(getAllLeafUniqueIds(rootDescriptor))
+                    .as("all leaf unique ids of private members")
+                    .containsOnly(privateRuleFieldId(engineId), privateRuleMethodId(engineId));
+        }
+
+        @Test
+        void private_instance_libraries() {
+            EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(LibraryWithPrivateTests.class);
+
+            TestDescriptor rootDescriptor = testEngine.discover(discoveryRequest, engineId);
+
+            assertThat(getAllLeafUniqueIds(rootDescriptor))
+                    .as("all leaf unique ids of private members")
+                    .containsOnlyElementsOf(privateRuleLibraryIds(engineId));
+        }
+
         @Test
         void an_unique_id() {
             UniqueId ruleIdToDiscover = simpleRulesId(engineId)
@@ -609,20 +634,6 @@ private void assertClassSource(TestDescriptor child, Class<?> aClass) {
         private TestDescriptor findRulesDescriptor(Collection<TestDescriptor> archRulesDescriptors, Class<?> clazz) {
             return archRulesDescriptors.stream().filter(d -> d.getUniqueId().toString().contains(clazz.getSimpleName())).findFirst().get();
         }
-
-        private Set<UniqueId> getAllLeafUniqueIds(TestDescriptor rootDescriptor) {
-            return getAllLeafs(rootDescriptor).stream().map(TestDescriptor::getUniqueId).collect(toSet());
-        }
-
-        private Set<? extends TestDescriptor> getAllLeafs(TestDescriptor descriptor) {
-            Set<TestDescriptor> result = new HashSet<>();
-            descriptor.accept(possibleLeaf -> {
-                if (possibleLeaf.getChildren().isEmpty()) {
-                    result.add(possibleLeaf);
-                }
-            });
-            return result;
-        }
     }
 
     @Nested
@@ -682,6 +693,16 @@ void rule_library_with_violation() {
                     testListener.verifyViolation(testId, UnwantedClass.CLASS_VIOLATING_RULES.getSimpleName()));
         }
 
+        @Test
+        void private_instance_libraries() {
+            simulateCachedClassesForTest(LibraryWithPrivateTests.class, UnwantedClass.CLASS_VIOLATING_RULES);
+
+            EngineExecutionTestListener testListener = execute(engineId, LibraryWithPrivateTests.class);
+
+            privateRuleLibraryIds(engineId).forEach(testId ->
+                    testListener.verifyViolation(testId, UnwantedClass.CLASS_VIOLATING_RULES.getSimpleName()));
+        }
+
         @Test
         void rule_by_unique_id_without_violation() {
             UniqueId fieldRuleInLibrary = simpleRulesInLibraryId(engineId)
@@ -752,18 +773,6 @@ void cache_is_cleared_afterwards() {
 
     @Nested
     class Rejects {
-        @Test
-        void rule_method_that_is_not_static() {
-            EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(WrongRuleMethodNotStatic.class);
-
-            assertThatThrownBy(() -> testEngine.discover(discoveryRequest, engineId))
-                    .isInstanceOf(ArchTestInitializationException.class)
-                    .hasMessageContaining(ArchTest.class.getSimpleName())
-                    .hasMessageContaining(WrongRuleMethodNotStatic.class.getSimpleName())
-                    .hasMessageContaining(WrongRuleMethodNotStatic.NOT_STATIC_METHOD_NAME)
-                    .hasMessageContaining("must be static");
-        }
-
         @Test
         void rule_method_with_wrong_parameters() {
             EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(WrongRuleMethodWrongParameters.class);
@@ -932,6 +941,41 @@ private Set<UniqueId> getExpectedIdsForComplexRuleLibrary(UniqueId uniqueId) {
         return Stream.of(simpleRuleLibraryIds, simpleRulesIds).flatMap(Set::stream).collect(toSet());
     }
 
+    private Set<UniqueId> privateRuleLibraryIds(UniqueId uniqueId) {
+        UniqueId libraryId = uniqueId
+                .append(CLASS_SEGMENT_TYPE, LibraryWithPrivateTests.class.getName())
+                .append(FIELD_SEGMENT_TYPE, LibraryWithPrivateTests.PRIVATE_RULES_FIELD_NAME)
+                .append(CLASS_SEGMENT_TYPE, LibraryWithPrivateTests.SubRules.class.getName())
+                .append(FIELD_SEGMENT_TYPE, LibraryWithPrivateTests.SubRules.PRIVATE_RULES_FIELD_NAME);
+        return ImmutableSet.of(privateRuleFieldId(libraryId), privateRuleMethodId(libraryId));
+    }
+
+    private UniqueId privateRuleFieldId(UniqueId uniqueId) {
+        return uniqueId
+                .append(CLASS_SEGMENT_TYPE, ClassWithPrivateTests.class.getName())
+                .append(FIELD_SEGMENT_TYPE, ClassWithPrivateTests.PRIVATE_RULE_FIELD_NAME);
+    }
+
+    private UniqueId privateRuleMethodId(UniqueId uniqueId) {
+        return uniqueId
+                .append(CLASS_SEGMENT_TYPE, ClassWithPrivateTests.class.getName())
+                .append(METHOD_SEGMENT_TYPE, ClassWithPrivateTests.PRIVATE_RULE_METHOD_NAME);
+    }
+
+    private Set<UniqueId> getAllLeafUniqueIds(TestDescriptor rootDescriptor) {
+        return getAllLeafs(rootDescriptor).stream().map(TestDescriptor::getUniqueId).collect(toSet());
+    }
+
+    private Set<? extends TestDescriptor> getAllLeafs(TestDescriptor descriptor) {
+        Set<TestDescriptor> result = new HashSet<>();
+        descriptor.accept(possibleLeaf -> {
+            if (possibleLeaf.getChildren().isEmpty()) {
+                result.add(possibleLeaf);
+            }
+        });
+        return result;
+    }
+
     private Set<UniqueId> toUniqueIds(TestDescriptor rootDescriptor) {
         return rootDescriptor.getChildren().stream().map(TestDescriptor::getUniqueId).collect(toSet());
     }

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/testexamples/ClassWithPrivateTests.java

@@ -0,0 +1,20 @@
+package com.tngtech.archunit.junit.testexamples;
+
+import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.junit.AnalyzeClasses;
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.lang.ArchRule;
+
+@AnalyzeClasses(packages = "some.dummy.package")
+public class ClassWithPrivateTests {
+    @ArchTest
+    private final ArchRule privateRuleField = RuleThatFails.on(UnwantedClass.CLASS_VIOLATING_RULES);
+
+    @ArchTest
+    private void privateRuleMethod(JavaClasses classes) {
+        RuleThatFails.on(UnwantedClass.CLASS_VIOLATING_RULES).check(classes);
+    }
+
+    public static final String PRIVATE_RULE_FIELD_NAME = "privateRuleField";
+    public static final String PRIVATE_RULE_METHOD_NAME = "privateRuleMethod";
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/testexamples/LibraryWithPrivateTests.java

@@ -0,0 +1,20 @@
+package com.tngtech.archunit.junit.testexamples;
+
+import com.tngtech.archunit.junit.AnalyzeClasses;
+import com.tngtech.archunit.junit.ArchRules;
+import com.tngtech.archunit.junit.ArchTest;
+
+@AnalyzeClasses(packages = "some.dummy.package")
+public class LibraryWithPrivateTests {
+    @ArchTest
+    private final ArchRules privateRulesField = ArchRules.in(SubRules.class);
+
+    public static final String PRIVATE_RULES_FIELD_NAME = "privateRulesField";
+
+    public static class SubRules {
+        @ArchTest
+        private final ArchRules privateRulesField = ArchRules.in(ClassWithPrivateTests.class);
+
+        public static final String PRIVATE_RULES_FIELD_NAME = "privateRulesField";
+    }
+}