Arch Unit Violations in the Test Suite

javahippie · February 21, 2025, 4:38pm

Hi,

sometimes when I run mvn clean test on the project I see ArchUnit violations like this:

[ERROR]   ArchRulesTest Architecture Violation [Priority: MEDIUM] - Rule 'classes that are interfaces and reside outside of packages ['..impl..'] should only depend on classes that reside outside of packages ['..impl..'], because Interfaces that don't reside in an `impl` package are considered public API.' was violated (36 times):
Interface <org.operaton.bpm.model.xml.type.reference.ElementReferenceCollectionBuilder> extends interface <org.operaton.bpm.model.xml.impl.ModelBuildOperation> in (ElementReferenceCollectionBuilder.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Assignment.getFrom()> has return type <org.operaton.bpm.model.bpmn.impl.instance.From> in (Assignment.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Assignment.getTo()> has return type <org.operaton.bpm.model.bpmn.impl.instance.To> in (Assignment.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Assignment.setFrom(org.operaton.bpm.model.bpmn.impl.instance.From)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.From> in (Assignment.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Assignment.setTo(org.operaton.bpm.model.bpmn.impl.instance.To)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.To> in (Assignment.java:0)
Method <org.operaton.bpm.model.bpmn.instance.CorrelationPropertyBinding.getDataPath()> has return type <org.operaton.bpm.model.bpmn.impl.instance.DataPath> in (CorrelationPropertyBinding.java:0)
Method <org.operaton.bpm.model.bpmn.instance.CorrelationPropertyBinding.setDataPath(org.operaton.bpm.model.bpmn.impl.instance.DataPath)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.DataPath> in (CorrelationPropertyBinding.java:0)
Method <org.operaton.bpm.model.bpmn.instance.CorrelationPropertyRetrievalExpression.getMessagePath()> has return type <org.operaton.bpm.model.bpmn.impl.instance.MessagePath> in (CorrelationPropertyRetrievalExpression.java:0)
Method <org.operaton.bpm.model.bpmn.instance.CorrelationPropertyRetrievalExpression.setMessagePath(org.operaton.bpm.model.bpmn.impl.instance.MessagePath)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.MessagePath> in (CorrelationPropertyRetrievalExpression.java:0)
Method <org.operaton.bpm.model.bpmn.instance.DataAssociation.setTransformation(org.operaton.bpm.model.bpmn.impl.instance.Transformation)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.Transformation> in (DataAssociation.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.getChildLaneSet()> has return type <org.operaton.bpm.model.bpmn.impl.instance.ChildLaneSet> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.getPartitionElement()> has return type <org.operaton.bpm.model.bpmn.impl.instance.PartitionElement> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.getPartitionElementChild()> has return type <org.operaton.bpm.model.bpmn.impl.instance.PartitionElement> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.setChildLaneSet(org.operaton.bpm.model.bpmn.impl.instance.ChildLaneSet)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.ChildLaneSet> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.setPartitionElement(org.operaton.bpm.model.bpmn.impl.instance.PartitionElement)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.PartitionElement> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Lane.setPartitionElementChild(org.operaton.bpm.model.bpmn.impl.instance.PartitionElement)> has parameter of type <org.operaton.bpm.model.bpmn.impl.instance.PartitionElement> in (Lane.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Relationship.getSources()> has generic return type <java.util.Collection<org.operaton.bpm.model.bpmn.impl.instance.Source>> with type argument depending on <org.operaton.bpm.model.bpmn.impl.instance.Source> in (Relationship.java:0)
Method <org.operaton.bpm.model.bpmn.instance.Relationship.getTargets()> has generic return type <java.util.Collection<org.operaton.bpm.model.bpmn.impl.instance.Target>> with type argument depending on <org.operaton.bpm.model.bpmn.impl.instance.Target> in (Relationship.java:0)
Method <org.operaton.bpm.model.xml.instance.DomElement.getChildElementsByType(org.operaton.bpm.model.xml.impl.ModelInstanceImpl, java.lang.Class)> has parameter of type <org.operaton.bpm.model.xml.impl.ModelInstanceImpl> in (DomElement.java:0)
Method <org.operaton.bpm.model.xml.type.ModelElementTypeBuilder$ModelTypeInstanceProvider.newInstance(org.operaton.bpm.model.xml.impl.instance.ModelTypeInstanceContext)> has parameter of type <org.operaton.bpm.model.xml.impl.instance.ModelTypeInstanceContext> in (ModelElementTypeBuilder.java:0)
Method <org.operaton.bpm.model.xml.type.reference.ElementReference.clearReferenceTargetElement(org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl)> has parameter of type <org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl> in (ElementReference.java:0)
Method <org.operaton.bpm.model.xml.type.reference.ElementReference.getReferenceTargetElement(org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl)> has parameter of type <org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl> in (ElementReference.java:0)
Method <org.operaton.bpm.model.xml.type.reference.ElementReference.setReferenceTargetElement(org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl, org.operaton.bpm.model.xml.instance.ModelElementInstance)> has parameter of type <org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl> in (ElementReference.java:0)
Method <org.operaton.bpm.model.xml.type.reference.ElementReferenceCollection.getReferenceTargetElements(org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl)> has parameter of type <org.operaton.bpm.model.xml.impl.instance.ModelElementInstanceImpl> in (ElementReferenceCollection.java:0)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.FileValueTypeImpl.<init>()> in (ValueType.java:67)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.ObjectTypeImpl.<init>()> in (ValueType.java:65)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$BooleanTypeImpl.<init>()> in (ValueType.java:47)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$BytesTypeImpl.<init>()> in (ValueType.java:61)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$DateTypeImpl.<init>()> in (ValueType.java:59)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$DoubleTypeImpl.<init>()> in (ValueType.java:53)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$IntegerTypeImpl.<init>()> in (ValueType.java:57)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$LongTypeImpl.<init>()> in (ValueType.java:51)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$NullTypeImpl.<init>()> in (ValueType.java:45)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$NumberTypeImpl.<init>()> in (ValueType.java:63)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$ShortTypeImpl.<init>()> in (ValueType.java:49)
Static Initializer <org.operaton.bpm.engine.variable.type.ValueType.<clinit>()> calls constructor <org.operaton.bpm.engine.variable.impl.type.PrimitiveValueTypeImpl$StringTypeImpl.<init>()> in (ValueType.java:55)
[ERROR]   ConnectionPersistenceExceptionTest.shouldFailWithConnectionError:76->provokePersistenceConnectionError:94 
Expecting actual not to be null
[INFO] 
[ERROR] Tests run: 15805, Failures: 2, Errors: 0, Skipped: 55

When reading the ArchUnit rules written in the ArchRules, they are right:

@ArchTest
 public static final ArchRule publicApiShouldNotExposeImplementation = classes().that()
      .areInterfaces().and().resideOutsideOfPackages("..impl..")
      .should().onlyDependOnClassesThat().resideOutsideOfPackages("..impl..")
      .because("Interfaces that don't reside in an `impl` package are considered public API.");

One example is this interface:

package org.operaton.bpm.model.xml.type.reference;

import org.operaton.bpm.model.xml.impl.ModelBuildOperation;
import org.operaton.bpm.model.xml.instance.ModelElementInstance;

/**
 * @author Sebastian Menski
 */
public interface ElementReferenceCollectionBuilder<Target extends ModelElementInstance, Source extends ModelElementInstance> extends ModelBuildOperation {

  ElementReferenceCollection<Target, Source> build();

}

The interface itself is not in an impl package but inherits from one in an impl package.

I’m really not sure what to do here. One one hand I’m not sure why this test is not failing every single time, on the other hand it seems pretty useless to have a test which describes rules which are obviously violated in the codebase.

WDYT? Should we remove / disable those tests?

kthoms · February 21, 2025, 8:19pm

We can’t do much at the moment to change the violations. ArchUnit allows to declare exclusions from the rules. I think it is in general good to establish the rules and document by exclusions that there is work to be done for the first major release after 1.x.
What concerns me is that you experience these violations sometimes, and I can’t recall to have seen them so far.
So from my point of view we can do:

make sure that the tests are executed reliable
define exclusion rules for known/detected violations
optional: define a Maven profile which needs to be activated to execute these tests
TBD: document on classes that violate these constraints that they are planned to be moved
Maybe deprecate violating classes, move the content to a class at the right place and make the violating class an empty deprecated subclass