Apply assorted test cleanup

Summary of changes: - Inline more `CompilationTestHelper` fields. - Move inner class to the bottom of the outer class. - Improve test parameter name.
PicnicSupermarket · Apr 13, 2023 · 6498ced · 6498ced
1 parent 977019c
commit 6498ced
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 37 deletions.
diff --git a/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionTest.java b/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionTest.java
@@ -10,12 +10,9 @@
 // `String.valueOf(null)` may not. That is because the latter matches `String#valueOf(char[])`. We
 // could special-case `null` arguments, but that doesn't seem worth the trouble.
 final class RedundantStringConversionTest {
-  private final CompilationTestHelper compilationTestHelper =
-      CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass());
-
   @Test
   void identificationOfIdentityTransformation() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "class A {",
@@ -38,7 +35,7 @@ void identificationOfIdentityTransformation() {
 
   @Test
   void identificationWithinMutatingAssignment() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -99,7 +96,7 @@ void identificationWithinMutatingAssignment() {
 
   @Test
   void identificationWithinBinaryOperation() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -194,7 +191,7 @@ void identificationWithinBinaryOperation() {
 
   @Test
   void identificationWithinStringBuilderMethod() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -249,7 +246,7 @@ void identificationWithinStringBuilderMethod() {
   // XXX: Also test the other formatter methods.
   @Test
   void identificationWithinFormatterMethod() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.util.Formattable;",
@@ -294,7 +291,7 @@ void identificationWithinFormatterMethod() {
 
   @Test
   void identificationWithinGuavaGuardMethod() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import static com.google.common.base.Preconditions.checkArgument;",
@@ -354,7 +351,7 @@ void identificationWithinGuavaGuardMethod() {
 
   @Test
   void identificationWithinSlf4jLoggerMethod() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.util.Formattable;",

diff --git a/...prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/JavaKeywordsTest.java b/...prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/JavaKeywordsTest.java
@@ -10,7 +10,7 @@
 
 final class JavaKeywordsTest {
   private static Stream<Arguments> isValidIdentifierTestCases() {
-    /* { str, expected } */
+    /* { string, expected } */
     return Stream.of(
         arguments("", false),
         arguments("public", false),
@@ -28,7 +28,7 @@ private static Stream<Arguments> isValidIdentifierTestCases() {
 
   @MethodSource("isValidIdentifierTestCases")
   @ParameterizedTest
-  void isValidIdentifier(String str, boolean expected) {
-    assertThat(JavaKeywords.isValidIdentifier(str)).isEqualTo(expected);
+  void isValidIdentifier(String string, boolean expected) {
+    assertThat(JavaKeywords.isValidIdentifier(string)).isEqualTo(expected);
   }
 }
diff --git a/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java b/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java
@@ -16,22 +16,6 @@
 import org.junit.jupiter.api.Test;
 
 final class MethodMatcherFactoryTest {
-  /** A {@link BugChecker} that flags method invocations matched by {@link #TEST_MATCHER}. */
-  @BugPattern(severity = SUGGESTION, summary = "Flags methods matched by the test matcher.")
-  public static final class MatchedMethodsFlagger extends BugChecker
-      implements MethodInvocationTreeMatcher {
-    private static final long serialVersionUID = 1L;
-
-    @Override
-    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-      if (TEST_MATCHER.matches(tree, state)) {
-        return buildDescription(tree).build();
-      }
-
-      return Description.NO_MATCH;
-    }
-  }
-
   private static final Matcher<ExpressionTree> TEST_MATCHER =
       new MethodMatcherFactory()
           .create(
@@ -207,4 +191,20 @@ void matcher() {
             "}")
         .doTest();
   }
+
+  /** A {@link BugChecker} that flags method invocations matched by {@link #TEST_MATCHER}. */
+  @BugPattern(severity = SUGGESTION, summary = "Flags methods matched by the test matcher.")
+  public static final class MatchedMethodsFlagger extends BugChecker
+      implements MethodInvocationTreeMatcher {
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+      if (TEST_MATCHER.matches(tree, state)) {
+        return buildDescription(tree).build();
+      }
+
+      return Description.NO_MATCH;
+    }
+  }
 }
diff --git a/...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java b/...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java
@@ -18,12 +18,9 @@
 import reactor.core.publisher.Flux;
 
 final class ThirdPartyLibraryTest {
-  private final CompilationTestHelper compilationTestHelper =
-      CompilationTestHelper.newInstance(TestChecker.class, getClass());
-
   @Test
   void isIntroductionAllowed() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(TestChecker.class, getClass())
         .addSourceLines(
             "A.java",
             "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true",
@@ -33,7 +30,7 @@ void isIntroductionAllowed() {
 
   @Test
   void isIntroductionAllowedWitnessClassesInSymtab() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(TestChecker.class, getClass())
         .addSourceLines(
             "A.java",
             "import com.google.common.collect.ImmutableList;",
@@ -55,7 +52,7 @@ void isIntroductionAllowedWitnessClassesInSymtab() {
 
   @Test
   void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(TestChecker.class, getClass())
         .withClasspath(ImmutableList.class, Flux.class)
         .addSourceLines(
             "A.java",
@@ -66,7 +63,7 @@ void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
 
   @Test
   void isIntroductionAllowedWitnessClassesNotOnClassPath() {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(TestChecker.class, getClass())
         .withClasspath()
         .addSourceLines(
             "A.java",
@@ -79,7 +76,7 @@ void isIntroductionAllowedWitnessClassesNotOnClassPath() {
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) {
-    compilationTestHelper
+    CompilationTestHelper.newInstance(TestChecker.class, getClass())
         .setArgs("-XepOpt:ErrorProneSupport:IgnoreClasspathCompat=" + ignoreClassPath)
         .withClasspath(ImmutableList.class, Flux.class)
         .addSourceLines(