Skip to content

Commit

Permalink
fix @ArchIgnore not being fully respected by JUnit 4 support (#1277)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@codecholeric
codecholeric authored Apr 9, 2024
2 parents c644d0c + ce6a69d commit e680f88
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 15 deletions.
13 changes: 4 additions & 9 deletions ...t-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.Set;

Expand Down Expand Up @@ -56,17 +55,13 @@ private static ArchRuleDeclaration<Field> from(Class<?> testClass, Field field,
return new AsField(testClass, field, fieldOwner, forceIgnore);
}

static <T extends AnnotatedElement & Member> boolean elementShouldBeIgnored(T member) {
return elementShouldBeIgnored(member.getDeclaringClass(), member);
}

static boolean elementShouldBeIgnored(Class<?> testClass, AnnotatedElement ruleDeclaration) {
return testClass.getAnnotation(ArchIgnore.class) != null ||
static boolean elementShouldBeIgnored(Class<?> owner, AnnotatedElement ruleDeclaration) {
return owner.getAnnotation(ArchIgnore.class) != null ||
ruleDeclaration.getAnnotation(ArchIgnore.class) != null;
}

boolean shouldBeIgnored() {
return forceIgnore || elementShouldBeIgnored(testClass, declaration);
return forceIgnore || elementShouldBeIgnored(owner, declaration);
}

static Set<ArchRuleDeclaration<?>> toDeclarations(
Expand All @@ -87,7 +82,7 @@ private static Set<ArchRuleDeclaration<?>> archRuleDeclarationsFrom(Class<?> tes
Class<? extends Annotation> 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));
}

Expand Down
4 changes: 2 additions & 2 deletions ...unit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private Collection<ArchTestExecution> findArchRuleFields() {
}

private Set<ArchTestExecution> 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);
}
Expand All @@ -114,7 +114,7 @@ private ArchTests getArchRules(Field field) {
private Collection<ArchTestExecution> findArchRuleMethods() {
List<ArchTestExecution> 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;
Expand Down
31 changes: 31 additions & 0 deletions ...nit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsMethodsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
ArchUnitRunnerInternal runner = newRunnerFor(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);
Expand Down Expand Up @@ -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";
Expand Down
30 changes: 30 additions & 0 deletions ...4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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";
Expand Down
81 changes: 77 additions & 4 deletions ...it4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleSetsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand All @@ -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";

Expand All @@ -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";
Expand Down

0 comments on commit e680f88

Please sign in to comment.