From b52ce64d692cffbbbdfc5f737c74fe044fcda14b Mon Sep 17 00:00:00 2001 From: Manfred Hanke Date: Sun, 1 May 2022 10:51:42 +0200 Subject: [PATCH] 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 * 8188cdc68abd9bbedd87a9800d89e6a260e3a52d * 625c3d6fe42781a5f62c4d4ffe103d1376b338e4 * 59f8c4da95fb95a4be49e9aad2311b3af06edd26 which added the same feature for JUnit4 (Issue: #104). Signed-off-by: Manfred Hanke --- .../internal/ArchUnitTestDescriptor.java | 91 +++++++++++++------ .../internal/ArchUnitTestEngineTest.java | 45 +++++++++ .../AbstractBaseClassWithFieldRule.java | 13 +++ ...ClassWithLibraryWithAbstractBaseClass.java | 13 +++ .../AbstractBaseClassWithMethodRule.java | 15 +++ ...estWithAbstractBaseClassWithFieldRule.java | 7 ++ ...stWithAbstractBaseClassWithMethodRule.java | 7 ++ ...hTestWithLibraryWithAbstractBaseClass.java | 7 ++ 8 files changed, 171 insertions(+), 27 deletions(-) create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithFieldRule.java create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithLibraryWithAbstractBaseClass.java create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithMethodRule.java create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithFieldRule.java create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithMethodRule.java create mode 100644 archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithLibraryWithAbstractBaseClass.java diff --git a/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java b/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java index 95b19be7b1..2b818a0e1f 100644 --- a/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java +++ b/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 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 classes, Field field) { - resolver.resolveField(field) + private void resolveField(ElementResolver resolver, Supplier classes, TestMember field) { + resolver.resolveField(field.member) .ifUnresolved(childResolver -> resolveChildren(this, childResolver, field, classes)); } - private void resolveMethod(ElementResolver resolver, Supplier classes, Method method) { - resolver.resolveMethod(method) + private void resolveMethod(ElementResolver resolver, Supplier classes, TestMember method) { + resolver.resolveMethod(method.member) .ifUnresolved(childResolver -> addChild(new ArchUnitMethodDescriptor(getUniqueId(), method, classes))); } private static void resolveChildren( - TestDescriptor parent, ElementResolver resolver, Field field, Supplier classes) { + TestDescriptor parent, ElementResolver resolver, TestMember field, Supplier 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 getValue(Field field) { - return getValueOrThrowException(field, field.getDeclaringClass(), ArchTestInitializationException::new); + private static T getValue(TestMember field) { + return getValueOrThrowException(field.member, field.owner, ArchTestInitializationException::new); } private static void resolveArchRules( - TestDescriptor parent, ElementResolver resolver, Field field, Supplier classes) { + TestDescriptor parent, ElementResolver resolver, TestMember field, Supplier 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) { return new DeclaredArchTests(getValue(field)); } @@ -148,8 +150,8 @@ private static class ArchUnitRuleDescriptor extends AbstractArchUnitTestDescript private final ArchRule rule; private final Supplier classes; - ArchUnitRuleDescriptor(UniqueId uniqueId, ArchRule rule, Supplier classes, Field field) { - super(uniqueId, determineDisplayName(field.getName()), FieldSource.from(field), field); + ArchUnitRuleDescriptor(UniqueId uniqueId, ArchRule rule, Supplier classes, TestMember 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; private final Supplier classes; - ArchUnitMethodDescriptor(UniqueId uniqueId, Method method, Supplier classes) { - super(uniqueId.append("method", method.getName()), - determineDisplayName(method.getName()), MethodSource.from(method), method); - validate(method); + ArchUnitMethodDescriptor(UniqueId uniqueId, TestMember method, Supplier 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 classes; - ArchUnitArchTestsDescriptor(ElementResolver resolver, DeclaredArchTests archTests, Supplier classes, Field field) { + ArchUnitArchTestsDescriptor(ElementResolver resolver, DeclaredArchTests archTests, Supplier classes, TestMember 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 { + final Class owner; + final MEMBER member; + + TestMember(Class owner, MEMBER member) { + this.owner = owner; + this.member = member; + } + + String getName() { + return member.getName(); + } + } } diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java index 0fe06a69c7..14153aad69 100644 --- a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java +++ b/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) diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithFieldRule.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithFieldRule.java new file mode 100644 index 0000000000..b9e4b0e24b --- /dev/null +++ b/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); +} diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithLibraryWithAbstractBaseClass.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithLibraryWithAbstractBaseClass.java new file mode 100644 index 0000000000..4fb08ceeb9 --- /dev/null +++ b/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); +} diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithMethodRule.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/AbstractBaseClassWithMethodRule.java new file mode 100644 index 0000000000..b0d03366b4 --- /dev/null +++ b/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); + } +} diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithFieldRule.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithFieldRule.java new file mode 100644 index 0000000000..d7f16af74b --- /dev/null +++ b/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 { +} diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithMethodRule.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithAbstractBaseClassWithMethodRule.java new file mode 100644 index 0000000000..766b57288e --- /dev/null +++ b/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 { +} diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithLibraryWithAbstractBaseClass.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/abstractbase/ArchTestWithLibraryWithAbstractBaseClass.java new file mode 100644 index 0000000000..87beab24e1 --- /dev/null +++ b/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 { +}