ArchUnit icon indicating copy to clipboard operation
ArchUnit copied to clipboard

Unable to use Mixin pattern because interface instances cannot be created

Open minborg opened this issue 6 years ago • 3 comments

Sometimes it is useful to be able to compose ArchUnit tests from interfaces with default implementations. This is a pattern that works for JUnit for example.

So I have an "trait" interface like this:

public interface OpenClassMethodsShouldNotThrowInternalClasses {

    @ArchTest
    default void openClassMethodsShouldNotThrowInternalClasses(JavaClasses classes) {
        OpenClassesRuleUtil.openClassMethodsShouldNotThrowInternalClasses(classes);
    }

}

Then I can just elect to implement this rule in applicable module tests. I also have a default set like this:

public interface DefaultArchitectureRuleMixin extends
    UtilityClassRuleMixin,
    OpenClassesShouldNotImplementInternalClasses,
    OpenClassMethodsShouldNotReturnInternalClasses,
    OpenClassMethodsShouldNotThrowInternalClasses
{ }

And then module tests can implement this and get all checks or just pick individual depending on the situation.

However, ArchUnit cannot create interfaces:

com.tngtech.archunit.base.ArchUnitException$ReflectionException: java.lang.NoSuchMethodException: com.speedment.common.archtest.trait.OpenClassMethodsShouldNotThrowInternalClasses.<init>()

	at com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(ReflectionUtils.java:30)
	at com.tngtech.archunit.junit.ReflectionUtils.newInstanceOf(ReflectionUtils.java:81)
	at com.tngtech.archunit.junit.ReflectionUtils.invokeMethod(ReflectionUtils.java:110)
	at com.tngtech.archunit.junit.ArchUnitTestDescriptor$ArchUnitMethodDescriptor.execute(ArchUnitTestDescriptor.java:191)
	at com.tngtech.archunit.junit.ArchUnitTestDescriptor$ArchUnitMethodDescriptor.execute(ArchUnitTestDescriptor.java:164)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:229)
	at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$6(DefaultLauncher.java:197)
	at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:211)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:191)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:128)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:69)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.lang.NoSuchMethodException: com.speedment.common.archtest.trait.OpenClassMethodsShouldNotThrowInternalClasses.<init>()
	at java.lang.Class.getConstructor0(Class.java:3082)
	at java.lang.Class.getDeclaredConstructor(Class.java:2178)
	at com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(ReflectionUtils.java:26)
	... 44 more

minborg avatar Aug 29 '19 15:08 minborg

Honestly, the way ArchUnit implements composability at the moment is not via traits, but via ArchRules. You could pretty much achieve the same thing by

public class OpenClassMethodsShouldNotThrowInternalClasses {
    @ArchTest
    static void openClassMethodsShouldNotThrowInternalClasses(JavaClasses classes) {
        OpenClassesRuleUtil.openClassMethodsShouldNotThrowInternalClasses(classes);
    }
}

public class DefaultArchitectureRules {
    @ArchTest
    static final ArchRules openClassMethodsShouldNotThrowInternalClasses = 
        ArchRules.in(OpenClassMethodsShouldNotThrowInternalClasses.class);
}

@AnalyzeClasses(..)
public class MyArchitectureTest {
    @ArchTest
    static final ArchRules default_rules = ArchRules.in(DefaultArchitectureRules.class);
}

Even though I think composing the rules as fields would make this even easier, to just pick individual rules in individual contexts i.e.

public class OpenClassesRules {
    @ArchTest
    public static final openClassMethodsShouldNotThrowInternalClasses = ...

    @ArchTest
    public static final openClassMethodsShouldNotReturnInternalClasses = ...
}

public class DefaultArchitectureRules {
    @ArchTest
    static final ArchRule pickSomeRule = OpenClassesRules.openClassMethodsShouldNotThrowInternalClasses;
}

But only if you even need that, for me it seems like you always want to test OpenClassesRules as unit anyway, so one class declaring those rules and imported into DefaultArchitectureRules would seem reasonable in that case.

I think this might be related to #104 though? I just didn't get around to implement this, because it isn't high priority for me (since I consider composition as layed out here good enough). I'm open for a PR though :wink:

codecholeric avatar Sep 07 '19 12:09 codecholeric

Thanks for your reply. I ended up having a facade ArchitectureRules with methods that take an enum. The facade then executes a set of Arch rules applicable for that enum:

public final class ArchitectureRules {

    private ArchitectureRules(){};

    public static void check(JavaClasses classes, Collection<RuleType> set) {
        ArchitectureRulesUtil.rules(set).forEach(c -> c.accept(classes));
    }

    public static void check(JavaClasses classes, RuleType... set) {
         check(classes, Stream.of(set).collect(Collectors.toList()));
    }

}
public enum RuleType {
    /**
     * Represents rules that relate to non-instantiable classes (i.e. "Util" classes).
     */
    UTIL,
    /**
     * Represents rules that involves classes that are <em>internal</em> and not a part
     * of the public API.
     */
    INTERNAL;
}

As a maintainer of an open-source project (speedment), I fully understand that features need to be prioritized and I agree that this issue might be of less importance relative to others.

minborg avatar Sep 09 '19 07:09 minborg

This is how it looks like in a module doing the actual tests (doing all the available tests):

@AnalyzeClasses(packages = "com.speedment.common.function", importOptions = {DoNotIncludeTests.class})
final class ArchitectureTest {

    @ArchTest
    void all(JavaClasses classes) {
        ArchitectureRules.check(classes, RuleType.values());
    }

}

minborg avatar Sep 09 '19 07:09 minborg