Re-enable coding convention tests

test/architecture-tests/pom.xml

@@ -193,6 +193,22 @@
                     <skip>true</skip>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>${maven.surefire.version}</version>
+                <configuration>
+                    <argLine>-XX:MaxMetaspaceSize=1g -Xmx5120m</argLine>
+                    <forkCount>0</forkCount>
+                    <includes>
+                        <include>**/Test*.java</include>
+                        <include>**/*Tests.java</include>
+                        <include>**/*Test.java</include>
+                        <include>**/*TestCase.java</include>
+                    </includes>
+                    <skipTests>${skip.unit.tests}</skipTests>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 </project>

test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionTest.java

@@ -33,6 +33,7 @@
 import com.tngtech.archunit.core.domain.JavaMethod;
 import com.tngtech.archunit.core.domain.JavaModifier;
 import com.tngtech.archunit.core.domain.JavaParameter;
+import com.tngtech.archunit.core.importer.ImportOption;
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchIgnore;
 import com.tngtech.archunit.junit.ArchTest;
@@ -48,8 +49,9 @@
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 
-@AnalyzeClasses(packages = "software.amazon.awssdk..")
-@ArchIgnore(reason = "CI keeps crashing when running the tests. Ignoring them for now")
+@AnalyzeClasses(packages = "software.amazon.awssdk",
+    importOptions = ImportOption.DoNotIncludeTests.class)
+@ArchIgnore
 public class CodingConventionTest {
 
     @ArchTest
@@ -60,17 +62,20 @@ public class CodingConventionTest {
             .because("public APIs SHOULD be final");
 
     @ArchTest
+    @ArchIgnore(reason = "Ignoring it for now to avoid tests crashing")
     static final ArchRule mustNotUseJavaLogging =
         NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING;
 
     @ArchTest
+    @ArchIgnore(reason = "Ignoring it for now to avoid tests crashing")
     static final ArchRule mustNotUseSlfLoggerDirectly =
         freeze(noClasses().should(setFieldWhere(assignableFrom(org.slf4j.Logger.class)
                                                     .onResultOf(JavaAccess.Functions.Get.<JavaFieldAccess,
                                                         AccessTarget.FieldAccessTarget>target().then(GET_RAW_TYPE)))
                                       .as("use org.slf4j.Logger")).because("use software.amazon.awssdk.utils.Logger instead"));
 
     @ArchTest
+    @ArchIgnore
     static final ArchRule mustNotUseJodaTime =
         NO_CLASSES_SHOULD_USE_JODATIME;
 
@@ -107,6 +112,7 @@ public class CodingConventionTest {
                           .because("public APIs MUST NOT throw checked exception"));
 
     @ArchTest
+    @ArchIgnore(reason = "Ignoring it for now to avoid tests crashing")
     static final ArchRule shouldNotHaveMoreThanFourParams =
         freeze(noClasses().that().areAnnotatedWith(SdkProtectedApi.class).or().areAnnotatedWith(SdkPublicApi.class)
                    .should(new HasMoreThanFourParams())

test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionWithSuppressionTest.java

@@ -15,23 +15,30 @@
 
 package software.amazon.awssdk.archtests;
 
+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.noFields;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noMethods;
 import static com.tngtech.archunit.library.freeze.FreezingArchRule.freeze;
 
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.domain.JavaMethod;
+import com.tngtech.archunit.core.domain.JavaModifier;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.core.importer.ImportOption;
 import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchCondition;
 import com.tngtech.archunit.lang.ArchRule;
 import com.tngtech.archunit.lang.ConditionEvents;
 import com.tngtech.archunit.lang.SimpleConditionEvent;
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
 import org.junit.jupiter.api.Test;
+import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.utils.Logger;
 
 /**
@@ -58,13 +65,72 @@ public class CodingConventionWithSuppressionTest {
      */
     private static final Set<Pattern> ALLOWED_ERROR_LOG_SUPPRESSION = new HashSet<>();
 
