From 66b3a66b5466a296ddbaba98b97b02e07d459e77 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Tue, 26 Mar 2024 13:34:51 +0100 Subject: [PATCH] fix `@ArchIgnore` not being fully respected by JUnit 4 support In the case of inheritance we were looking at the wrong class for an `@ArchIgnore`. I.e. rule declarations in base classes would only be ignored if the base class was annotated with `@ArchIgnore`, not the actual class. We now check the actual class for the annotation instead. If the base class is annotated with `@ArchIgnore` we ignore it for now, since this is a corner case and one could argue for both, either ignoring all subclasses or not ignoring subclasses. Signed-off-by: Peter Gafert --- .../junit/internal/ArchRuleDeclaration.java | 8 +- .../internal/ArchUnitRunnerInternal.java | 4 +- .../ArchUnitRunnerRunsMethodsTest.java | 31 +++++++ .../ArchUnitRunnerRunsRuleFieldsTest.java | 30 +++++++ .../ArchUnitRunnerRunsRuleSetsTest.java | 81 ++++++++++++++++++- 5 files changed, 142 insertions(+), 12 deletions(-) diff --git a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java index 0a9e700cba..6a19bb3b89 100644 --- a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java +++ b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java @@ -56,17 +56,13 @@ private static ArchRuleDeclaration from(Class testClass, Field field, return new AsField(testClass, field, fieldOwner, forceIgnore); } - static boolean elementShouldBeIgnored(T member) { - return elementShouldBeIgnored(member.getDeclaringClass(), member); - } - static boolean elementShouldBeIgnored(Class testClass, AnnotatedElement ruleDeclaration) { return testClass.getAnnotation(ArchIgnore.class) != null || ruleDeclaration.getAnnotation(ArchIgnore.class) != null; } boolean shouldBeIgnored() { - return forceIgnore || elementShouldBeIgnored(testClass, declaration); + return forceIgnore || elementShouldBeIgnored(owner, declaration); } static Set> toDeclarations( @@ -87,7 +83,7 @@ private static Set> archRuleDeclarationsFrom(Class tes Class archTestAnnotationType, boolean forceIgnore) { return ArchTests.class.isAssignableFrom(field.getType()) ? - toDeclarations(getArchRulesIn(field, fieldOwner), testClass, archTestAnnotationType, forceIgnore || elementShouldBeIgnored(field)) : + toDeclarations(getArchRulesIn(field, fieldOwner), testClass, archTestAnnotationType, forceIgnore || elementShouldBeIgnored(fieldOwner, field)) : singleton(ArchRuleDeclaration.from(testClass, field, fieldOwner, forceIgnore)); } diff --git a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java index 975da12046..a68a2ab788 100644 --- a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java +++ b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java @@ -92,7 +92,7 @@ private Collection findArchRuleFields() { } private Set findArchRulesIn(FrameworkField ruleField) { - boolean ignore = elementShouldBeIgnored(ruleField.getField()); + boolean ignore = elementShouldBeIgnored(getTestClass().getJavaClass(), ruleField.getField()); if (ruleField.getType() == ArchTests.class) { return asTestExecutions(getArchRules(ruleField.getField()), ignore); } @@ -114,7 +114,7 @@ private ArchTests getArchRules(Field field) { private Collection findArchRuleMethods() { List result = new ArrayList<>(); for (FrameworkMethod testMethod : getTestClass().getAnnotatedMethods(ArchTest.class)) { - boolean ignore = elementShouldBeIgnored(testMethod.getMethod()); + boolean ignore = elementShouldBeIgnored(getTestClass().getJavaClass(), testMethod.getMethod()); result.add(new ArchTestMethodExecution(getTestClass().getJavaClass(), testMethod.getMethod(), ignore)); } return result; diff --git a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsMethodsTest.java b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsMethodsTest.java index bffe20dd52..c6ed0de062 100644 --- a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsMethodsTest.java +++ b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsMethodsTest.java @@ -31,6 +31,8 @@ import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsMethodsTest.ArchTestWithTestMethod.testSomething; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsMethodsTest.IgnoredArchTest.toBeIgnoredOne; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsMethodsTest.IgnoredArchTest.toBeIgnoredTwo; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsMethodsTest.IgnoredArchTestWithBaseClass.toBeIgnoredOneInBaseClass; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsMethodsTest.IgnoredArchTestWithBaseClass.toBeIgnoredTwoInBaseClass; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerTestUtils.BE_SATISFIED; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerTestUtils.getRule; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerTestUtils.newRunnerFor; @@ -119,6 +121,18 @@ public void ignores_all_methods_in_classes_annotated_with_ArchIgnore() throws In .contains(toBeIgnoredTwo); } + @Test + public void ignores_all_methods_in_base_classes_of_classes_annotated_with_ArchIgnore() throws InitializationError { + ArchUnitRunnerInternal runner = new ArchUnitRunnerInternal(IgnoredArchTestWithBaseClass.class); + + runner.runChild(getRule(toBeIgnoredOneInBaseClass, runner), runNotifier); + runner.runChild(getRule(toBeIgnoredTwoInBaseClass, runner), runNotifier); + verify(runNotifier, times(2)).fireTestIgnored(descriptionCaptor.capture()); + assertThat(descriptionCaptor.getAllValues()).extractingResultOf("getMethodName") + .contains(toBeIgnoredOneInBaseClass) + .contains(toBeIgnoredTwoInBaseClass); + } + @Test public void ignores_methods_annotated_with_ArchIgnore() throws InitializationError { ArchUnitRunnerInternal runner = new ArchUnitRunnerInternal(ArchTestWithIgnoredMethod.class); @@ -239,6 +253,23 @@ public static void toBeIgnoredTwo(JavaClasses classes) { } } + @ArchIgnore + @AnalyzeClasses(packages = "some.pkg") + public static class IgnoredArchTestWithBaseClass extends BaseClass { + static final String toBeIgnoredOneInBaseClass = "toBeIgnoredOne"; + static final String toBeIgnoredTwoInBaseClass = "toBeIgnoredTwo"; + } + + public static class BaseClass { + @ArchTest + public static void toBeIgnoredOne(JavaClasses classes) { + } + + @ArchTest + public static void toBeIgnoredTwo(JavaClasses classes) { + } + } + @AnalyzeClasses(packages = "some.pkg") public static class ArchTestWithIgnoredMethod { static final String toBeIgnored = "toBeIgnored"; diff --git a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java index cba3c560e0..494ef8b176 100644 --- a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java +++ b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java @@ -27,6 +27,8 @@ import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.ArchTestWithPrivateInstanceField.PRIVATE_RULE_FIELD_NAME; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.IgnoredArchTest.RULE_ONE_IN_IGNORED_TEST; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.IgnoredArchTest.RULE_TWO_IN_IGNORED_TEST; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.IgnoredArchTestWithBaseClass.RULE_ONE_IN_IGNORED_BASE_CLASS; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.IgnoredArchTestWithBaseClass.RULE_TWO_IN_IGNORED_BASE_CLASS; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.SomeArchTest.FAILING_FIELD_NAME; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.SomeArchTest.IGNORED_FIELD_NAME; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleFieldsTest.SomeArchTest.SATISFIED_FIELD_NAME; @@ -163,6 +165,19 @@ public void should_skip_ignored_test() { .contains(RULE_ONE_IN_IGNORED_TEST, RULE_TWO_IN_IGNORED_TEST); } + @Test + public void should_skip_ignored_test_with_rule_in_base_class() { + ArchUnitRunnerInternal runner = newRunnerFor(IgnoredArchTestWithBaseClass.class, cache); + + runner.runChild(ArchUnitRunnerTestUtils.getRule(RULE_ONE_IN_IGNORED_BASE_CLASS, runner), runNotifier); + runner.runChild(ArchUnitRunnerTestUtils.getRule(RULE_TWO_IN_IGNORED_BASE_CLASS, runner), runNotifier); + + verify(runNotifier, times(2)).fireTestIgnored(descriptionCaptor.capture()); + + assertThat(descriptionCaptor.getAllValues()).extractingResultOf("getMethodName") + .contains(RULE_ONE_IN_IGNORED_BASE_CLASS, RULE_TWO_IN_IGNORED_BASE_CLASS); + } + @Test public void should_pass_annotations_of_rule_field() { ArchUnitRunnerInternal runner = newRunnerFor(ArchTestWithFieldWithAdditionalAnnotation.class, cache); @@ -275,6 +290,21 @@ public static class IgnoredArchTest { public static final ArchRule someRuleTwo = classes().should(NEVER_BE_SATISFIED); } + @ArchIgnore + @AnalyzeClasses(packages = "some.pkg") + public static class IgnoredArchTestWithBaseClass extends BaseClass { + static final String RULE_ONE_IN_IGNORED_BASE_CLASS = "someRuleOne"; + static final String RULE_TWO_IN_IGNORED_BASE_CLASS = "someRuleTwo"; + } + + public static abstract class BaseClass { + @ArchTest + public static final ArchRule someRuleOne = classes().should(NEVER_BE_SATISFIED); + + @ArchTest + public static final ArchRule someRuleTwo = classes().should(NEVER_BE_SATISFIED); + } + @AnalyzeClasses(packages = "some.pkg") public static class ArchTestWithFieldWithAdditionalAnnotation { static final String TEST_FIELD_NAME = "annotatedTestField"; diff --git a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleSetsTest.java b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleSetsTest.java index 2c4f4a3466..8457e9ed78 100644 --- a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleSetsTest.java +++ b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleSetsTest.java @@ -29,12 +29,17 @@ import static com.google.common.base.Preconditions.checkState; import static com.tngtech.archunit.core.domain.TestUtils.importClassesWithContext; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.ArchTestWithRuleLibrary.someOtherMethodRuleName; -import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredRules.someIgnoredFieldRuleName; -import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredRules.someIgnoredMethodRuleName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.FullyIgnoredRules.someFieldRuleInBaseClassOfIgnoredClassName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.FullyIgnoredRules.someFieldRuleInIgnoredClassName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.FullyIgnoredRules.someMethodRuleInBaseClassOfIgnoredClassName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.FullyIgnoredRules.someMethodRuleInIgnoredClassName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredSubRules.someIgnoredSubFieldRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredSubRules.someIgnoredSubMethodRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredSubRules.someNonIgnoredSubFieldRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.IgnoredSubRules.someNonIgnoredSubMethodRuleName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.OtherRules.someOtherFieldRuleName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.PartiallyIgnoredRules.someIgnoredFieldRuleName; +import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.PartiallyIgnoredRules.someIgnoredMethodRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.Rules.someFieldRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerRunsRuleSetsTest.Rules.someMethodRuleName; import static com.tngtech.archunit.junit.internal.ArchUnitRunnerTestUtils.getRule; @@ -74,6 +79,9 @@ public class ArchUnitRunnerRunsRuleSetsTest { @InjectMocks private ArchUnitRunnerInternal runnerForIgnoredRuleLibrary = newRunnerFor(ArchTestWithIgnoredRuleLibrary.class); + @InjectMocks + private ArchUnitRunnerInternal runnerForFullyIgnoredRuleLibrary = newRunnerFor(ArchTestWithFullyIgnoredRuleLibrary.class); + private final JavaClasses cachedClasses = importClassesWithContext(ArchUnitRunnerRunsRuleSetsTest.class); @Before @@ -137,6 +145,31 @@ public void ignores_nested_method_rule() { run(someIgnoredMethodRuleName, runnerForIgnoredRuleLibrary, verifyTestIgnored()); } + @Test + public void ignores_nested_field_rule_of_ignored_class() { + run(someFieldRuleInIgnoredClassName, runnerForFullyIgnoredRuleLibrary, verifyTestIgnored()); + } + + @Test + public void ignores_nested_method_rule_of_ignored_class() { + run(someMethodRuleInIgnoredClassName, runnerForFullyIgnoredRuleLibrary, verifyTestIgnored()); + } + + @Test + public void ignores_nested_field_rule_in_base_class_of_ignored_class() { + run(someFieldRuleInBaseClassOfIgnoredClassName, runnerForFullyIgnoredRuleLibrary, verifyTestIgnored()); + } + + @Test + public void ignores_nested_method_rule_in_base_class_of_ignored_class() { + run(someMethodRuleInBaseClassOfIgnoredClassName, runnerForFullyIgnoredRuleLibrary, verifyTestIgnored()); + } + + @Test + public void ignores_nested_rule_set_in_base_class_of_ignored_class() { + run(someOtherFieldRuleName, runnerForFullyIgnoredRuleLibrary, verifyTestIgnored()); + } + @Test public void ignores_double_nested_field_rule() { run(someIgnoredSubFieldRuleName, runnerForIgnoredRuleLibrary, verifyTestIgnored()); @@ -248,7 +281,13 @@ public static class ArchTestWithIgnoredRuleSet { @AnalyzeClasses(packages = "some.pkg") public static class ArchTestWithIgnoredRuleLibrary { @ArchTest - public static final ArchTests rules = ArchTests.in(IgnoredRules.class); + public static final ArchTests rules = ArchTests.in(PartiallyIgnoredRules.class); + } + + @AnalyzeClasses(packages = "some.pkg") + public static class ArchTestWithFullyIgnoredRuleLibrary { + @ArchTest + public static final ArchTests rules = ArchTests.in(FullyIgnoredRules.class); } public static class Rules { @@ -263,7 +302,7 @@ public static void someMethodRule(JavaClasses classes) { } } - public static class IgnoredRules { + public static class PartiallyIgnoredRules { static final String someIgnoredFieldRuleName = "someIgnoredFieldRule"; static final String someIgnoredMethodRuleName = "someIgnoredMethodRule"; @@ -284,6 +323,40 @@ public static void someIgnoredMethodRule(JavaClasses classes) { public static final ArchTests ignoredSubRules = ArchTests.in(Rules.class); } + @ArchIgnore + public static class FullyIgnoredRules extends BaseClass { + static final String someFieldRuleInIgnoredClassName = "someFieldRuleInIgnoredClass"; + static final String someMethodRuleInIgnoredClassName = "someMethodRuleInIgnoredClass"; + static final String someFieldRuleInBaseClassOfIgnoredClassName = "someFieldRuleInBaseClass"; + static final String someMethodRuleInBaseClassOfIgnoredClassName = "someMethodRuleInBaseClass"; + + @ArchTest + public static final ArchRule someFieldRuleInIgnoredClass = classes().should(satisfySomething()); + + @ArchTest + public static void someMethodRuleInIgnoredClass(JavaClasses classes) { + } + } + + public static class BaseClass { + @ArchTest + public static final ArchRule someFieldRuleInBaseClass = classes().should(satisfySomething()); + + @ArchTest + public static void someMethodRuleInBaseClass(JavaClasses classes) { + } + + @ArchTest + public static final ArchTests someSubRulesInBaseClass = ArchTests.in(OtherRules.class); + } + + public static class OtherRules { + static final String someOtherFieldRuleName = "someOtherFieldRule"; + + @ArchTest + public static final ArchRule someOtherFieldRule = classes().should(satisfySomething()); + } + public static class IgnoredSubRules { static final String someIgnoredSubFieldRuleName = "someIgnoredSubFieldRule"; static final String someNonIgnoredSubFieldRuleName = "someNonIgnoredSubFieldRule";