Closes #311 - Bump Spring Boot 4#426
Conversation
…ot-4 # Conflicts: # kadai-adapter-camunda8-parent/kadai-adapter-camunda-8-system-connector/src/test/java/io/kadai/adapter/systemconnector/camunda/monitoring/Camunda8CompositeHealthIntTest.java
|
…ot-4 # Conflicts: # .github/workflows/continuous-integration.yml # kadai-adapter-camunda7-parent/kadai-adapter-camunda-system-connector/src/main/java/io/kadai/adapter/monitoring/Camunda7HealthIndicator.java # kadai-adapter-camunda7-parent/kadai-adapter-camunda-system-connector/src/main/java/io/kadai/adapter/monitoring/Camunda7OutboxHealthIndicator.java # kadai-adapter-camunda7-parent/kadai-adapter-camunda-system-connector/src/main/java/io/kadai/adapter/systemconnector/camunda/config/Camunda7SystemConnectorConfiguration.java # kadai-adapter-camunda7-parent/kadai-adapter-camunda-system-connector/src/test/java/io/kadai/adapter/monitoring/Camunda7HealthIndicatorTest.java # kadai-adapter-camunda7-parent/kadai-adapter-camunda-system-connector/src/test/java/io/kadai/adapter/monitoring/Camunda7OutboxHealthIndicatorTest.java # kadai-adapter-camunda8-parent/pom.xml
| .areNotDeclaredIn(ArchitectureTest.class) | ||
| .should() | ||
| .haveNameMatching("^should_[A-Z][^_]+(_(For|When)_[A-Z][^_]+)?$") | ||
| .allowEmptyShould(true) |
There was a problem hiding this comment.
Why do we allow this? As far as i know this bypasses test failures if the criteria in the that() clause evaluate to zero matching classes, right? Is that the case here or why do we need this?
There was a problem hiding this comment.
Yes. We need it because besides the arch-test there are no tests in the module (anymore).
CI failed before that change here: https://github.com/kadai-io/KadaiAdapter/actions/runs/25724530689/job/75533993183
Weird behavior from arch imo - they didn't use the well-known all-quantification semantics.
| private double doubleField; | ||
| private boolean booleanField; | ||
| private ProcessVariableTestObjectTwo processVariableTestObjectTwoField; | ||
| private List<ProcessVariableTestObjectTwo> processVariableTestObjectTwoField; |
There was a problem hiding this comment.
Why did you change this to a List?
There was a problem hiding this comment.
Because the helper class moved.
Previously the C7 spring-boot-test copy of ProcessVariableTestObject already used List<ProcessVariableTestObjectTwo> and the tests asserted that JSON array shape.
With Camunda now running in Docker, the helper classes need to come from kadai-adapter-camunda-listener-example and are staged into the Camunda container. I changed that copy to match the old kadai-adapter-camunda-spring-boot-test test fixture and keep the existing acceptance-test payload semantics.
| import org.springframework.boot.test.context.SpringBootTest; | ||
|
|
||
| @SpringBootTest(classes = Camunda7SpringBootExample.class, webEnvironment = RANDOM_PORT) | ||
| class Camunda7SpringBootExampleIntegrationTest { |
There was a problem hiding this comment.
Is this covered somewhere else now?
There was a problem hiding this comment.
It inherently is with the acceptance-tests for C7, isn't it?
| <artifactId>resteasy-servlet-spring-boot-starter</artifactId> | ||
| <version>${version.resteasy.spring.boot}</version> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-jersey</artifactId> |
There was a problem hiding this comment.
we change from resteasy to jersey? I remember jersey was creating issues some 5 years+ ago
There was a problem hiding this comment.
Jersey is only used for tests and examples so we're fine either way I guess.
| </goals> | ||
| </execution> | ||
| <execution> | ||
| <id>default-jar</id> |
There was a problem hiding this comment.
Yeah not pretty at all, but intentional.
The C7 test suite has to copy the current reactor-built listener jar into the Camunda BPM Run container’s userlib during process-test-resources, before Maven’s normal package jar exists.
Early jar execution makes that local artifact available for container startup.
| * This class is responsible for setting complex variables containing Objects on a camunda user | ||
| * task. | ||
| */ | ||
| public class ComplexProcessVariableSetter implements JavaDelegate { |
There was a problem hiding this comment.
all those classes you deleted aren't needed anymore?
There was a problem hiding this comment.
They've been moved to kadai-adapter-camunda-listener-example
| Map<String, Object> valueInfo = new HashMap<>(); | ||
| valueInfo.put("serializationDataFormat", "application/json"); | ||
| valueInfo.put("objectTypeName", processVariable.getValue().getClass()); | ||
| if (processVariable instanceof ObjectValue) { |
There was a problem hiding this comment.
this is thouroughly tested? Because it changes the core logic
There was a problem hiding this comment.
I reverted these changes.
They were relicts from experimentations.
Very good spot!
leantios
left a comment
There was a problem hiding this comment.
I looked at the C8 changes. Looks good generally. With C8.9, we have the possibility that the tests share a runtime, making the tests faster: https://docs.camunda.io/docs/apis-tools/testing/configuration/#shared-runtime. Should we integrate that here or in a different PR?
|
|
Looks good from my side! |


I basically did four things:
Definition of Done
In ReviewCloses #ISSUE_ID - PROBLEM/SOLUTION