Skip to content

Commit

Permalink
make tests included via ArchTests report outer class as test locati…
Browse files Browse the repository at this point in the history 
…on (#1279)

So far, if we include rules from other test classes, e.g.
```
@AnalyzeClasses(..)
class ExampleTest {
  @archtest
  static final ArchTests nested = ArchTests.in(Other.class);
}
```
then the nested class (`Other.class` in this case) will be used as test
location.
This can cause confusion, because by this if `Other.class` is included
in two separate test classes,
analyzing different locations via `@AnalyzeClasses`,
the results will be reported for the same test class `Other.class`.
We now report the outermost class that started the test (the one with
`@AnalyzeClasses)
as the test location for both JUnit 4 and JUnit 5.
To make it easy to understand through which path a rule is included in
more complex scenarios,
we extend the display name to include the path of nested classes.

Resolves: #452
  • Loading branch information
@codecholeric
codecholeric authored Apr 9, 2024
2 parents e680f88 + e658be0 commit 72c2f80
Show file tree
Hide file tree
Showing 11 changed files with 218 additions and 90 deletions.
47 changes: 25 additions & 22 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 @@ -19,8 +19,10 @@
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Set;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.tngtech.archunit.junit.ArchIgnore;
import com.tngtech.archunit.junit.ArchTests;
Expand All @@ -33,26 +35,26 @@
import static java.util.Collections.singleton;

abstract class ArchRuleDeclaration<T extends AnnotatedElement> {
private final Class<?> testClass;
final List<Class<?>> testClassPath;
final T declaration;
final Class<?> owner;
private final boolean forceIgnore;

ArchRuleDeclaration(Class<?> testClass, T declaration, Class<?> owner, boolean forceIgnore) {
this.testClass = testClass;
ArchRuleDeclaration(List<Class<?>> testClassPath, T declaration, Class<?> owner, boolean forceIgnore) {
this.testClassPath = testClassPath;
this.declaration = declaration;
this.owner = owner;
this.forceIgnore = forceIgnore;
}

abstract void handleWith(Handler handler);

private static ArchRuleDeclaration<Method> from(Class<?> testClass, Method method, Class<?> methodOwner, boolean forceIgnore) {
return new AsMethod(testClass, method, methodOwner, forceIgnore);
private static ArchRuleDeclaration<Method> from(List<Class<?>> testClassPath, Method method, Class<?> methodOwner, boolean forceIgnore) {
return new AsMethod(testClassPath, method, methodOwner, forceIgnore);
}

private static ArchRuleDeclaration<Field> from(Class<?> testClass, Field field, Class<?> fieldOwner, boolean forceIgnore) {
return new AsField(testClass, field, fieldOwner, forceIgnore);
private static ArchRuleDeclaration<Field> from(List<Class<?>> testClassPath, Field field, Class<?> fieldOwner, boolean forceIgnore) {
return new AsField(testClassPath, field, fieldOwner, forceIgnore);
}

static boolean elementShouldBeIgnored(Class<?> owner, AnnotatedElement ruleDeclaration) {
Expand All @@ -65,57 +67,58 @@ boolean shouldBeIgnored() {
}

static Set<ArchRuleDeclaration<?>> toDeclarations(
ArchTests archTests, Class<?> testClass, Class<? extends Annotation> archTestAnnotationType, boolean forceIgnore) {
ArchTests archTests, List<Class<?>> testClassPath, Class<? extends Annotation> archTestAnnotationType, boolean forceIgnore) {

ImmutableSet.Builder<ArchRuleDeclaration<?>> result = ImmutableSet.builder();
Class<?> definitionLocation = archTests.getDefinitionLocation();
List<Class<?>> childTestClassPath = ImmutableList.<Class<?>>builder().addAll(testClassPath).add(definitionLocation).build();
for (Field field : getAllFields(definitionLocation, withAnnotation(archTestAnnotationType))) {
result.addAll(archRuleDeclarationsFrom(testClass, field, definitionLocation, archTestAnnotationType, forceIgnore));
result.addAll(archRuleDeclarationsFrom(childTestClassPath, field, definitionLocation, archTestAnnotationType, forceIgnore));
}
for (Method method : getAllMethods(definitionLocation, withAnnotation(archTestAnnotationType))) {
result.add(ArchRuleDeclaration.from(testClass, method, definitionLocation, forceIgnore));
result.add(ArchRuleDeclaration.from(childTestClassPath, method, definitionLocation, forceIgnore));
}
return result.build();
}

private static Set<ArchRuleDeclaration<?>> archRuleDeclarationsFrom(Class<?> testClass, Field field, Class<?> fieldOwner,
private static Set<ArchRuleDeclaration<?>> archRuleDeclarationsFrom(List<Class<?>> testClassPath, Field field, Class<?> fieldOwner,
Class<? extends Annotation> archTestAnnotationType, boolean forceIgnore) {

return ArchTests.class.isAssignableFrom(field.getType()) ?
toDeclarations(getArchRulesIn(field, fieldOwner), testClass, archTestAnnotationType, forceIgnore || elementShouldBeIgnored(fieldOwner, field)) :
singleton(ArchRuleDeclaration.from(testClass, field, fieldOwner, forceIgnore));
toDeclarations(getArchTestsIn(field, fieldOwner), testClassPath, archTestAnnotationType, forceIgnore || elementShouldBeIgnored(fieldOwner, field)) :
singleton(ArchRuleDeclaration.from(testClassPath, field, fieldOwner, forceIgnore));
}

private static ArchTests getArchRulesIn(Field field, Class<?> fieldOwner) {
private static ArchTests getArchTestsIn(Field field, Class<?> fieldOwner) {
ArchTests value = getValue(field, fieldOwner);
return checkNotNull(value, "Field %s.%s is not initialized", fieldOwner.getName(), field.getName());
}

private static class AsMethod extends ArchRuleDeclaration<Method> {
AsMethod(Class<?> testClass, Method method, Class<?> methodOwner, boolean forceIgnore) {
super(testClass, method, methodOwner, forceIgnore);
AsMethod(List<Class<?>> testClassPath, Method method, Class<?> methodOwner, boolean forceIgnore) {
super(testClassPath, method, methodOwner, forceIgnore);
}

@Override
void handleWith(Handler handler) {
handler.handleMethodDeclaration(declaration, owner, shouldBeIgnored());
handler.handleMethodDeclaration(testClassPath, declaration, owner, shouldBeIgnored());
}
}

private static class AsField extends ArchRuleDeclaration<Field> {
AsField(Class<?> testClass, Field field, Class<?> fieldOwner, boolean forceIgnore) {
super(testClass, field, fieldOwner, forceIgnore);
AsField(List<Class<?>> testClassPath, Field field, Class<?> fieldOwner, boolean forceIgnore) {
super(testClassPath, field, fieldOwner, forceIgnore);
}

@Override
void handleWith(Handler handler) {
handler.handleFieldDeclaration(declaration, owner, shouldBeIgnored());
handler.handleFieldDeclaration(testClassPath, declaration, owner, shouldBeIgnored());
}
}

interface Handler {
void handleFieldDeclaration(Field field, Class<?> fieldOwner, boolean ignore);
void handleFieldDeclaration(List<Class<?>> testClassPath, Field field, Class<?> fieldOwner, boolean ignore);

void handleMethodDeclaration(Method method, Class<?> methodOwner, boolean ignore);
void handleMethodDeclaration(List<Class<?>> testClassPath, Method method, Class<?> methodOwner, boolean ignore);
}
}
12 changes: 7 additions & 5 deletions ...nit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchRuleExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.tngtech.archunit.junit.internal;

import java.lang.reflect.Field;
import java.util.List;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.lang.ArchRule;
Expand All @@ -26,19 +27,19 @@
class ArchRuleExecution extends ArchTestExecution {
private final Field ruleField;

ArchRuleExecution(Class<?> testClass, Field ruleField, boolean ignore) {
super(testClass, ignore);
ArchRuleExecution(List<Class<?>> testClassPath, Class<?> ruleDeclaringClass, Field ruleField, boolean ignore) {
super(testClassPath, ruleDeclaringClass, ignore);

ArchTestInitializationException.check(ArchRule.class.isAssignableFrom(ruleField.getType()),
"Rule field %s.%s to check must be of type %s",
testClass.getSimpleName(), ruleField.getName(), ArchRule.class.getSimpleName());
ruleDeclaringClass.getSimpleName(), ruleField.getName(), ArchRule.class.getSimpleName());

this.ruleField = ruleField;
}

@Override
Result evaluateOn(JavaClasses classes) {
ArchRule rule = getValue(ruleField, testClass);
ArchRule rule = getValue(ruleField, ruleDeclaringClass);
try {
rule.check(classes);
} catch (Exception | AssertionError e) {
Expand All @@ -49,7 +50,8 @@ Result evaluateOn(JavaClasses classes) {

@Override
Description describeSelf() {
return Description.createTestDescription(testClass, determineDisplayName(ruleField.getName()), ruleField.getAnnotations());
String testName = formatWithPath(ruleField.getName());
return Description.createTestDescription(getTestClass(), determineDisplayName(testName), ruleField.getAnnotations());
}

@Override
Expand Down
26 changes: 23 additions & 3 deletions ...nit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchTestExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,25 @@
package com.tngtech.archunit.junit.internal;

import java.lang.reflect.Field;
import java.util.List;
import java.util.stream.Stream;

import com.tngtech.archunit.core.domain.JavaClasses;
import org.junit.runner.Description;
import org.junit.runner.notification.Failure;
import org.junit.runner.notification.RunNotifier;

import static com.tngtech.archunit.junit.internal.ReflectionUtils.getValueOrThrowException;
import static java.util.stream.Collectors.joining;

abstract class ArchTestExecution {
final Class<?> testClass;
final List<Class<?>> testClassPath;
final Class<?> ruleDeclaringClass;
private final boolean ignore;

ArchTestExecution(Class<?> testClass, boolean ignore) {
this.testClass = testClass;
ArchTestExecution(List<Class<?>> testClassPath, Class<?> ruleDeclaringClass, boolean ignore) {
this.testClassPath = testClassPath;
this.ruleDeclaringClass = ruleDeclaringClass;
this.ignore = ignore;
}

Expand All @@ -42,6 +47,21 @@ public String toString() {
return describeSelf().toString();
}

Class<?> getTestClass() {
return testClassPath.get(0);
}

String formatWithPath(String testName) {
if (testClassPath.size() <= 1) {
return testName;
}

return Stream.concat(
testClassPath.subList(1, testClassPath.size()).stream().map(Class::getSimpleName),
Stream.of(testName)
).collect(joining(" > "));
}

abstract String getName();

boolean ignore() {
Expand Down
10 changes: 6 additions & 4 deletions ...nit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchTestMethodExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.junit.ArchTest;
Expand All @@ -28,8 +29,8 @@
class ArchTestMethodExecution extends ArchTestExecution {
private final Method testMethod;

ArchTestMethodExecution(Class<?> testClass, Method testMethod, boolean ignore) {
super(testClass, ignore);
ArchTestMethodExecution(List<Class<?>> testClassPath, Class<?> ruleDeclaringClass, Method testMethod, boolean ignore) {
super(testClassPath, ruleDeclaringClass, ignore);
this.testMethod = testMethod;
}

Expand All @@ -49,12 +50,13 @@ private void executeTestMethod(JavaClasses classes) {
"Methods annotated with @%s must have exactly one parameter of type %s",
ArchTest.class.getSimpleName(), JavaClasses.class.getSimpleName());

invokeMethod(testMethod, testClass, classes);
invokeMethod(testMethod, ruleDeclaringClass, classes);
}

@Override
Description describeSelf() {
return Description.createTestDescription(testClass, determineDisplayName(testMethod.getName()), testMethod.getAnnotations());
String testName = formatWithPath(testMethod.getName());
return Description.createTestDescription(getTestClass(), determineDisplayName(testName), testMethod.getAnnotations());
}

@Override
Expand Down
19 changes: 10 additions & 9 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 @@ -43,6 +43,7 @@
import static com.tngtech.archunit.junit.internal.ArchRuleDeclaration.toDeclarations;
import static com.tngtech.archunit.junit.internal.ArchTestExecution.getValue;
import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toList;

final class ArchUnitRunnerInternal extends ParentRunner<ArchTestExecution> implements ArchUnitRunner.InternalRunner<ArchTestExecution> {
Expand Down Expand Up @@ -94,28 +95,28 @@ private Collection<ArchTestExecution> findArchRuleFields() {
private Set<ArchTestExecution> findArchRulesIn(FrameworkField ruleField) {
boolean ignore = elementShouldBeIgnored(getTestClass().getJavaClass(), ruleField.getField());
if (ruleField.getType() == ArchTests.class) {
return asTestExecutions(getArchRules(ruleField.getField()), ignore);
return asTestExecutions(getArchTests(ruleField.getField()), ignore);
}
return singleton(new ArchRuleExecution(getTestClass().getJavaClass(), ruleField.getField(), ignore));
return singleton(new ArchRuleExecution(singletonList(getTestClass().getJavaClass()), getTestClass().getJavaClass(), ruleField.getField(), ignore));
}

private Set<ArchTestExecution> asTestExecutions(ArchTests archTests, boolean forceIgnore) {
ExecutionTransformer executionTransformer = new ExecutionTransformer();
for (ArchRuleDeclaration<?> declaration : toDeclarations(archTests, getTestClass().getJavaClass(), ArchTest.class, forceIgnore)) {
for (ArchRuleDeclaration<?> declaration : toDeclarations(archTests, singletonList(getTestClass().getJavaClass()), ArchTest.class, forceIgnore)) {
declaration.handleWith(executionTransformer);
}
return executionTransformer.getExecutions();
}

private ArchTests getArchRules(Field field) {
private ArchTests getArchTests(Field field) {
return getValue(field, field.getDeclaringClass());
}

private Collection<ArchTestExecution> findArchRuleMethods() {
List<ArchTestExecution> result = new ArrayList<>();
for (FrameworkMethod testMethod : getTestClass().getAnnotatedMethods(ArchTest.class)) {
boolean ignore = elementShouldBeIgnored(getTestClass().getJavaClass(), testMethod.getMethod());
result.add(new ArchTestMethodExecution(getTestClass().getJavaClass(), testMethod.getMethod(), ignore));
result.add(new ArchTestMethodExecution(singletonList(getTestClass().getJavaClass()), getTestClass().getJavaClass(), testMethod.getMethod(), ignore));
}
return result;
}
Expand Down Expand Up @@ -154,13 +155,13 @@ private static class ExecutionTransformer implements ArchRuleDeclaration.Handler
private final ImmutableSet.Builder<ArchTestExecution> executions = ImmutableSet.builder();

@Override
public void handleFieldDeclaration(Field field, Class<?> fieldOwner, boolean ignore) {
executions.add(new ArchRuleExecution(fieldOwner, field, ignore));
public void handleFieldDeclaration(List<Class<?>> testClassPath, Field field, Class<?> fieldOwner, boolean ignore) {
executions.add(new ArchRuleExecution(testClassPath, fieldOwner, field, ignore));
}

@Override
public void handleMethodDeclaration(Method method, Class<?> methodOwner, boolean ignore) {
executions.add(new ArchTestMethodExecution(methodOwner, method, ignore));
public void handleMethodDeclaration(List<Class<?>> testClassPath, Method method, Class<?> methodOwner, boolean ignore) {
executions.add(new ArchTestMethodExecution(testClassPath, methodOwner, method, ignore));
}

Set<ArchTestExecution> getExecutions() {
Expand Down
9 changes: 6 additions & 3 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 @@ -223,12 +223,15 @@ private void verifyTestFinishedSuccessfully(String expectedDescriptionMethodName
verifyTestFinishedSuccessfully(runNotifier, descriptionCaptor, expectedDescriptionMethodName);
}

static void verifyTestFinishedSuccessfully(RunNotifier runNotifier, ArgumentCaptor<Description> descriptionCaptor,
String expectedDescriptionMethodName) {
static void verifyTestFinishedSuccessfully(
RunNotifier runNotifier,
ArgumentCaptor<Description> descriptionCaptor,
String expectedDescriptionMethodName
) {
verify(runNotifier, never()).fireTestFailure(any(Failure.class));
verify(runNotifier).fireTestFinished(descriptionCaptor.capture());
Description description = descriptionCaptor.getValue();
assertThat(description.getMethodName()).isEqualTo(expectedDescriptionMethodName);
assertThat(description.getMethodName()).endsWith(expectedDescriptionMethodName);
}

@AnalyzeClasses(packages = "some.pkg")
Expand Down
47 changes: 30 additions & 17 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 @@ -92,20 +92,40 @@ public void setUp() {

@Test
public void should_find_children_in_rule_set() {
assertThat(runnerForRuleSet.getChildren()).as("Rules defined in Test Class").hasSize(2);
assertThat(runnerForRuleSet.getChildren())
.extracting(resultOf("describeSelf"))
.extractingResultOf("getMethodName")
.as("Descriptions").containsOnly(someFieldRuleName, someMethodRuleName);
assertThat(runnerForRuleSet.getChildren()).as("Rules defined in Test Class")
.hasSize(2)
.map(ArchTestExecution::describeSelf)
.containsExactlyInAnyOrder(
Description.createTestDescription(
ArchTestWithRuleSet.class,
Rules.class.getSimpleName() + " > " + someFieldRuleName
),
Description.createTestDescription(
ArchTestWithRuleSet.class,
Rules.class.getSimpleName() + " > " + someMethodRuleName
)
);
}

@Test
public void should_find_children_in_rule_library() {
assertThat(runnerForRuleLibrary.getChildren()).as("Rules defined in Library").hasSize(3);
assertThat(runnerForRuleLibrary.getChildren())
.extracting(resultOf("describeSelf"))
.extractingResultOf("getMethodName")
.as("Descriptions").containsOnly(someFieldRuleName, someMethodRuleName, someOtherMethodRuleName);
assertThat(runnerForRuleLibrary.getChildren()).as("Rules defined in Library")
.hasSize(3)
.map(ArchTestExecution::describeSelf)
.containsExactlyInAnyOrder(
Description.createTestDescription(
ArchTestWithRuleLibrary.class,
ArchTestWithRuleSet.class.getSimpleName() + " > " + Rules.class.getSimpleName() + " > " + someFieldRuleName
),
Description.createTestDescription(
ArchTestWithRuleLibrary.class,
ArchTestWithRuleSet.class.getSimpleName() + " > " + Rules.class.getSimpleName() + " > " + someMethodRuleName
),
Description.createTestDescription(
ArchTestWithRuleLibrary.class.getName(),
someOtherMethodRuleName
)
);
}

@Test
Expand All @@ -118,13 +138,6 @@ public void can_run_rule_method() {
run(someMethodRuleName, runnerForRuleSet, verifyTestRan());
}

@Test
public void describes_nested_rules_within_their_declaring_class() {
for (ArchTestExecution execution : runnerForRuleSet.getChildren()) {
assertThat(execution.describeSelf().getTestClass()).isEqualTo(Rules.class);
}
}

@Test
public void ignores_field_rule_of_ignored_rule_set() {
run(someFieldRuleName, runnerForIgnoredRuleSet, verifyTestIgnored());
Expand Down
Loading

0 comments on commit 72c2f80

Please sign in to comment.