Refactor Tests: Naming Conventions and Annotation Cleanup (#259)

This PR introduces several small improvements to the test codebase

- Naming Conventions
- Improved Test Names: Renamed tests to better reflect their purpose and functionality.
- Redundant Annotations Removal: Removed unnecessary @RunWith annotations where they were not adding value.
- Fix grammar and punctuations.

annotator-core/src/test/java/edu/ucr/cs/riple/core/AnalysisModeTest.java

@@ -49,14 +49,14 @@ public void strictModeTest() {
         .withDependency("Dep")
         .withSourceFile("Dep.java", "analysismode/Dep.java")
         .withExpectedReports(
-            // Resolves 6 errors locally and all errors on downstream dependencies can be resolved
-            // with no new triggered error, therefore the effect is -6. The fix triggers no error
-            // in downstream dependencies, should be approved.
+            // Resolves 6 errors locally, and all errors on downstream dependencies can be resolved
+            // with no new triggered error; therefore, the effect is -6.
+            // The fix triggers no error in downstream dependencies, should be approved.
             new TReport(
                 new OnMethod("Foo.java", "test.target.Foo", "returnNullGood()"), -6, APPROVE),
             // Resolves 6 errors locally but creates 1 error on downstream dependencies that cannot
-            // be resolved, therefore the effect is -5. The fix triggers 1 error in downstream
-            // dependencies, should be rejected.
+            // be resolved; therefore, the effect is -5.
+            // The fix triggers 1 error in downstream dependencies, should be rejected.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullBad()"), -5, REJECT))
         .setPredicate(
             (expected, found) ->
@@ -105,8 +105,8 @@ public void upperBoundModeTest() {
             new TReport(
                 new OnMethod("Foo.java", "test.target.Foo", "returnNullGood()"), -6, APPROVE),
             // Resolves 6 errors locally but creates 1 error on downstream dependencies that cannot
-            // be resolved, one of the triggered fixes in the tree, triggers an error in downstream
-            // dependency, the overall effect is -6 + 1 + 1 = -4 in upper bound mode.
+            // be resolved; one of the triggered fixes in the tree, triggers an error in downstream
+            // dependency; the overall effect is -6 + 1 + 1 = -4 in upper-bound mode.
             new TReport(
                 new OnMethod("Foo.java", "test.target.Foo", "returnNullBad()"), -4, APPROVE))
         .setPredicate(
@@ -129,14 +129,14 @@ public void strictModeWithSuppressRemainingErrorsTest() {
         .withDependency("Dep")
         .withSourceFile("Dep.java", "analysismode/Dep.java")
         .withExpectedReports(
-            // Resolves 6 errors locally and all errors on downstream dependencies can be resolved
-            // with no new triggered error, therefore the effect is -6. The fix triggers no error
-            // in downstream dependencies, should be approved.
+            // Resolves 6 errors locally, and all errors on downstream dependencies can be resolved
+            // with no new triggered error; therefore, the effect is -6.
+            // The fix triggers no error in downstream dependencies, should be approved.
             new TReport(
                 new OnMethod("Foo.java", "test.target.Foo", "returnNullGood()"), -6, APPROVE),
             // Resolves 6 errors locally but creates 1 error on downstream dependencies that cannot
-            // be resolved, therefore the effect is -5. The fix triggers 1 error in downstream
-            // dependencies, should be rejected.
+            // be resolved; therefore, the effect is -5.
+            // The fix triggers 1 error in downstream dependencies, should be rejected.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullBad()"), -5, REJECT))
         .setPredicate(
             (expected, found) ->

annotator-core/src/test/java/edu/ucr/cs/riple/core/AnnotatorBaseCoreTest.java

@@ -31,12 +31,14 @@
 import java.nio.file.Paths;
 import org.apache.commons.io.FileUtils;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 /** Base class for all core tests. */
+@Ignore
 @RunWith(JUnit4.class)
 public abstract class AnnotatorBaseCoreTest {
 

annotator-core/src/test/java/edu/ucr/cs/riple/core/ConfigurationTest.java

@@ -75,7 +75,7 @@ public class ConfigurationTest {
   @Before
   public void init() {
     testDir = temporaryFolder.getRoot().toPath();
-    // Make dummy config paths for 5 targets
+    // Make fake config paths for 5 targets
     try (OutputStream os = new FileOutputStream(testDir.resolve("paths.tsv").toFile())) {
       for (int i = 0; i < 5; i++) {
         String row = i + "nullaway.xml" + "\t" + i + "scanner.xml" + "\n";
@@ -303,9 +303,9 @@ private Config makeConfigWithFlags(List<CLIFlag> flags) {
 
   /**
    * Helper method for reading value of a node located at /key_1/key_2/.../key_n (in the form of
-   * {@code Xpath} query) from a xml document at the given path.
+   * {@code Xpath} query) from an XML document at the given path.
    *
-   * @param path Path to xml file.
+   * @param path Path to an XML file.
    * @param key Key to locate the value, can be nested in the form of {@code Xpath} query (e.g.
    *     /key1/key2/.../key_n).
    * @return The value in the specified keychain as {@code String}.

annotator-core/src/test/java/edu/ucr/cs/riple/core/CoreTest.java

@@ -35,10 +35,7 @@
 import java.util.Set;
 import org.junit.Assert;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
 public class CoreTest extends AnnotatorBaseCoreTest {
 
   public CoreTest() {
@@ -386,7 +383,7 @@ public void fieldNoInitialization() {
                 // effect is -1 + 1 (triggered error on this.f = foo()) = 0.
                 new OnMethod("A.java", "test.A", "foo()"),
                 -2,
-                // adding @Nullable on f will resolve the triggered error by foo() and also resolves
+                // Adding @Nullable on f will resolve the triggered error by foo() and also resolves
                 // the initialization error on A() as well.
                 // Therefore, the combined effect is -1 + (-1) = -2. It resolves all existing
                 // errors.

annotator-core/src/test/java/edu/ucr/cs/riple/core/DeepTest.java

@@ -33,10 +33,7 @@
 import edu.ucr.cs.riple.injector.location.OnParameter;
 import java.util.Collections;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
 public class DeepTest extends AnnotatorBaseCoreTest {
 
   public DeepTest() {

annotator-core/src/test/java/edu/ucr/cs/riple/core/DownstreamAnalysisTest.java

@@ -59,7 +59,7 @@ public void publicMethodWithDownstreamDependencyEnabled() {
             // DepA by 0, DepB by 1 and DepC by 0. Hence, the total effect is: -4.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullableGood(int)"), -4),
             // Change increases errors on target by 1, and also increases them in downstream
-            // dependency DepA by 1. Hence , the total effect is: 2.
+            // dependency DepA by 1.Hence, the total effect is: 2.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "bar()"), 2))
         .setPredicate(
             (expected, found) ->
@@ -108,21 +108,21 @@ public void publicFieldWithDownstreamDependencyEnabled() {
         .withSourceFile("DepC.java", "downstreamDependencyFieldCheck/DepC.java")
         .withExpectedReports(
             // Effect on target is -1, Effect on DepA is 0 and on DepB and DepC is 1 ->
-            // Lower bound is 2. And overall effect is -1 + 2 = 1. Effect is greater than 0 and
-            // triggers unresolved errors on downstream dependencies, hence the tree should be
-            // tagged as REJECT.
+            // Lower bound is 2. And overall effect is -1 + 2 = 1.
+            // The Effect is greater than 0 and triggers unresolved errors on downstream
+            // dependencies, hence the tree should be tagged as REJECT.
             new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f")), 1, REJECT),
             // Effect on target is -2. Root is on f1, but it triggers making f @Nullable as well.
-            // Fix tree containing both fixes resolves two errors leaving no remaining error, effect
-            // on target is -2. But f creates 2 errors on downstream dependencies, hence the lower
-            // bound is 2. Overall effect is -2 + 2 = 0. Since f creates unresolved errors on
+            // Fix a tree containing both fixes resolves two errors leaving no remaining error,
+            // effect on target is -2. But f creates 2 errors on downstream dependencies, hence the
+            // lower bound is 2. Overall effect is -2 + 2 = 0. Since f creates unresolved errors on
             // downstream dependencies, the tree should be tagged as REJECT even though the overall
             // effect is not greater than 0.
             new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f1")), 0, REJECT),
             // Effect on target is -1. Root is on f2, but it triggers making f3 @Nullable through an
-            // assignment in DepA and the tree is extended to include the corresponding fix. Since,
+            // assignment in DepA and the tree is extended to include the corresponding fix. Since
             // f2 creates a resolvable error in downstream dependencies that the corresponding fix
-            // is present in the fix tree, the lower bound effect is 0. Overall effect is -1 + 0 =
+            // is present in the fix tree, the lower-bound effect is 0. Overall effect is -1 + 0 =
             // -1. Since the overall effect is less than 0, with no error in downstream
             // dependencies, the tree should be tagged as APPROVE.
             new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f2")), -1, APPROVE))
@@ -158,8 +158,8 @@ public void lowerBoundComputationTest() {
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullableBad(int)"), 10),
             // Only returnNullableGood triggers new errors in this fix chain (+1), lower bound is 1.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullableGood(int)"), 1),
-            // Root fix triggers 1 error on downstream dependency but returnNullableBad is
-            // present in the fix tree, therefore the lower bound effect for the tree should be 10.
+            // Root fix triggers 1 error on downstream dependency, but returnNullableBad is
+            // present in the fix tree, therefore, the lower-bound effect for the tree should be 10.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "bar()"), 10))
         .setPredicate(
             (expected, found) ->
@@ -191,8 +191,8 @@ public void upperBoundComputationTest() {
             // should be 1
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "returnNullableGood(int)"), 1),
             // Root fix triggers 1 error on downstream dependency and returnNullableBad is
-            // present in the fix tree and triggers 10 errors on downstream dependency, therefore
-            // the upper bound effect for the tree should be 11.
+            // present in the fix tree and triggers 10 errors on downstream dependency, therefore,
+            // the upper-bound effect for the tree should be 11.
             new TReport(new OnMethod("Foo.java", "test.target.Foo", "bar()"), 11))
         .setPredicate(
             (expected, found) ->

annotator-core/src/test/java/edu/ucr/cs/riple/core/LombokTest.java

@@ -50,10 +50,10 @@ public void regionComputationOnFieldParallelProcessingEnables() {
             "class Main {",
             "   Object f;",
             "   public Object m1(){",
-            "       return getF();", // Should be error
+            "       return getF();", // Should be an error
             "   }",
             "   public Object m2(){",
-            "       return getF();", // Should be error
+            "       return getF();", // Should be an error
             "   }",
             "}")
         .withExpectedReports(
@@ -80,15 +80,15 @@ public void rejectOnFieldForGeneratedGetterInDownstreamDependencies() {
             "package test;",
             "class Dep {",
             "   public Object returnNullable(){",
-            "       return new Main().getF();", // Should be error
+            "       return new Main().getF();", // Should be an error
             "   }",
             "}")
         .withExpectedReports(
             new TReport(
                 new OnField("Main.java", "test.Main", Collections.singleton("f")),
                 0,
                 // The copied annotation on the getF creates an unresolvable error in downstream
-                // dependency, hence, the tree should be rejected.
+                // dependency; hence, the tree should be rejected.
                 Report.Tag.REJECT))
         .setPredicate(
             (expected, found) ->

annotator-core/src/test/java/edu/ucr/cs/riple/core/NullableFlowToUpstreamTest.java

@@ -77,7 +77,7 @@ public void nullableFlowDetectionEnabledTest() {
                         "Foo.java", "test.target.Foo", "bar1(java.lang.Object,java.lang.Object)")),
                 Collections.emptySet()),
             // Change creates two errors on downstream dependencies (1 resolvable) and resolves one
-            // error locally, therefore the overall effect is 0.
+            // error locally; therefore, the overall effect is 0.
             new TReport(
                 new OnMethod("Foo.java", "test.target.Foo", "getNull()"),
                 0,
@@ -141,7 +141,6 @@ public void nullableFlowToUpstreamThroughFieldWriteTest() {
             new TReport(
                 new OnField("Bar.java", "test.Bar", Set.of("foo")),
                 -1,
-                // fixes in tree:
                 Set.of(
                     new OnMethod("Bar.java", "test.Bar", "getFoo(String)"),
                     // coming from flow of nullable back to target through a field write.

annotator-core/src/test/java/edu/ucr/cs/riple/core/OffsetChangeHandlingTest.java

@@ -212,7 +212,7 @@ private void removeAnnotationOn(String... fields) {
             .collect(Collectors.toSet()));
   }
 
-  /** Verifies if the calculated offsets matches the original offsets. */
+  /** Verifies if the calculated offsets match the original offsets. */
   private void verifyCalculatedOffsets() {
     try {
       String content = Files.readString(root.resolve("benchmark.java"));

annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/AnnotatorScannerBaseTest.java

@@ -36,9 +36,14 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 
+@Ignore
+@RunWith(JUnit4.class)
 public abstract class AnnotatorScannerBaseTest<T extends Display> {
 
   @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();

annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/ClassRecordTest.java

@@ -28,10 +28,7 @@
 import edu.ucr.cs.riple.scanner.tools.ClassRecordDisplay;
 import edu.ucr.cs.riple.scanner.tools.DisplayFactory;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
 public class ClassRecordTest extends AnnotatorScannerBaseTest<ClassRecordDisplay> {
 
   private static final DisplayFactory<ClassRecordDisplay> CLASS_DISPLAY_FACTORY =
@@ -59,7 +56,7 @@ public void basicTest() {
   }
 
   @Test
-  public void checkClassesAreWrittenInFlatName() {
+  public void checkClassesAreWrittenInFlatNameTest() {
     tester
         .addSourceFile("SampleClassForTest.java")
         .setExpectedOutputs(

annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java

@@ -58,7 +58,7 @@
 public class ConfigurationTest {
 
   @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
-  // Just a dummy factory to run the tests with, tests will not even start with a null factory.
+  // Just a fake factory to run the tests with, tests will not even start with a null factory.
   protected DisplayFactory<Display> factory =
       values -> new ClassRecordDisplay("Unknown", "Unknown");
   protected SerializationTestHelper<Display> tester;
@@ -71,7 +71,7 @@ public void setup() {
 
   @Test
   public void checkExceptionIsThrownIfConfigPathNotSet() {
-    // -XepOpt:Scanner:ConfigPath is not set should expect an error.
+    // - XepOpt:Scanner:ConfigPath is not set should expect an error.
     tester =
         new SerializationTestHelper<>(root)
             .setArgs(

annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/FieldImpactedRegionTest.java

@@ -28,10 +28,7 @@
 import edu.ucr.cs.riple.scanner.tools.DisplayFactory;
 import edu.ucr.cs.riple.scanner.tools.ImpactedRegionRecordDisplay;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
 public class FieldImpactedRegionTest extends AnnotatorScannerBaseTest<ImpactedRegionRecordDisplay> {
 
   private static final DisplayFactory<ImpactedRegionRecordDisplay> FIELD_TRACKER_DISPLAY_FACTORY =
@@ -57,7 +54,7 @@ public FieldImpactedRegionTest() {
   }
 
   @Test
-  public void BasicTest() {
+  public void basicTest() {
     tester
         .addSourceLines(
             "edu/ucr/A.java",
@@ -146,7 +143,7 @@ public void fieldDeclaredRegionComputationAllCases() {
   }
 
   @Test
-  public void LombokGeneratedCodeDetectionTest() {
+  public void lombokGeneratedCodeDetectionTest() {
     tester
         .addSourceLines(
             "lombok/Generated.java", "package lombok;", "public @interface Generated { }")

annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/MethodImpactedRegionTest.java

@@ -28,10 +28,7 @@
 import edu.ucr.cs.riple.scanner.tools.DisplayFactory;
 import edu.ucr.cs.riple.scanner.tools.ImpactedRegionRecordDisplay;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
 public class MethodImpactedRegionTest
     extends AnnotatorScannerBaseTest<ImpactedRegionRecordDisplay> {
 
@@ -58,7 +55,7 @@ public MethodImpactedRegionTest() {
   }
 
   @Test
-  public void BasicTest() {
+  public void basicTest() {
     tester
         .addSourceLines(
             "edu/ucr/A.java",
@@ -104,7 +101,7 @@ public void constructorCallTest() {
   }
 
   @Test
-  public void methodReference() {
+  public void methodReferenceTest() {
     tester
         .addSourceLines(
             "edu/ucr/A.java",
@@ -131,7 +128,7 @@ public void methodReference() {
   }
 
   @Test
-  public void lambda() {
+  public void lambdaTest() {
     tester
         .addSourceLines(
             "edu/ucr/A.java",