add support for reusable architectural tests

[monperrus]

Spoon is great for writing architectural tests, see http://spoon.gforge.inria.fr/architecture_test.html

Some tests we write could actually be useful for lots of Java projects, such as the recent one that checks for unused fields.

We could package the reusable architectural tests in some way so that we can share them with the world.

List of already implemented reusable architectural tests:

• all classes are documented with a documentation of certain length
• all methods are documented with a documentation of certain length
• don't use TreeSet without a comparator
• all tests follow the surefire convention
• all public methods of the API are directly called in a test
• helper classes only have static methods and a private constructor
• allowed packages are specified
• all fields are read
• all private methods are used

[monperrus]

Architectural checks is the most exciting topic, because the other 3 are "solved" to some degree. Do you have some ideas for checks spoons is missing?

All tests that I thought of are already implemented :-) For identifying new ones, I would recommend to ask practitioners. I can introduce you to relevant persons if you'd like to proceed this way.

[MartinWitt]

It would be a pleasure to proceed this way and gain more knowledge in this field.

[MartinWitt]

After reading @vmassol mail, I would describe the current state as following: Current state:

• Checkstyle doesn't provide easy check creation for complex tests.
• Sonarqube is too complex for projects even at xwiki size.
• Checks must run and fail the CI like unit tests.

So there is some need for a better solution.

Xwiki Checks

After reading the the xwiki tests, I tried to formulate them as architectural rule. The rules follow a common pattern. Lets first look at the xwiki architectural checks:

Requirement/Rule:

Verify that @Unstable annotation don't stay too long (and that a @since annotation is present too)

=> Each element that has a @Unstable Annotation also has a @since annotation. => Each @since annotation isn't too old.

Requirement/Rule:

Verify that Script Services are not located in the internal package.

=> Each class(?) that is script service, doesn't have a package name containing internal.

Requirement/Rule:

