From e1103fb8541394cf374409101a989317a434bb6b Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:10:42 -0800 Subject: [PATCH] Use junit directly --- .../archtests/CodingConventionTest.java | 1 + .../CodingConventionWithSuppressionTest.java | 130 ++++++++++++++---- 2 files changed, 101 insertions(+), 30 deletions(-) diff --git a/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionTest.java b/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionTest.java index 87d2628c512..1c05dd5b1ac 100644 --- a/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionTest.java +++ b/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionTest.java @@ -51,6 +51,7 @@ @AnalyzeClasses(packages = "software.amazon.awssdk..", importOptions = ImportOption.DoNotIncludeTests.class) +@ArchIgnore public class CodingConventionTest { @ArchTest diff --git a/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionWithSuppressionTest.java b/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionWithSuppressionTest.java index 8259a12ca0d..2a1a24f73ab 100644 --- a/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionWithSuppressionTest.java +++ b/test/architecture-tests/src/test/java/software/amazon/awssdk/archtests/CodingConventionWithSuppressionTest.java @@ -15,11 +15,16 @@ 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.noClasses; +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; @@ -27,11 +32,15 @@ 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.concurrent.Future; import java.util.regex.Pattern; import org.junit.jupiter.api.Test; +import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.utils.Logger; /** @@ -58,41 +67,102 @@ public class CodingConventionWithSuppressionTest { */ private static final Set ALLOWED_ERROR_LOG_SUPPRESSION = new HashSet<>(); - @Test - void 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.."); - - ArchRule rule = - freeze(methods().that().areDeclaredIn(Logger.class).and() - .haveName("warn").should(new MethodBeingUsedByOthers( - "log.warn is detected"))) - .as("log.warn is detected. Review it with the team. If this is a valid case, add it" - + " to ALLOWED_WARN_LOG_SUPPRESSION allowlist"); - - rule.check(classes); - } + // @Test + // void 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); + // } @Test - void shouldNotAbuseErrorLog() { + void shouldNotUseFuture() { JavaClasses classes = new ClassFileImporter() - .withImportOptions(Arrays.asList( - location -> ALLOWED_ERROR_LOG_SUPPRESSION.stream().noneMatch(location::matches), - new ImportOption.Predefined.DoNotIncludeTests())) - .importPackages("software.amazon.awssdk.."); - - ArchRule rule = - freeze(methods().that().areDeclaredIn(Logger.class).and() - .haveName("error").should(new MethodBeingUsedByOthers("log.error is detected"))) - .as("log.error is detected. Review it with the team. If this is a valid case, add it to " - + "ALLOWED_ERROR_LOG_SUPPRESSION allowlist"); - - rule.check(classes); + .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests())) + .importPackages("software.amazon.awssdk"); + freeze(noClasses().should().dependOnClassesThat().areAssignableFrom(Future.class) + .as("use java.util.concurrent.Future") + .because("Future SHOULD NOT be used, use CompletableFuture instead")) + .check(classes); } + // @Test + // void 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); + // } + // + // @Test + // void 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); + // } + // + // @Test + // void publicApisMustNotDeclareThrowableOfCheckedException() { + // JavaClasses classes = new ClassFileImporter() + // .withImportOptions(Arrays.asList(new ImportOption.Predefined.DoNotIncludeTests())) + // .importPackages("software.amazon.awssdk"); + // freeze(noMethods().that() + // .areDeclaredInClassesThat().areAnnotatedWith(SdkPublicApi.class) + // .should() + // .declareThrowableOfType(Exception.class).orShould().declareThrowableOfType(IOException.class) + // .because("public APIs MUST NOT throw checked exception")) + // .check(classes); + // } + // + // @Test + // void 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.."); + // + // ArchRule rule = + // freeze(methods().that().areDeclaredIn(Logger.class).and() + // .haveName("warn").should(new MethodBeingUsedByOthers( + // "log.warn is detected"))) + // .as("log.warn is detected. Review it with the team. If this is a valid case, add it" + // + " to ALLOWED_WARN_LOG_SUPPRESSION allowlist"); + // + // rule.check(classes); + // } + // + // @Test + // void 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.."); + // + // ArchRule rule = + // freeze(methods().that().areDeclaredIn(Logger.class).and() + // .haveName("error").should(new MethodBeingUsedByOthers("log.error is detected"))) + // .as("log.error is detected. Review it with the team. If this is a valid case, add it to " + // + "ALLOWED_ERROR_LOG_SUPPRESSION allowlist"); + // + // rule.check(classes); + // } + private static final class MethodBeingUsedByOthers extends ArchCondition { public MethodBeingUsedByOthers(String description) { super(description);