Enabling checkstyle for test and integration test folders and adds a check to prohibit the @ignore annotation (#3631)

Author: cenedhryn

build-tools/src/main/java/software/amazon/awssdk/buildtools/checkstyle/NoIgnoreAnnotationsCheck.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.buildtools.checkstyle;
+
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import com.puppycrawl.tools.checkstyle.utils.AnnotationUtil;
+
+/**
+ * Checks if a class uses the @Ignore annotation. Avoid disabling tests and work to
+ * resolve issues with the test instead.
+ *
+ * For manual tests and exceptional circumstances, use the commentation feature CHECKSTYLE: OFF
+ * to mark a test as ignored.
+ */
+public class NoIgnoreAnnotationsCheck extends AbstractCheck {
+
+    private static final String IGNORE_ANNOTATION = "Ignore";
+
+    @Override
+    public int[] getDefaultTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return new int[] {TokenTypes.CLASS_DEF, TokenTypes.METHOD_DEF};
+    }
+
+    @Override
+    public void visitToken(DetailAST ast) {
+        if (!AnnotationUtil.containsAnnotation(ast, IGNORE_ANNOTATION)) {
+            return;
+        }
+
+        log(ast, "@Ignore annotation is not allowed");
+    }
+}

build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml

@@ -16,17 +16,13 @@
   -->
 
 <!DOCTYPE suppressions PUBLIC
-        "-//Puppy Crawl//DTD Suppressions 1.1//EN"
-        "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+        "-//Puppy Crawl//DTD Suppressions 1.2//EN"
+        "http://www.puppycrawl.com/dtds/suppressions_1_2.dtd">
 <suppressions>
-    <!-- TODO: Disable these suppressions. -->
-    <suppress checks=".*"
+    <!-- ignore all checks for test classes except for the NoIgnoreAnnotationsCheck -->
+    <suppress checks="^((?!NoIgnoreAnnotationsCheck).)*$"
               files=".*[\\/](test|it)[\\/]java[\\/].+\.java$"/>
 
-    <!-- TODO: Disable these suppressions. -->
-    <suppress checks=".*"
-              files=".(flow)[\\/].+\.java$"/>
-
     <!-- ignore missing annotation checks under test/codegen directory -->
     <suppress checks="MissingSdkAnnotationCheck"
               files=".(codegen|test|release)[\\/].+\.java$"/>

build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml

@@ -384,7 +384,6 @@
             <property name="ignoreComments" value="true"/>
         </module>
 
-
         <!-- Checks that we don't use AttributeKey.newInstance directly -->
         <module name="Regexp">
             <property name="format" value="AttributeKey\.newInstance"/>
@@ -401,6 +400,10 @@
             <property name="ignoreComments" value="true"/>
         </module>
 
+        <!-- Checks that we don't use the @Ignore annotation on tests -->
+        <module name="software.amazon.awssdk.buildtools.checkstyle.NoIgnoreAnnotationsCheck">
+        </module>
+
         <!-- Checks that we don't use plural enum names -->
         <module name="software.amazon.awssdk.buildtools.checkstyle.PluralEnumNames"/>
 

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/ExecutionInterceptor.java

@@ -23,8 +23,8 @@
 import software.amazon.awssdk.core.SdkRequest;
 import software.amazon.awssdk.core.SdkResponse;
 import software.amazon.awssdk.core.async.AsyncRequestBody;
-// Disable CS to avoid "Unused Import" error. If we use the FQCN in the Javadoc, we'll run into line length issues instead.
-// CHECKSTYLE:OFF
+
+// CHECKSTYLE:OFF - Avoid "Unused Import" error. If we use the FQCN in the Javadoc, we'll run into line length issues instead.
 import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
 // CHECKSTYLE:ON
 import software.amazon.awssdk.core.retry.RetryPolicy;

pom.xml

@@ -170,6 +170,7 @@
         <reactive-streams.version>1.0.3</reactive-streams.version>
 
         <skip.unit.tests>${skipTests}</skip.unit.tests>
+        <integTestSourceDirectory>${project.basedir}/src/it/java</integTestSourceDirectory>
     </properties>
 
     <dependencyManagement>
@@ -422,6 +423,8 @@
                         <consoleOutput>true</consoleOutput>
                         <failsOnError>true</failsOnError>
                         <logViolationsToConsole>true</logViolationsToConsole>
+                        <testSourceDirectories>${project.build.testSourceDirectory},${integTestSourceDirectory}</testSourceDirectories>
+                        <includeTestSourceDirectory>true</includeTestSourceDirectory>
                         <failOnViolation>true</failOnViolation>
                         <excludes>**/module-info.java</excludes>
                     </configuration>

services/sqs/src/it/java/software/amazon/awssdk/services/sqs/SqsConcurrentPerformanceIntegrationTest.java

@@ -45,8 +45,10 @@ public class SqsConcurrentPerformanceIntegrationTest extends IntegrationTestBase
      * You can use the netstat command to look at the current sockets connected to SQS and verify
      * that they don't sit around in CLOSE_WAIT, and are correctly being reaped.
      */
+    // CHECKSTYLE:OFF - Allowing @Ignore for this manual test
     @Test
     @Ignore
+    // CHECKSTYLE:ON
     public void testIdleConnectionReaping() throws Exception {
         sqs = SqsAsyncClient.builder().credentialsProvider(getCredentialsProvider()).build();
         sqs = SqsAsyncClient.builder().credentialsProvider(getCredentialsProvider()).build();
@@ -79,4 +81,4 @@ public void run() {
     private void waitForUserInput() throws IOException {
         new BufferedReader(new InputStreamReader(System.in)).readLine();
     }
-}
+}

utils/src/main/java/software/amazon/awssdk/utils/UserHomeDirectoryUtils.java

@@ -30,8 +30,7 @@ private UserHomeDirectoryUtils() {
     }
 
     public static String userHomeDirectory() {
-        // To match the logic of the CLI we have to consult environment variables directly.
-        // CHECKSTYLE:OFF
+        // CHECKSTYLE:OFF - To match the logic of the CLI we have to consult environment variables directly.
         Optional<String> home = SystemSetting.getStringValueFromEnvironmentVariable("HOME");
 
         if (home.isPresent()) {