remove configuration archRule.failOnEmptyShould=false for tests

It turned out to be a bad idea to override the default configuration for `archRule.failOnEmptyShould=false` for all tests, since that covered up the fact that the default configuration actually breaks `optionalLayers` for `LayeredArchitecture` and `OnionArchitecture`. We now also use the default configuration for tests and override it on an individual basis. Note that we should use the default config for `ArchitecturesTest`, but at the moment the tests would be broken. So we deactivate it for now and fix it together with making `failOnEmptyShould` configurable on a per-rule basis.

Note that this also showed some tests that were a little shady, i.e. tests that checked for `assertThatMembers(..).matchInAnyOrderMembersOf(..)`, but actually those members were an empty set, at least with certain JDK versions. For members declared in meta-annotated types, this actually also hits the annotation type annotated with the meta-annotation, because meta-annotated counts direct annotations as well. Since the behavior always was like this we adjust the test to match reality in this case.

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
(cherry picked from commit 904587d)

archunit/src/test/java/com/tngtech/archunit/lang/ArchRuleTest.java

@@ -10,7 +10,6 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.common.io.Files;
-import com.tngtech.archunit.ArchConfiguration;
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.domain.JavaClassesTest;
@@ -33,13 +32,12 @@
 import static com.tngtech.archunit.lang.Priority.HIGH;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.all;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
+import static com.tngtech.archunit.testutil.ArchConfigurationRule.FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME;
 import static com.tngtech.archunit.testutil.TestUtils.toUri;
-import static java.lang.Boolean.FALSE;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class ArchRuleTest {
-    private static final String FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME = "archRule.failOnEmptyShould";
 
     @Rule
     public final ExpectedException thrown = ExpectedException.none();
@@ -170,8 +168,6 @@ public void finish(ConditionEvents events) {
 
     @Test
     public void evaluation_fails_because_of_empty_set_of_classes_with_default_fail_on_empty_should() {
-        archConfigurationRule.removeProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME);
-
         thrown.expect(AssertionError.class);
         thrown.expectMessage("Rule failed to check any classes");
         thrown.expectMessage(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME);
@@ -181,8 +177,6 @@ public void evaluation_fails_because_of_empty_set_of_classes_with_default_fail_o
 
     @Test
     public void evaluation_fails_because_of_empty_set_of_classes_after_filter_with_default_fail_on_empty_should() {
-        archConfigurationRule.removeProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME);
-
         thrown.expect(AssertionError.class);
         thrown.expectMessage("Rule failed to check any classes");
         thrown.expectMessage(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME);
@@ -192,7 +186,7 @@ public void evaluation_fails_because_of_empty_set_of_classes_after_filter_with_d
 
     @Test
     public void evaluation_passes_on_empty_set_of_classes_with_deactivated_fail_on_empty_should() {
-        ArchConfiguration.get().setProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, FALSE.toString());
+        archConfigurationRule.setFailOnEmptyShould(false);
 
         classes().should(ALWAYS_BE_VALID).evaluate(importClasses());
     }

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassesThatTest.java

@@ -26,6 +26,7 @@
 import com.tngtech.archunit.core.domain.properties.HasType;
 import com.tngtech.archunit.lang.ArchCondition;
 import com.tngtech.archunit.lang.ConditionEvents;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import org.junit.Rule;
 import org.junit.Test;
@@ -56,6 +57,9 @@ public class GivenClassesThatTest {
     @Rule
     public final ExpectedException thrown = ExpectedException.none();
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule();
+
     @Test
     public void haveFullyQualifiedName() {
         List<JavaClass> classes = filterResultOf(classes().that().haveFullyQualifiedName(List.class.getName()))
@@ -474,6 +478,8 @@ public void implement_type() {
 
         assertThatType(getOnlyElement(classes)).matches(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         classes = filterResultOf(classes().that().implement(Set.class))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -511,6 +517,8 @@ public void implement_typeName() {
 
         assertThatType(getOnlyElement(classes)).matches(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         classes = filterResultOf(classes().that().implement(AbstractList.class.getName()))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -532,6 +540,8 @@ public void implement_predicate() {
 
         assertThatType(getOnlyElement(classes)).matches(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         classes = filterResultOf(classes().that().implement(classWithNameOf(AbstractList.class)))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -890,6 +900,8 @@ public void or_conjunction() {
      */
     @Test
     public void conjunctions_aggregate_in_sequence_without_special_precedence() {
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         List<JavaClass> classes = filterResultOf(
                 // (List OR String) AND Collection => empty
                 classes().that().haveSimpleName(List.class.getSimpleName())

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenCodeUnitsTest.java

@@ -17,9 +17,11 @@
 import com.tngtech.archunit.lang.EvaluationResult;
 import com.tngtech.archunit.lang.SimpleConditionEvent;
 import com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.DescribedRuleStart;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -45,6 +47,9 @@
 @RunWith(DataProviderRunner.class)
 public class GivenCodeUnitsTest {
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule();
+
     @Test
     public void complex_code_unit_syntax() {
         EvaluationResult result = codeUnits()
@@ -209,6 +214,8 @@ public static Object[][] restricted_return_type_rule_starts() {
     @Test
     @UseDataProvider("restricted_return_type_rule_starts")
     public void return_type_predicates(DescribedRuleStart ruleStart, Collection<String> expectedMembers) {
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         EvaluationResult result = ruleStart.should(everythingViolationPrintMemberName())
                 .evaluate(importClasses(ClassWithVariousMembers.class));
 

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersDeclaredInClassesThatTest.java

@@ -23,6 +23,7 @@
 import com.tngtech.archunit.core.domain.properties.HasName;
 import com.tngtech.archunit.core.domain.properties.HasType;
 import com.tngtech.archunit.lang.syntax.elements.GivenClassesThatTest.Evaluator;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import org.junit.Rule;
 import org.junit.Test;
@@ -51,6 +52,9 @@ public class GivenMembersDeclaredInClassesThatTest {
     @Rule
     public final ExpectedException thrown = ExpectedException.none();
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule();
+
     @Test
     public void haveFullyQualifiedName() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().haveFullyQualifiedName(List.class.getName()))
@@ -420,31 +424,31 @@ public void areMetaAnnotatedWith_type() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(SomeAnnotation.class))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class);
     }
 
     @Test
     public void areNotMetaAnnotatedWith_type() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(SomeAnnotation.class))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class);
     }
 
     @Test
     public void areMetaAnnotatedWith_typeName() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(SomeAnnotation.class.getName()))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class);
     }
 
     @Test
     public void areNotMetaAnnotatedWith_typeName() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(SomeAnnotation.class.getName()))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class);
     }
 
     @Test
@@ -453,7 +457,7 @@ public void areMetaAnnotatedWith_predicate() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(hasNamePredicate))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class);
     }
 
     @Test
@@ -462,7 +466,7 @@ public void areNotMetaAnnotatedWith_predicate() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(hasNamePredicate))
                 .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class);
     }
 
     @Test
@@ -472,6 +476,8 @@ public void implement_type() {
 
         assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         members = filterResultOf(members().that().areDeclaredInClassesThat().implement(Set.class))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -509,6 +515,8 @@ public void implement_typeName() {
 
         assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         members = filterResultOf(members().that().areDeclaredInClassesThat().implement(AbstractList.class.getName()))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -530,6 +538,8 @@ public void implement_predicate() {
 
         assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class);
 
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         members = filterResultOf(members().that().areDeclaredInClassesThat().implement(classWithNameOf(AbstractList.class)))
                 .on(ArrayList.class, List.class, Iterable.class);
 
@@ -690,15 +700,15 @@ public void areNotEnums_predicate() {
     @Test
     public void areAnnotations_predicate() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areAnnotations())
-                .on(Deprecated.class, Collection.class, SafeVarargs.class, Integer.class);
+                .on(SomeAnnotation.class, Collection.class, MetaAnnotatedAnnotation.class, Integer.class);
 
-        assertThatMembers(members).matchInAnyOrderMembersOf(Deprecated.class, SafeVarargs.class);
+        assertThatMembers(members).matchInAnyOrderMembersOf(SomeAnnotation.class, MetaAnnotatedAnnotation.class);
     }
 
     @Test
     public void areNotAnnotations_predicate() {
         List<JavaMember> members = filterResultOf(members().that().areDeclaredInClassesThat().areNotAnnotations())
-                .on(Deprecated.class, Collection.class, SafeVarargs.class, Integer.class);
+                .on(SomeAnnotation.class, Collection.class, MetaAnnotatedAnnotation.class, Integer.class);
 
         assertThatMembers(members).matchInAnyOrderMembersOf(Collection.class, Integer.class);
     }
@@ -899,6 +909,8 @@ public void or_conjunction() {
      */
     @Test
     public void conjunctions_aggregate_in_sequence_without_special_precedence() {
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         List<JavaMember> members = filterResultOf(
                 // (List OR String) AND Collection => empty
                 members().that().areDeclaredInClassesThat().haveSimpleName(List.class.getSimpleName())
@@ -929,11 +941,13 @@ static Evaluator<JavaMember> filterResultOf(GivenMembersConjunction<JavaMember>
 
     @Retention(RetentionPolicy.RUNTIME)
     private @interface SomeAnnotation {
+        String value() default "default";
     }
 
     @Retention(RetentionPolicy.RUNTIME)
     @SomeAnnotation
     private @interface MetaAnnotatedAnnotation {
+        Class<?> value() default Object.class;
     }
 
     private static class SimpleClass {

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java

@@ -20,9 +20,11 @@
 import com.tngtech.archunit.lang.ConditionEvents;
 import com.tngtech.archunit.lang.EvaluationResult;
 import com.tngtech.archunit.lang.SimpleConditionEvent;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -58,6 +60,9 @@
 @RunWith(DataProviderRunner.class)
 public class GivenMembersTest {
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule();
+
     @DataProvider
     public static Object[][] member_syntax_testcases() {
         return $$(
@@ -431,6 +436,8 @@ private static Object[][] annotatedWithDataPoints(
     @Test
     @UseDataProvider("restricted_property_rule_starts")
     public void property_predicates(DescribedRuleStart conjunction, Set<String> expectedMessages) {
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         EvaluationResult result = conjunction.should(everythingViolationPrintMemberName())
                 .evaluate(importClasses(ClassWithVariousMembers.class, A.class, B.class, C.class, MetaAnnotation.class));
 
@@ -676,7 +683,7 @@ static Set<String> allConstructorsExcept(String constructorDescription) {
     static final Set<String> ALL_MEMBER_DESCRIPTIONS =
             union(ALL_CODE_UNIT_DESCRIPTIONS, ALL_FIELD_DESCRIPTIONS);
 
-    @SuppressWarnings({"unused"})
+    @SuppressWarnings({"unused", "FieldCanBeLocal"})
     static class ClassWithVariousMembers {
         @A
         private String fieldA;

archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java

@@ -29,6 +29,7 @@
 import com.tngtech.archunit.library.testclasses.second.three.any.SecondThreeAnyClass;
 import com.tngtech.archunit.library.testclasses.some.pkg.SomePkgClass;
 import com.tngtech.archunit.library.testclasses.some.pkg.sub.SomePkgSubclass;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.DataProviders;
@@ -60,6 +61,9 @@ public class ArchitecturesTest {
     @Rule
     public final ExpectedException thrown = ExpectedException.none();
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule().setFailOnEmptyShould(false);
+
     @DataProvider
     public static Object[][] layeredArchitectureDefinitions() {
         return testForEach(

archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java

@@ -12,6 +12,8 @@
 import com.tngtech.archunit.lang.syntax.elements.GivenConjunction;
 import com.tngtech.archunit.library.ArchitecturesTest;
 import com.tngtech.archunit.library.dependencies.syntax.GivenSlices;
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
+import org.junit.Rule;
 import org.junit.Test;
 
 import static com.tngtech.archunit.lang.conditions.ArchPredicates.have;
@@ -21,6 +23,9 @@
 public class GivenSlicesTest {
     static final String TEST_CLASSES_PACKAGE = ArchitecturesTest.class.getPackage().getName() + ".testclasses";
 
+    @Rule
+    public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule();
+
     @Test
     public void chosen_slices() {
         Set<Slice> slices = getSlicesMatchedByFilter(
@@ -46,6 +51,8 @@ public void chosen_slices() {
 
     @Test
     public void restricting_slices_that_should_not_depend_on_each_other() {
+        archConfigurationRule.setFailOnEmptyShould(false);
+
         GivenSlices givenSlices = slices().matching("..testclasses.(*)..");
         JavaClasses classes = new ClassFileImporter().importPackages("com.tngtech.archunit.library.testclasses");
 
@@ -85,4 +92,4 @@ public void check(Slice item, ConditionEvents events) {
         }).evaluate(new ClassFileImporter().importPackages(TEST_CLASSES_PACKAGE));
         return matched;
     }
-}
+}