Skip to content

Commit

Permalink
Closes #612: fix Code smells, removes junit-vintag
Browse files Browse the repository at this point in the history
* increases test coverage
* fixes codesmells from Java17 upgrade
* adding more useful ArchUnitTests
* changes related to new ArchUnitTest
* removes junit-vintage dependency
  • Loading branch information
arolfes committed Nov 27, 2023
1 parent cbffdfc commit fd0b451
Show file tree
Hide file tree
Showing 28 changed files with 804 additions and 141 deletions.
7 changes: 7 additions & 0 deletions camunda-outbox-example-boot/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@
<artifactId>camunda-bpm-spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit</artifactId>
<version>${version.archunit}</version>
<scope>test</scope>
</dependency>

</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package pro.taskana.camunda.spring.boot;

import static com.tngtech.archunit.core.domain.JavaCall.Predicates.target;
import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith;
import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.nameMatching;
import static com.tngtech.archunit.lang.conditions.ArchPredicates.are;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.methods;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.core.importer.ImportOption.OnlyIncludeTests;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;
import org.junit.jupiter.api.TestTemplate;

class ArchitectureTest {

public static final String[] PACKAGE_NAMES = {"pro.taskana", "acceptance"};
private static final JavaClasses IMPORTED_CLASSES =
new ClassFileImporter().importPackages(PACKAGE_NAMES);

private static final JavaClasses IMPORTED_TEST_CLASSES =
new ClassFileImporter(List.of(new OnlyIncludeTests())).importPackages(PACKAGE_NAMES);

@Test
void testMethodNamesShouldMatchAccordingToOurGuidelines() {
methods()
.that(
are(
annotatedWith(Test.class)
.or(annotatedWith(TestFactory.class))
.or(annotatedWith(TestTemplate.class))))
.and()
.areNotDeclaredIn(ArchitectureTest.class)
.should()
.haveNameMatching("^should_[A-Z][^_]+(_(For|When)_[A-Z][^_]+)?$")
.check(IMPORTED_TEST_CLASSES);
}

@Test
void testClassesAndTestMethodsShouldBePackagePrivate() {
classes()
.that()
.haveSimpleNameStartingWith("Test")
.or()
.haveSimpleNameEndingWith("Test")
.should()
.bePackagePrivate()
.check(IMPORTED_TEST_CLASSES);
methods()
.that()
.areDeclaredInClassesThat()
.haveSimpleNameStartingWith("Test")
.or()
.areDeclaredInClassesThat()
.haveSimpleNameEndingWith("Test")
.should()
.bePackagePrivate()
.orShould()
.bePrivate()
.check(IMPORTED_TEST_CLASSES);
}

@Test
void noMethodsShouldUseThreadSleep() {
noClasses()
.should()
.callMethod(Thread.class, "sleep", long.class)
.orShould()
.callMethod(Thread.class, "sleep", long.class, int.class)
.check(IMPORTED_CLASSES);
}

@Test
void noMethodsShouldUsePrintln() {
noClasses()
.should()
.callMethodWhere(target(nameMatching("println")))
.check(IMPORTED_CLASSES);
}

@Test
void noImportsForOldJunitClasses() {
noClasses()
.should()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.Test")
.orShould()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.Assert")
.check(IMPORTED_TEST_CLASSES);
}

@Test
void noImportsForJunitAssertionsWeWantAssertJ() {
noClasses()
.should()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.jupiter.api.Assertions")
.check(IMPORTED_TEST_CLASSES);
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,47 @@
package pro.taskana.camunda.spring.boot;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import pro.taskana.adapter.camunda.outbox.rest.config.OutboxRestServiceConfig;
import pro.taskana.adapter.camunda.outbox.rest.controller.CamundaTaskEventsController;
import pro.taskana.adapter.camunda.parselistener.TaskanaParseListenerProcessEnginePlugin;

@SpringBootTest(classes = CamundaSpringBootExample.class, webEnvironment = RANDOM_PORT)
class CamundaSpringBootExampleIntegrationTest {

private final OutboxRestServiceConfig outboxRestServiceConfig;

private final CamundaTaskEventsController camundaTaskEventsController;

private final TaskanaParseListenerProcessEnginePlugin taskanaParseListenerProcessEnginePlugin;

CamundaSpringBootExampleIntegrationTest(
@Autowired(required = false) OutboxRestServiceConfig outboxRestServiceConfig,
@Autowired(required = false) CamundaTaskEventsController camundaTaskEventsController,
@Autowired(required = false)
TaskanaParseListenerProcessEnginePlugin taskanaParseListenerProcessEnginePlugin) {
this.outboxRestServiceConfig = outboxRestServiceConfig;
this.camundaTaskEventsController = camundaTaskEventsController;
this.taskanaParseListenerProcessEnginePlugin = taskanaParseListenerProcessEnginePlugin;
}

@Test
void should_AutowireOutboxRestServiceConfig_When_ApplicationIsStarting() {
assertThat(outboxRestServiceConfig).isNotNull();
}

@Test
void should_AutowireCamundaTaskEventsController_When_ApplicationIsStarting() {
assertThat(camundaTaskEventsController).isNotNull();
}

@Test
void module_context_can_be_started() {
// there is no test code here,
// because creating the configuration to run this method is the actual test
void should_AutowireTaskanaParseListenerProcessEnginePlugin_When_ApplicationIsStarting() {
assertThat(taskanaParseListenerProcessEnginePlugin).isNotNull();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
import org.junit.jupiter.api.Test;

/** Acceptance test for ReferencedTask setters and getters. */
public class ReferencedTaskTest {
class ReferencedTaskTest {

private final String theValue = "blablabla";
private ReferencedTask theTask;

public ReferencedTaskTest() {
ReferencedTaskTest() {
theTask = new ReferencedTask();
theTask.setAssignee("1");
theTask.setBusinessProcessId("2");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package pro.taskana.adapter.camunda.outbox.rest.service;

import static java.util.stream.Collectors.toList;

import jakarta.ws.rs.core.MultivaluedMap;
import java.io.IOException;
import java.sql.Connection;
Expand Down Expand Up @@ -42,7 +40,7 @@ public class CamundaTaskEventsService {
private static final String RETRIES = "retries";
private static final String TYPE = "type";

private static final List<String> ALLOWED_PARAMS = Stream.of(TYPE, RETRIES).collect(toList());
private static final List<String> ALLOWED_PARAMS = Stream.of(TYPE, RETRIES).toList();

private static final String OUTBOX_SCHEMA = OutboxRestConfiguration.getOutboxSchema();
private static final String SQL_GET_CREATE_EVENTS =
Expand Down Expand Up @@ -365,7 +363,7 @@ private void verifyNoInvalidParameters(MultivaluedMap<String, String> filterPara
List<String> invalidParams =
filterParams.keySet().stream()
.filter(key -> !ALLOWED_PARAMS.contains(key))
.collect(Collectors.toList());
.toList();

if (!invalidParams.isEmpty()) {
throw new InvalidArgumentException("Provided invalid request params: " + invalidParams);
Expand Down
6 changes: 6 additions & 0 deletions taskana-adapter-camunda-spring-boot-example/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit</artifactId>
<version>${version.archunit}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package pro.taskana.adapter;

import static com.tngtech.archunit.core.domain.JavaCall.Predicates.target;
import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith;
import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.nameMatching;
import static com.tngtech.archunit.lang.conditions.ArchPredicates.are;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.methods;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.core.importer.ImportOption.OnlyIncludeTests;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;
import org.junit.jupiter.api.TestTemplate;

class ArchitectureTest {

public static final String[] PACKAGE_NAMES = {"pro.taskana", "acceptance"};
private static final JavaClasses IMPORTED_CLASSES =
new ClassFileImporter().importPackages(PACKAGE_NAMES);

private static final JavaClasses IMPORTED_TEST_CLASSES =
new ClassFileImporter(List.of(new OnlyIncludeTests())).importPackages(PACKAGE_NAMES);

@Test
void testMethodNamesShouldMatchAccordingToOurGuidelines() {
methods()
.that(
are(
annotatedWith(Test.class)
.or(annotatedWith(TestFactory.class))
.or(annotatedWith(TestTemplate.class))))
.and()
.areNotDeclaredIn(ArchitectureTest.class)
.should()
.haveNameMatching("^should_[A-Z][^_]+(_(For|When)_[A-Z][^_]+)?$")
.check(IMPORTED_TEST_CLASSES);
}

@Test
void testClassesAndTestMethodsShouldBePackagePrivate() {
classes()
.that()
.haveSimpleNameStartingWith("Test")
.or()
.haveSimpleNameEndingWith("Test")
.should()
.bePackagePrivate()
.check(IMPORTED_TEST_CLASSES);
methods()
.that()
.areDeclaredInClassesThat()
.haveSimpleNameStartingWith("Test")
.or()
.areDeclaredInClassesThat()
.haveSimpleNameEndingWith("Test")
.should()
.bePackagePrivate()
.orShould()
.bePrivate()
.check(IMPORTED_TEST_CLASSES);
}

@Test
void noMethodsShouldUseThreadSleep() {
noClasses()
.should()
.callMethod(Thread.class, "sleep", long.class)
.orShould()
.callMethod(Thread.class, "sleep", long.class, int.class)
.check(IMPORTED_CLASSES);
}

@Test
void noMethodsShouldUsePrintln() {
noClasses()
.should()
.callMethodWhere(target(nameMatching("println")))
.check(IMPORTED_CLASSES);
}

@Test
void noImportsForOldJunitClasses() {
noClasses()
.should()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.Test")
.orShould()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.Assert")
.check(IMPORTED_TEST_CLASSES);
}

@Test
void noImportsForJunitAssertionsWeWantAssertJ() {
noClasses()
.should()
.dependOnClassesThat()
.haveFullyQualifiedName("org.junit.jupiter.api.Assertions")
.check(IMPORTED_TEST_CLASSES);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,41 @@
package pro.taskana.adapter;

import static org.assertj.core.api.Assertions.assertThat;

import java.lang.reflect.Field;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import pro.taskana.common.api.TaskanaEngine;
import pro.taskana.spi.routing.api.TaskRoutingProvider;
import pro.taskana.spi.routing.internal.TaskRoutingManager;
import pro.taskana.taskrouting.ExampleTaskRouter;

@SpringBootTest(classes = TaskanaAdapterApplication.class)
class TaskanaAdapterApplicationTest {

private final List<TaskRoutingProvider> taskRoutingProviders;

TaskanaAdapterApplicationTest(@Autowired TaskanaEngine taskanaEngine) throws Exception {
TaskRoutingManager taskRoutingManager =
(TaskRoutingManager) getValueFromPrivateField(taskanaEngine, "taskRoutingManager");
this.taskRoutingProviders =
(List<TaskRoutingProvider>)
getValueFromPrivateField(taskRoutingManager, "taskRoutingProviders");
}

@Test
void module_context_can_be_started_and_outbox_rest_events_are_fetched() {
// there is no test code here,
// because creating the configuration to run this method is the actual test
void should_AutowireExampleTaskRouter_When_ApplicationIsStarting() {
assertThat(taskRoutingProviders).isNotNull().hasSize(1);
assertThat(taskRoutingProviders.get(0)).isInstanceOf(ExampleTaskRouter.class);
}

private Object getValueFromPrivateField(Object obj, String fieldName)
throws NoSuchFieldException, IllegalAccessException {
Field nameField = obj.getClass().getDeclaredField(fieldName);
nameField.setAccessible(true);

return nameField.get(obj);
}
}
5 changes: 0 additions & 5 deletions taskana-adapter-camunda-spring-boot-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,6 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit</artifactId>
Expand Down
Loading

0 comments on commit fd0b451

Please sign in to comment.