Verify that @since javadoc tags have the correct format (and that they don't contain several versions per tag).

=> Each @since annotation has only 1 value, and the value is conform to format.

Requirement/Rule:

Verify none of the listed methods are called in our Java code

=> For every method invocation, the method name is never .../ is not part of a list.

Requirement/Rule:

Verify that JUnit4 and JUnit5 APIs are not both used at the same time in a test

=> For every method, holds there is no method/field from junit 4 AND junit 5.

Requirement/Rule:

Verify that JUnit tests don't output content to stdout or stderr.

=> For every method, holds there is no reference to system.out/system.err

Generalization

We see a base rule:

Base rule:

∀ astElement FilterRule(a) => condition(a)

FilterRule

Element instanceof $WantedType AND ....

e.g. Every private field...

Condition:

e.g. is called

The filter rule is a selector for elements. Elements could be filtered by

• properties e.g. private/public
• there type e.g element instanceOf CtField
• there inputFolder src/test These filter are more or less already implemented in spoon and only need a nice package for easy use. The conditions have broad spectrum of possibility's. Here we have could define default methods like checkInvocation, field access etc.

After this short analysis I propose the following structure.

Framework structure

Every architecture check should consist of 2 methods. A filter and condition method, comparable to a junit test case. For filter and condition the framework provides out of the box implementations and allows defining own implementations. Here is the goal to provide an easy API for common checks but allow defining more complex checks.

       |Spoon|
          ↑
  |Architectural Checks|
          ↑
  |XWiki-Architectural Checks|

XWiki Test cases

Shall we write a framework for xwiki only? I would say no, but we should use xwiki as a partner to gather the user needs. A good result would be that a xwiki developer could easily define new check without knowledge about java-asts and every check is readable for a normal user.

Junit Dependency

The architectural checks shouldn't be junit cases by default, but allow them. If we simply throw exceptions a user can wrap them in a junit case or provide use the provided runner. I personally hate mixing unit tests and architectural test cases. Both have different concerns and should fail independently. Unit tests show verifies the code, architectural checks keep the code quality up.

Spoon integration

We should not deliver the framework with spoon-core, but as a module. This keeps spoon modular and is done for cases like spoon-maven-plugin. I'm unsure where to place the code because there are some possibilities

• folder in spoon repo
• repo in SpoonLabs
• repo somewhere(where?) But this is only a minor issue we can solve later and start with a repository anywhere and move it later.

Next steps

• Create an API draft
• Write common filters, conditions.
• Create a checkrunner e.g annotation based @ArchitectureCheck
• Create the xiki checks and existing spoon checks with the new framework
• Provide the xwiki-checks with a test runner to xwiki.

Wdyt about this plan? Any comments?

[monperrus]

Thanks for the wrap-up.

Shall we write a framework for xwiki only?

No, we extract from xwiki what's reusable in other projects.

I'm unsure where to place the code because there are some possibilities

A folder in spoon is perfect, as the other modules

[vmassol]

Nice summary. Cool!

Note that we have plenty of other small checks in the maven enforcer plugins (you can find all the skip properties there: https://github.com/xwiki/xwiki-commons/blob/master/pom.xml#L121). For example: https://github.com/xwiki/xwiki-commons/blob/master/pom.xml#L1373

[MartinWitt]

As my current implementation is still heavy wip, here a small update. Currently checks following the style at the bottom look like the way to go.

• The annotation @Architecture is a simple marker for the checkrunner.
• Precondition filters elements like all methods, that are public.
• Constraint formulates the rule. A constraint consists of an IError and (currently varags) Predicate<T>. Using predicates allows easy combinates with already defined methods AND, OR, NEGATE. IError is the errorHandler for violations. It has only a printError(T element) method allowing all kinds of error handling. E.g. create xml report, throw exception(allows usage in junit), etc. A testcase checking that every public method names in the testModel starts with "test" looks like the following. Both parameter are injected by the runner.

  @Architecture
  public void methodNameStartsWithTest(CtModel srcModel, CtModel testModel) {
      Precondition<CtMethod<?>> pre = Precondition.of(DefaultElementFilter.METHODS.getFilter(), Visibility.PUBLIC);
      Constraint<CtNamedElement> con = Constraint.of((element) -> System.out.println(element), Naming.startsWith("test"));
      ArchitectureTest.of(pre, con).runCheck(testModel);
  }

  Edit: And here is a test case checking that every private method is invoked/used.

  @Architecture
  public void methodInvocationLookUp(CtModel srcModel, CtModel testModel) {
      Precondition<CtMethod<?>> pre = Precondition.of(DefaultElementFilter.METHODS.getFilter(), Visibility.PRIVATE);
      InvocationMatcher matcher = new InvocationMatcher(srcModel);
      Constraint<CtMethod<?>> con = Constraint.of((element) -> System.out.println("element has no invocation: " + element), (element) -> matcher.test(element));
      ArchitectureTest.of(pre, con).runCheck(srcModel);
  }

[monperrus]

Cool. What's the intention of @Architecture? Which driver acts upon it?

[MartinWitt]

We have a CheckRunner with a method to invoke all methods with this annotation. A comparable annotation is @Test. This allows to run all architecture tests with on method. It's the entry point of the api for a launch. A cli like Spring-Shell, a maven plugin and junit-test case can reuse this entry point.
The intention is to provide one entry point for running all architecture tests without any external component/dependency for invocation.

    public void runChecks(String srcPath, String testPath) {
        ModelBuilder builder = new ModelBuilder(srcPath, testPath);
        builder.getTestModel().getElements(new TypeFilter<CtMethod<?>>(CtMethod.class)).stream()
                .filter(v -> v.hasAnnotation(Architecture.class))
                .map(v -> v.getReference().getActualMethod())
                .filter(Objects::nonNull)
                .forEach(clazz -> invokeMethod(builder, clazz));
    }

[monperrus]

A cli like Spring-Shell, a maven plugin and junit-test case can reuse this entry point.

We have to think of the user experience. What would be the simplest way to activate the Spoon architectural checks in a project?

[MartinWitt]

Currently the simplest way is wrap the runner in a junit test. This enables integration like run, debug in most IDEs. The CheckerRunner currently looks in the test path for all methods annotated with @Architecture and invokes them. A starter solution looks like the following, we could even set both paths as default.

    @Test
    public void allChecksMustRun() {
        CheckerRunner runner = new CheckerRunner();
        runner.runChecks("./src/main/java", "./src/test/java");
    }

Edit: I try to get the current code state in a readable form today to enhance discussions

[monperrus]

we could even set both paths as default

yes, we want that :)

Plus some renaming:

    @Test
    public void allChecksMustRun() {
        new SpoonArchitecturalChecker().check();
    }

[monperrus]

FYI, about architectural tests in CI:

Architecture violations detection and visualization in the continuous integration pipeline (SPE 2021) https://onlinelibrary.wiley.com/doi/10.1002/spe.3004?af=R