Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use Mixin pattern because interface instances cannot be created #223

Open
minborg opened this issue Aug 29, 2019 · 3 comments
Open

Comments

@minborg
Copy link

minborg commented Aug 29, 2019

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

@codecholeric
Copy link
Collaborator

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 😉

@minborg
Copy link
Author

minborg commented Sep 9, 2019

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
Copy link
Author

minborg commented Sep 9, 2019

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());
    }

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants