Add tests to checkstyle (#6006)

* Add tests to checkstyle * Add includede test dirs in execution * wip * wip * bump checkstyle deps * bump checkstyle deps * wip50 * wip * Handle upgrade * triple-wip * wip * Wip replacement of prints * All checkstyle now passing * wip * Reset checkstyle * spotless * reset checkstyle * Add checkstyle to pipeline * Tidy * Tidy * wip * Cut over to not empty for correct behaviour * imports
hapifhir · Jun 16, 2024 · 73a6a8e · 73a6a8e
1 parent c1c6d4f
commit 73a6a8e
Show file tree

Hide file tree

Showing 63 changed files with 653 additions and 662 deletions.
diff --git a/hapi-fhir-android/src/test/java/android/util/Log.java b/hapi-fhir-android/src/test/java/android/util/Log.java
@@ -1,5 +1,6 @@
 package android.util;
 
+@SuppressWarnings("checkstyle:NoPrintln")
 public class Log {
 
  public static final int VERBOSE = 2;

diff --git a/hapi-fhir-base/pom.xml b/hapi-fhir-base/pom.xml
@@ -208,7 +208,7 @@
  <version>1.8</version>
  <configuration>
  <target>
- <!--suppress UnresolvedMavenProperty -->
+<!-- suppress UnresolvedMavenProperty-->
  <delete dir="${pom.basedir}/target/" includes="checkstyle*"/>
  </target>
  </configuration>
@@ -223,40 +223,7 @@
  </execution>
  </executions>
  </plugin>
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-checkstyle-plugin</artifactId>
- <configuration>
- <!--suppress UnresolvedMavenProperty -->
- <configLocation>${maven.multiModuleProjectDirectory}/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle.xml</configLocation>
- <!--suppress UnresolvedMavenProperty -->
- <suppressionsLocation>${maven.multiModuleProjectDirectory}/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle-suppression.xml</suppressionsLocation>
- <inputEncoding>UTF-8</inputEncoding>
- <consoleOutput>true</consoleOutput>
- </configuration>
- <inherited>true</inherited>
- <executions>
- <execution>
- <id>hapi-single-module-checkstyle</id>
- <phase>validate</phase>
- <goals>
- <goal>check</goal>
- </goals>
- </execution>
- </executions>
- <dependencies>
- <dependency>
- <groupId>ca.uhn.hapi.fhir</groupId>
- <artifactId>hapi-fhir-checkstyle</artifactId>
- <version>${project.version}</version>
- </dependency>
- <dependency>
- <groupId>com.puppycrawl.tools</groupId>
- <artifactId>checkstyle</artifactId>
- <version>${checkstyle_version}</version>
- </dependency>
- </dependencies>
- </plugin>
+
  </plugins>
  <resources>
  <resource>

diff --git a/...model/api/BaseBatchJobParametersTest.java → ...hir/model/api/BatchJobParametersTest.java b/...model/api/BaseBatchJobParametersTest.java → ...hir/model/api/BatchJobParametersTest.java
@@ -13,7 +13,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
-public class BaseBatchJobParametersTest {
+public class BatchJobParametersTest {
 
  private static class TestParameters extends BaseBatchJobParameters {
 

diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/QualifiedParamListTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/QualifiedParamListTest.java
@@ -25,11 +25,10 @@ public void testSplit2() {
  assertEquals("aaa", actual.get(0));
  assertEquals("bbb", actual.get(1));
  }
- 
+
  @Test
  public void testSplit3() {
  List<String> actual = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(null,"aaa,b\\,bb");
- System.out.println(actual);
  assertThat(actual).hasSize(2);
  assertEquals("aaa", actual.get(0));
  assertEquals("b,bb", actual.get(1));

diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/MessageSupplierTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/MessageSupplierTest.java
@@ -44,16 +44,14 @@ public void before() {
  }
 
 
+
  @Test
  public void testLog() {
 
  ourLog.info("Hello: {}", msg(() -> "Goodbye"));
 
  verify(myMockAppender, times(1)).doAppend(argThat((ArgumentMatcher<ILoggingEvent>) argument -> {
  String formattedMessage = argument.getFormattedMessage();
- System.out.flush();
- System.out.println("** Got Message: " + formattedMessage);
- System.out.flush();
  return formattedMessage.equals("Hello: Goodbye");
  }));
 

diff --git a/hapi-fhir-checkstyle/pom.xml b/hapi-fhir-checkstyle/pom.xml
@@ -36,6 +36,7 @@
  </dependency>
  </dependencies>
 
+
  <build>
  <pluginManagement>
  <plugins>
@@ -59,7 +60,6 @@
  </plugins>
  </pluginManagement>
  </build>
-
  <profiles>
  <!-- For releases, we need to generate javadoc and sources JAR -->
  <profile>
@@ -102,7 +102,7 @@
  </build>
  </profile>
  <profile>
- <id>CI</id>
+ <id>CHECKSTYLE</id>
  <build>
  <plugins>
  <plugin>
@@ -115,6 +115,7 @@
  <!--suppress UnresolvedMavenProperty -->
  <directory>${maven.multiModuleProjectDirectory}/</directory>
  </sourceDirectories>
+ <includeTestSourceDirectory>true</includeTestSourceDirectory>
  <!--suppress UnresolvedMavenProperty -->
  <configLocation>${maven.multiModuleProjectDirectory}/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle.xml</configLocation>
  <!--suppress UnresolvedMavenProperty -->

diff --git a/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle-suppression.xml b/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle-suppression.xml
@@ -4,7 +4,6 @@
  "https://checkstyle.org/dtds/suppressions_1_2.dtd">
 
 <suppressions>
- <suppress files="[/\\]test[/\\]" checks="[a-zA-Z0-9]*" />
  <suppress files="[/\\]resources[/\\]" checks="[a-zA-Z0-9]*" />
  <suppress files="[\\/]target[\\/]" checks="[a-zA-Z0-9]*" />
  <suppress files="[\\/]hapi-fhir-docs[\\/]" checks="[a-zA-Z0-9]*" />
@@ -15,11 +14,18 @@
  <suppress files="[\\/]osgi[\\/]" checks="[a-zA-Z0-9]*"/>
 
  <!-- Name suppressions -->
+ <!-- We don't care if test files have `base` or abstract in the name` -->
+ <suppress files="[/\\]test[/\\]" checks="AbstractClassName" />
+ <suppress files="[/\\]test[/\\]" checks="ca.uhn.fhir.checks.HapiErrorCodeCheck" id="HapiErrorCodeUniqueness"/>
+
  <suppress files="Base64BinaryDt\.java" checks="AbstractClassName"/>
- <!-- TODO KHS cdr instantiates these. Remove them once cdr has been fixed. -->
+ <!-- TODO CHECKSTYLE KHS cdr instantiates these. Remove them once cdr has been fixed. -->
  <suppress files="BaseMigrationTasks\.java" checks="AbstractClassName"/>
  <suppress files="BaseLoincTop2000LabResultsHandler\.java" checks="AbstractClassName"/>
 
+ <!-- TODO GGG Checkstyle MDM tests can currently use hamcrest until we fix em -->
+ <suppress files=".*/mdm/.*" checks="RegexpSinglelineJava" id="NoHamcrestAssert"/>
+
  <!-- Missing "Base" prefix suppressions -->
  <suppress files="ResourceMetadataKeyEnum\.java" checks="AbstractClassName"/>
  <suppress files="RequestDetails\.java" checks="AbstractClassName"/>

diff --git a/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle.xml b/hapi-fhir-checkstyle/src/checkstyle/hapi-base-checkstyle.xml
@@ -4,14 +4,17 @@
  "http:https://www.puppycrawl.com/dtds/configuration_1_3.dtd">
 
 <module name="Checker">
-
+ <module name="SuppressWarningsFilter" />
  <property name="severity" value="error"/>
  <property name="charset" value="UTF-8"/>
  <property name="fileExtensions" value="java, properties, xml, js, json"/>
 
  <module name="TreeWalker">
+ <module name="SuppressWarningsHolder" />
  <!-- Run custom HapiErrorCodeCheck to find duplicate error codes -->
- <module name="ca.uhn.fhir.checks.HapiErrorCodeCheck"/>
+ <module name="ca.uhn.fhir.checks.HapiErrorCodeCheck">
+ <property name="id" value="HapiErrorCodeUniqueness"/>
+ </module>
  <!-- Throw error if any FIX ME is left in the code -->
  <module name="TodoComment">
  <!-- The (?i) below means Case Insensitive -->
@@ -26,10 +29,12 @@
  -->
 
  <module name="RegexpSinglelineJava">
+ <property name="id" value="NoPrintln"/>
  <property name="format" value="System\.out\.println"/>
  <property name="ignoreComments" value="true"/>
  </module>
  <module name="RegexpSinglelineJava">
+ <property name="id" value="NoHamcrestAssert"/>
  <property name="format" value="org.hamcrest.MatcherAssert.assertThat"/>
  <property name="message" value="Incorrect assertThat import used: The &quot;org.assertj.core.api.Assertions.assertThat&quot; import should be used for tests"/>
  </module>

diff --git a/hapi-fhir-checkstyle/src/test/java/ca/uhn/fhir/checks/HapiErrorCodeCheckTest.java b/hapi-fhir-checkstyle/src/test/java/ca/uhn/fhir/checks/HapiErrorCodeCheckTest.java
@@ -1,5 +1,6 @@
 package ca.uhn.fhir.checks;
 
+import com.puppycrawl.tools.checkstyle.AbstractAutomaticBean;
 import com.puppycrawl.tools.checkstyle.Checker;
 import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
 import com.puppycrawl.tools.checkstyle.DefaultLogger;
@@ -31,8 +32,8 @@ public void testExpectedErrors() throws CheckstyleException {
  files.add(getFile("BadClass.java"));
 
  ByteArrayOutputStream errors = new ByteArrayOutputStream();
- DefaultLogger listener = new DefaultLogger(NullOutputStream.NULL_OUTPUT_STREAM, AutomaticBean.OutputStreamOptions.CLOSE,
- errors, AutomaticBean.OutputStreamOptions.CLOSE);
+ DefaultLogger listener = new DefaultLogger(NullOutputStream.NULL_OUTPUT_STREAM, AbstractAutomaticBean.OutputStreamOptions.CLOSE,
+ errors, AbstractAutomaticBean.OutputStreamOptions.CLOSE);
  checker.addListener(listener);
 
  // execute

diff --git a/...c/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu2Hl7OrgTest.java b/...c/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu2Hl7OrgTest.java
@@ -22,6 +22,8 @@
 import static org.mockito.Mockito.when;
 
 public class AbstractJaxRsConformanceProviderDstu2Hl7OrgTest {
+ private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(AbstractJaxRsConformanceProviderDstu2Hl7OrgTest.class);
+
 
  private static final String BASEURI = "http:https://basiuri";
  private static final String REQUESTURI = BASEURI + "/metadata";
@@ -49,15 +51,15 @@ public void testConformance() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu2Hl7Org.class, new TestJaxRsDummyPatientProviderDstu2Hl7Org());
  Response response = createConformanceProvider(providers).conformance();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
  public void testConformanceUsingOptions() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu2Hl7Org.class, new TestJaxRsDummyPatientProviderDstu2Hl7Org());
  Response response = createConformanceProvider(providers).conformanceUsingOptions();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
@@ -68,8 +70,8 @@ public void testConformanceWithMethods() throws Exception {
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
  assertThat(response.getEntity().toString()).contains("\"type\": \"Patient\"");
  assertThat(response.getEntity().toString()).contains("someCustomOperation");
- System.out.println(response);
- System.out.println(response.getEntity());
+ ourLog.info(response.toString());
+ ourLog.info(response.getEntity().toString());
  }
 
  @Test
@@ -79,10 +81,10 @@ public void testConformanceInXml() throws Exception {
  providers.put(TestJaxRsMockPatientRestProviderDstu2Hl7Org.class, new TestJaxRsMockPatientRestProviderDstu2Hl7Org());
  Response response = createConformanceProvider(providers).conformance();
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  assertThat(response.getEntity().toString()).contains(" <type value=\"Patient\"/>");
  assertThat(response.getEntity().toString()).contains("someCustomOperation");
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  }
 
  private AbstractJaxRsConformanceProvider createConformanceProvider(final ConcurrentHashMap<Class<? extends IResourceProvider>, IResourceProvider> providers)

diff --git a/...e/src/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu2_1Test.java b/...e/src/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu2_1Test.java
@@ -23,6 +23,8 @@
 import static org.mockito.Mockito.when;
 
 public class AbstractJaxRsConformanceProviderDstu2_1Test {
+ private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(AbstractJaxRsConformanceProviderDstu2_1Test.class);
+
 
  private static final String BASEURI = "http:https://basiuri";
  private static final String REQUESTURI = BASEURI + "/metadata";
@@ -50,15 +52,15 @@ public void testConformance() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu2_1.class, new TestJaxRsDummyPatientProviderDstu2_1());
  Response response = createConformanceProvider(providers).conformance();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
  public void testConformanceUsingOptions() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu2_1.class, new TestJaxRsDummyPatientProviderDstu2_1());
  Response response = createConformanceProvider(providers).conformanceUsingOptions();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
@@ -69,8 +71,8 @@ public void testConformanceWithMethods() throws Exception {
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
  assertThat(response.getEntity().toString()).contains("\"type\": \"Patient\"");
  assertThat(response.getEntity().toString()).contains("\"someCustomOperation");
- System.out.println(response);
- System.out.println(response.getEntity());
+ ourLog.info(response.toString());
+ ourLog.info(response.getEntity().toString());
  }
 
  @Test
@@ -80,10 +82,10 @@ public void testConformanceInXml() throws Exception {
  providers.put(TestJaxRsMockPatientRestProviderDstu2_1.class, new TestJaxRsMockPatientRestProviderDstu2_1());
  Response response = createConformanceProvider(providers).conformance();
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  assertThat(response.getEntity().toString()).contains(" <type value=\"Patient\"/>");
  assertThat(response.getEntity().toString()).contains("\"someCustomOperation");
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  }
 
  private AbstractJaxRsConformanceProvider createConformanceProvider(final ConcurrentHashMap<Class<? extends IResourceProvider>, IResourceProvider> providers)

diff --git a/...ase/src/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu3Test.java b/...ase/src/test/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsConformanceProviderDstu3Test.java
@@ -22,6 +22,8 @@
 import static org.mockito.Mockito.when;
 
 public class AbstractJaxRsConformanceProviderDstu3Test {
+ private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(AbstractJaxRsConformanceProviderDstu3Test.class);
+
 
  private static final String BASEURI = "http:https://basiuri";
  private static final String REQUESTURI = BASEURI + "/metadata";
@@ -49,15 +51,15 @@ public void testConformance() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu3.class, new TestJaxRsDummyPatientProviderDstu3());
  Response response = createConformanceProvider(providers).conformance();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
  public void testConformanceUsingOptions() throws Exception {
  providers.put(AbstractJaxRsConformanceProvider.class, provider);
  providers.put(TestJaxRsDummyPatientProviderDstu3.class, new TestJaxRsDummyPatientProviderDstu3());
  Response response = createConformanceProvider(providers).conformanceUsingOptions();
- System.out.println(response);
+ ourLog.info(response.toString());
  }
 
  @Test
@@ -68,8 +70,8 @@ public void testConformanceWithMethods() throws Exception {
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
  assertThat(response.getEntity().toString()).contains("\"type\": \"Patient\"");
  assertThat(response.getEntity().toString()).contains("\"someCustomOperation");
- System.out.println(response);
- System.out.println(response.getEntity());
+ ourLog.info(response.toString());
+ ourLog.info(response.getEntity().toString());
  }
 
  @Test
@@ -79,10 +81,10 @@ public void testConformanceInXml() throws Exception {
  providers.put(TestJaxRsMockPatientRestProviderDstu3.class, new TestJaxRsMockPatientRestProviderDstu3());
  Response response = createConformanceProvider(providers).conformance();
  assertEquals(Constants.STATUS_HTTP_200_OK, response.getStatus());
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  assertThat(response.getEntity().toString()).contains(" <type value=\"Patient\"/>");
  assertThat(response.getEntity().toString()).contains("\"someCustomOperation");
- System.out.println(response.getEntity());
+ ourLog.info(response.getEntity().toString());
  }
 
  private AbstractJaxRsConformanceProvider createConformanceProvider(final ConcurrentHashMap<Class<? extends IResourceProvider>, IResourceProvider> providers)