add JUnit5-support for @ArchTest members in abstract base classes

So far, having `@ArchTest` fields or methods in an abstract base class would cause the JUnit 5 engine to crash. This also made the pattern to "share" tests via extending a common base class impossible, which some users would like to have.

The change corresponds to the commits
* 8188cdc
* 625c3d6
* 59f8c4d
which added the same feature for JUnit4 (Issue: #104).

Signed-off-by: Manfred Hanke <[email protected]>

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

@@ -15,7 +15,9 @@
  */
 package com.tngtech.archunit.junit.internal;
 
+import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
+import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -87,37 +89,37 @@ public void createChildren(ElementResolver resolver) {
         Supplier<JavaClasses> classes = () -> classCache.getClassesToAnalyzeFor(testClass, new JUnit5ClassAnalysisRequest(testClass));
 
         getAllFields(testClass, withAnnotation(ArchTest.class))
-                .forEach(field -> resolveField(resolver, classes, field));
+                .forEach(field -> resolveField(resolver, classes, new TestMember<>(testClass, field)));
         getAllMethods(testClass, withAnnotation(ArchTest.class))
-                .forEach(method -> resolveMethod(resolver, classes, method));
+                .forEach(method -> resolveMethod(resolver, classes, new TestMember<>(testClass, method)));
     }
 
-    private void resolveField(ElementResolver resolver, Supplier<JavaClasses> classes, Field field) {
-        resolver.resolveField(field)
+    private void resolveField(ElementResolver resolver, Supplier<JavaClasses> classes, TestMember<Field> field) {
+        resolver.resolveField(field.member)
                 .ifUnresolved(childResolver -> resolveChildren(this, childResolver, field, classes));
     }
 
-    private void resolveMethod(ElementResolver resolver, Supplier<JavaClasses> classes, Method method) {
-        resolver.resolveMethod(method)
+    private void resolveMethod(ElementResolver resolver, Supplier<JavaClasses> classes, TestMember<Method> method) {
+        resolver.resolveMethod(method.member)
                 .ifUnresolved(childResolver -> addChild(new ArchUnitMethodDescriptor(getUniqueId(), method, classes)));
     }
 
     private static void resolveChildren(
-            TestDescriptor parent, ElementResolver resolver, Field field, Supplier<JavaClasses> classes) {
+            TestDescriptor parent, ElementResolver resolver, TestMember<Field> field, Supplier<JavaClasses> classes) {
 
-        if (ArchTests.class.isAssignableFrom(field.getType())) {
+        if (ArchTests.class.isAssignableFrom(field.member.getType())) {
             resolveArchRules(parent, resolver, field, classes);
         } else {
             parent.addChild(new ArchUnitRuleDescriptor(resolver.getUniqueId(), getValue(field), classes, field));
         }
     }
 
-    private static <T> T getValue(Field field) {
-        return getValueOrThrowException(field, field.getDeclaringClass(), ArchTestInitializationException::new);
+    private static <T> T getValue(TestMember<Field> field) {
+        return getValueOrThrowException(field.member, field.owner, ArchTestInitializationException::new);
     }
 
     private static void resolveArchRules(
-            TestDescriptor parent, ElementResolver resolver, Field field, Supplier<JavaClasses> classes) {
+            TestDescriptor parent, ElementResolver resolver, TestMember<Field> field, Supplier<JavaClasses> classes) {
 
         DeclaredArchTests archTests = getDeclaredArchTests(field);
 
@@ -130,7 +132,7 @@ private static void resolveArchRules(
                 });
     }
 
-    private static DeclaredArchTests getDeclaredArchTests(Field field) {
+    private static DeclaredArchTests getDeclaredArchTests(TestMember<Field> field) {
         return new DeclaredArchTests(getValue(field));
     }
 
@@ -148,8 +150,8 @@ private static class ArchUnitRuleDescriptor extends AbstractArchUnitTestDescript
         private final ArchRule rule;
         private final Supplier<JavaClasses> classes;
 
-        ArchUnitRuleDescriptor(UniqueId uniqueId, ArchRule rule, Supplier<JavaClasses> classes, Field field) {
-            super(uniqueId, determineDisplayName(field.getName()), FieldSource.from(field), field);
+        ArchUnitRuleDescriptor(UniqueId uniqueId, ArchRule rule, Supplier<JavaClasses> classes, TestMember<Field> field) {
+            super(uniqueId, determineDisplayName(field.getName()), FieldSource.from(field.member), field.member);
             this.rule = rule;
             this.classes = classes;
         }
@@ -167,17 +169,19 @@ public ArchUnitEngineExecutionContext execute(ArchUnitEngineExecutionContext con
     }
 
     private static class ArchUnitMethodDescriptor extends AbstractArchUnitTestDescriptor {
-        private final Method method;
+        private final TestMember<Method> method;
         private final Supplier<JavaClasses> classes;
 
-        ArchUnitMethodDescriptor(UniqueId uniqueId, Method method, Supplier<JavaClasses> classes) {
-            super(uniqueId.append("method", method.getName()),
-                    determineDisplayName(method.getName()), MethodSource.from(method), method);
-            validate(method);
+        ArchUnitMethodDescriptor(UniqueId uniqueId, TestMember<Method> method, Supplier<JavaClasses> classes) {
+            super(uniqueId.append("method", method.member.getName()),
+                    determineDisplayName(method.member.getName()),
+                    MethodSource.from(method.member),
+                    method.member);
+
+            validate(method.member);
 
             this.method = method;
             this.classes = classes;
-            this.method.setAccessible(true);
         }
 
         private void validate(Method method) {
@@ -194,7 +198,7 @@ public Type getType() {
 
         @Override
         public ArchUnitEngineExecutionContext execute(ArchUnitEngineExecutionContext context, DynamicTestExecutor dynamicTestExecutor) {
-            invokeMethod(method, method.getDeclaringClass(), classes.get());
+            invokeMethod(method.member, method.owner, classes.get());
             return context;
         }
     }
@@ -203,12 +207,12 @@ private static class ArchUnitArchTestsDescriptor extends AbstractArchUnitTestDes
         private final DeclaredArchTests archTests;
         private final Supplier<JavaClasses> classes;
 
-        ArchUnitArchTestsDescriptor(ElementResolver resolver, DeclaredArchTests archTests, Supplier<JavaClasses> classes, Field field) {
+        ArchUnitArchTestsDescriptor(ElementResolver resolver, DeclaredArchTests archTests, Supplier<JavaClasses> classes, TestMember<Field> field) {
 
             super(resolver.getUniqueId(),
                     archTests.getDisplayName(),
                     ClassSource.from(archTests.getDefinitionLocation()),
-                    field,
+                    field.member,
                     archTests.getDefinitionLocation());
             this.archTests = archTests;
             this.classes = classes;
@@ -217,12 +221,31 @@ private static class ArchUnitArchTestsDescriptor extends AbstractArchUnitTestDes
         @Override
         public void createChildren(ElementResolver resolver) {
             archTests.handleFields(field ->
-                    resolver.resolve(FIELD_SEGMENT_TYPE, field.getName(), childResolver ->
-                            resolveChildren(this, childResolver, field, classes)));
+                    resolver.resolve(
+                            FIELD_SEGMENT_TYPE,
+                            field.getName(),
+                            childResolver -> resolveChildren(field, childResolver)));
 
             archTests.handleMethods(method ->
-                    resolver.resolve(METHOD_SEGMENT_TYPE, method.getName(), childResolver ->
-                            addChild(new ArchUnitMethodDescriptor(getUniqueId(), method, classes))));
+                    resolver.resolve(
+                            METHOD_SEGMENT_TYPE,
+                            method.getName(),
+                            childResolver -> addChild(method)));
+        }
+
+        private void resolveChildren(Field field, ElementResolver childResolver) {
+            ArchUnitTestDescriptor.resolveChildren(
+                    this,
+                    childResolver,
+                    new TestMember<>(archTests.getDefinitionLocation(), field),
+                    classes);
+        }
+
+        private void addChild(Method method) {
+            addChild(new ArchUnitMethodDescriptor(
+                    getUniqueId(),
+                    new TestMember<>(archTests.getDefinitionLocation(), method),
+                    classes));
         }
 
         @Override
@@ -300,4 +323,18 @@ public boolean scanWholeClasspath() {
             return analyzeClasses.wholeClasspath();
         }
     }
+
+    private static class TestMember<MEMBER extends AccessibleObject & Member> {
+        final Class<?> owner;
+        final MEMBER member;
+
+        TestMember(Class<?> owner, MEMBER member) {
+            this.owner = owner;
+            this.member = member;
+        }
+
+        String getName() {
+            return member.getName();
+        }
+    }
 }

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

@@ -37,6 +37,9 @@
 import com.tngtech.archunit.junit.internal.testexamples.TestMethodWithMetaTags;
 import com.tngtech.archunit.junit.internal.testexamples.TestMethodWithTags;
 import com.tngtech.archunit.junit.internal.testexamples.UnwantedClass;
+import com.tngtech.archunit.junit.internal.testexamples.abstractbase.ArchTestWithAbstractBaseClassWithFieldRule;
+import com.tngtech.archunit.junit.internal.testexamples.abstractbase.ArchTestWithAbstractBaseClassWithMethodRule;
+import com.tngtech.archunit.junit.internal.testexamples.abstractbase.ArchTestWithLibraryWithAbstractBaseClass;
 import com.tngtech.archunit.junit.internal.testexamples.ignores.IgnoredClass;
 import com.tngtech.archunit.junit.internal.testexamples.ignores.IgnoredField;
 import com.tngtech.archunit.junit.internal.testexamples.ignores.IgnoredLibrary;
@@ -833,6 +836,17 @@ void a_simple_rule_field_with_violation() {
             testListener.verifyViolation(simpleRuleFieldTestId(engineId), UnwantedClass.CLASS_VIOLATING_RULES.getSimpleName());
         }
 
+        @Test
+        void instance_field_rule_in_abstract_base_class() {
+            simulateCachedClassesForTest(ArchTestWithAbstractBaseClassWithFieldRule.class, UnwantedClass.CLASS_SATISFYING_RULES);
+
+            EngineExecutionTestListener testListener = execute(engineId, ArchTestWithAbstractBaseClassWithFieldRule.class);
+
+            testListener.verifySuccessful(engineId
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithFieldRule.class.getName())
+                    .append(FIELD_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithFieldRule.INSTANCE_FIELD_NAME));
+        }
+
         @Test
         void a_simple_rule_method_without_violation() {
             simulateCachedClassesForTest(SimpleRuleMethod.class, UnwantedClass.CLASS_SATISFYING_RULES);
@@ -851,6 +865,17 @@ void a_simple_rule_method_with_violation() {
             testListener.verifyViolation(simpleRuleMethodTestId(engineId), UnwantedClass.CLASS_VIOLATING_RULES.getSimpleName());
         }
 
+        @Test
+        void instance_method_rule_in_abstract_base_class() {
+            simulateCachedClassesForTest(ArchTestWithAbstractBaseClassWithMethodRule.class, UnwantedClass.CLASS_SATISFYING_RULES);
+
+            EngineExecutionTestListener testListener = execute(engineId, ArchTestWithAbstractBaseClassWithMethodRule.class);
+
+            testListener.verifySuccessful(engineId
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithMethodRule.class.getName())
+                    .append(METHOD_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithMethodRule.INSTANCE_METHOD_NAME));
+        }
+
         @Test
         void rule_library_without_violation() {
             simulateCachedClassesForTest(SimpleRuleLibrary.class, UnwantedClass.CLASS_SATISFYING_RULES);
@@ -880,6 +905,26 @@ void private_instance_libraries() {
                     testListener.verifyViolation(testId, UnwantedClass.CLASS_VIOLATING_RULES.getSimpleName()));
         }
 
+        @Test
+        public void library_with_rules_in_abstract_base_class() {
+            simulateCachedClassesForTest(ArchTestWithLibraryWithAbstractBaseClass.class, UnwantedClass.CLASS_SATISFYING_RULES);
+
+            EngineExecutionTestListener testListener = execute(engineId, ArchTestWithLibraryWithAbstractBaseClass.class);
+
+            testListener.verifySuccessful(engineId
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithLibraryWithAbstractBaseClass.class.getName())
+                    .append(FIELD_SEGMENT_TYPE, ArchTestWithLibraryWithAbstractBaseClass.FIELD_RULE_LIBRARY_NAME)
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithFieldRule.class.getName())
+                    .append(FIELD_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithFieldRule.INSTANCE_FIELD_NAME)
+                    );
+            testListener.verifySuccessful(engineId
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithLibraryWithAbstractBaseClass.class.getName())
+                    .append(FIELD_SEGMENT_TYPE, ArchTestWithLibraryWithAbstractBaseClass.METHOD_RULE_LIBRARY_NAME)
+                    .append(CLASS_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithMethodRule.class.getName())
+                    .append(METHOD_SEGMENT_TYPE, ArchTestWithAbstractBaseClassWithMethodRule.INSTANCE_METHOD_NAME)
+                    );
+        }
+
         @Test
         void rule_by_unique_id_without_violation() {
             UniqueId fieldRuleInLibrary = simpleRulesInLibraryId(engineId)

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithFieldRule.java

@@ -0,0 +1,13 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.junit.internal.testexamples.RuleThatFails;
+import com.tngtech.archunit.junit.internal.testexamples.UnwantedClass;
+import com.tngtech.archunit.lang.ArchRule;
+
+public abstract class AbstractBaseClassWithFieldRule {
+    public static final String INSTANCE_FIELD_NAME = "abstractBaseClassInstanceField";
+
+    @ArchTest
+    ArchRule abstractBaseClassInstanceField = RuleThatFails.on(UnwantedClass.CLASS_VIOLATING_RULES);
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithLibraryWithAbstractBaseClass.java

@@ -0,0 +1,13 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.junit.ArchTests;
+
+public abstract class AbstractBaseClassWithLibraryWithAbstractBaseClass {
+    public static final String FIELD_RULE_LIBRARY_NAME = "fieldRules";
+    public static final String METHOD_RULE_LIBRARY_NAME = "methodRules";
+    @ArchTest
+    ArchTests fieldRules = ArchTests.in(ArchTestWithAbstractBaseClassWithFieldRule.class);
+    @ArchTest
+    ArchTests methodRules = ArchTests.in(ArchTestWithAbstractBaseClassWithMethodRule.class);
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithMethodRule.java

@@ -0,0 +1,15 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.junit.internal.testexamples.RuleThatFails;
+import com.tngtech.archunit.junit.internal.testexamples.UnwantedClass;
+
+public abstract class AbstractBaseClassWithMethodRule {
+    public static final String INSTANCE_METHOD_NAME = "abstractBaseClassInstanceMethod";
+
+    @ArchTest
+    void abstractBaseClassInstanceMethod(JavaClasses classes) {
+        RuleThatFails.on(UnwantedClass.CLASS_VIOLATING_RULES).check(classes);
+    }
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithFieldRule.java

@@ -0,0 +1,7 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.junit.AnalyzeClasses;
+
+@AnalyzeClasses(packages = "some.dummy.package")
+public class ArchTestWithAbstractBaseClassWithFieldRule extends AbstractBaseClassWithFieldRule {
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithMethodRule.java

@@ -0,0 +1,7 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.junit.AnalyzeClasses;
+
+@AnalyzeClasses(packages = "some.dummy.package")
+public class ArchTestWithAbstractBaseClassWithMethodRule extends AbstractBaseClassWithMethodRule {
+}

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithLibraryWithAbstractBaseClass.java

@@ -0,0 +1,7 @@
+package com.tngtech.archunit.junit.internal.testexamples.abstractbase;
+
+import com.tngtech.archunit.junit.AnalyzeClasses;
+
+@AnalyzeClasses(packages = "some.dummy.package")
+public class ArchTestWithLibraryWithAbstractBaseClass extends AbstractBaseClassWithLibraryWithAbstractBaseClass {
+}