+    @Test
+    void publicApisShouldBeFinal() {
+        System.out.println("publicApisShouldBeFinal");
+        JavaClasses classes = new ClassFileImporter()
+            .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests()))
+            .importPackages("software.amazon.awssdk");
+        freeze(classes().that().areAnnotatedWith(SdkPublicApi.class)
+                        .and().areNotInterfaces()
+                        .should().haveModifier(JavaModifier.FINAL))
+            .because("public APIs SHOULD be final")
+            .check(classes);
+        System.out.println("publicApisShouldBeFinal finished");
+    }
+
+    @Test
+    void shouldNotUseOptionalForFields() {
+        System.out.println("shouldNotUseOptionalForFields");
+        JavaClasses classes = new ClassFileImporter()
+            .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests()))
+            .importPackages("software.amazon.awssdk");
+        freeze(noFields().should().haveRawType(Optional.class)
+                         .as("use Optional for fields")
+                         .because("Optional SHOULD NOT be used for method parameters. See "
+                                  + "https://github.com/aws/aws-sdk-java-v2/blob/master/docs"
+                                  + "/design/UseOfOptional.md"))
+            .check(classes);
+        System.out.println("shouldNotUseOptionalForFields finished");
+    }
+
+    @Test
+    void mustNotUseOptionalForMethodParam() {
+        System.out.println("mustNotUseOptionalForMethodParam");
+        JavaClasses classes = new ClassFileImporter()
+            .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests()))
+            .importPackages("software.amazon.awssdk");
+        freeze(noMethods().should().haveRawParameterTypes(Optional.class)
+                          .as("use Optional for method parameters")
+                          .because("Optional MUST NOT be used for method parameters. See "
+                                   + "https://github.com/aws/aws-sdk-java-v2/blob/master/docs/design/UseOfOptional.md"))
+            .check(classes);
+        System.out.println("mustNotUseOptionalForMethodParam finished");
+    }
+
+    @Test
+    void publicApisMustNotDeclareThrowableOfCheckedException() {
+        System.out.println("publicApisMustNotDeclareThrowableOfCheckedException");
+        JavaClasses classes = new ClassFileImporter()
+            .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests()))
+            .importPackages("software.amazon.awssdk");
+        freeze(noMethods().that().arePublic().and()
+                          .areDeclaredInClassesThat().areAnnotatedWith(SdkPublicApi.class)
+                          .should()
+                          .declareThrowableOfType(Exception.class).orShould().declareThrowableOfType(IOException.class)
+                          .because("public APIs MUST NOT throw checked exception"))
+            .check(classes);
+        System.out.println("publicApisMustNotDeclareThrowableOfCheckedException finished");
+    }
+
     @Test
     void shouldNotAbuseWarnLog() {
+        System.out.println("shouldNotAbuseWarnLog");
         JavaClasses classes = new ClassFileImporter()
             .withImportOptions(Arrays.asList(
                 location -> ALLOWED_WARN_LOG_SUPPRESSION.stream().noneMatch(location::matches),
                 new ImportOption.Predefined.DoNotIncludeTests()))
-            .importPackages("software.amazon.awssdk..");
+            .importPackages("software.amazon.awssdk");
 
         ArchRule rule =
             freeze(methods().that().areDeclaredIn(Logger.class).and()
@@ -74,15 +140,17 @@ void shouldNotAbuseWarnLog() {
                     + " to ALLOWED_WARN_LOG_SUPPRESSION allowlist");
 
         rule.check(classes);
+        System.out.println("shouldNotAbuseWarnLog finished");
     }
 
     @Test
     void shouldNotAbuseErrorLog() {
+        System.out.println("shouldNotAbuseErrorLog");
         JavaClasses classes = new ClassFileImporter()
             .withImportOptions(Arrays.asList(
                 location -> ALLOWED_ERROR_LOG_SUPPRESSION.stream().noneMatch(location::matches),
                 new ImportOption.Predefined.DoNotIncludeTests()))
-            .importPackages("software.amazon.awssdk..");
+            .importPackages("software.amazon.awssdk");
 
         ArchRule rule =
             freeze(methods().that().areDeclaredIn(Logger.class).and()
@@ -91,6 +159,7 @@ void shouldNotAbuseErrorLog() {
                     + "ALLOWED_ERROR_LOG_SUPPRESSION allowlist");
 
         rule.check(classes);
+        System.out.println("shouldNotAbuseErrorLog finished");
     }
 
     private static final class MethodBeingUsedByOthers extends ArchCondition<JavaMethod> {

test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/NamingConventionTest.java

@@ -22,14 +22,9 @@
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchRule;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.function.Supplier;
-import java.util.regex.Pattern;
 import software.amazon.awssdk.awscore.presigner.SdkPresigner;
 
-@AnalyzeClasses(packages = "software.amazon.awssdk..")
+@AnalyzeClasses(packages = "software.amazon.awssdk")
 public class NamingConventionTest {
 
     @ArchTest

test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/NamingConventionWithSuppressionTest.java

@@ -54,7 +54,7 @@ void supplierImpl_shouldHaveSupplierSuffix() {
             .withImportOptions(Arrays.asList(
                 location -> ALLOWED_SUPPLIER_SUPPRESSION.stream().noneMatch(location::matches),
                 new ImportOption.Predefined.DoNotIncludeTests()))
-            .importPackages("software.amazon.awssdk..");
+            .importPackages("software.amazon.awssdk");
 
         ArchRule rule =
             classes().that().implement(Supplier.class).and().areNotPackagePrivate().should().haveSimpleNameEndingWith(