Port soft assertions from JUnit 4 to JUnit 5 + AssertJ

JUnit 5 itself offers only a very limited API for soft assertions.  The
rigid structure of this API does not align well with ways that WALA
already uses soft assertions.  AssertJ offers much more flexibility, if
we're willing to pick that up as an additional dependency.

core/build.gradle.kts

@@ -59,6 +59,7 @@ dependencies {
   api(projects.util) { because("public interface CallGraph extends interface NumberedGraph") }
   testFixturesImplementation(libs.ant)
   testFixturesImplementation(libs.junit.platform.launcher)
+  testImplementation(libs.assertj.core)
   testImplementation(libs.hamcrest)
   testRuntimeOnly(sourceSets["testSubjects"].output.classesDirs)
   // add the testSubjects source files to enable SourceMapTest to pass

core/src/test/java/com/ibm/wala/core/tests/arraybounds/ArrayboundsAnalysisTest.java

@@ -24,12 +24,14 @@
 import com.ibm.wala.ssa.SSAInstruction;
 import com.ibm.wala.types.TypeReference;
 import java.io.IOException;
+import org.assertj.core.api.HamcrestCondition;
+import org.assertj.core.api.SoftAssertions;
+import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
 import org.hamcrest.Matcher;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ErrorCollector;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 /**
  * The test data should be grouped, according to the behavior of the analysis. All array accesses of
@@ -42,6 +44,7 @@
  *
  * @author Stephan Gocht {@code <[email protected]>}
  */
+@ExtendWith(SoftAssertionsExtension.class)
 public class ArrayboundsAnalysisTest {
   private static final ClassLoader CLASS_LOADER = ArrayboundsAnalysisTest.class.getClassLoader();
 
@@ -59,9 +62,7 @@ public class ArrayboundsAnalysisTest {
   private static AnalysisScope scope;
   private static ClassHierarchy cha;
 
-  @Rule public ErrorCollector collector = new ErrorCollector();
-
-  @BeforeClass
+  @BeforeAll
   public static void init() throws IOException, ClassHierarchyException {
     scope =
         AnalysisScopeReader.instance.readJavaScope(TestConstants.WALA_TESTDATA, null, CLASS_LOADER);
@@ -82,28 +83,35 @@ public static IClass getIClass(String name) {
   }
 
   @Test
-  public void detectable() {
+  public void detectable(final SoftAssertions softly) {
     IClass iClass = getIClass(DETECTABLE_TESTDATA);
     assertAllSameNecessity(
-        iClass, DETECTABLE_NUMBER_OF_ARRAY_ACCESS, equalTo(UnnecessaryCheck.BOTH));
+        softly, iClass, DETECTABLE_NUMBER_OF_ARRAY_ACCESS, equalTo(UnnecessaryCheck.BOTH));
   }
 
   @Test
-  public void notDetectable() {
+  public void notDetectable(final SoftAssertions softly) {
     IClass iClass = getIClass(NOT_DETECTABLE_TESTDATA);
     assertAllSameNecessity(
-        iClass, NOT_DETECTABLE_NUMBER_OF_ARRAY_ACCESS, not(equalTo(UnnecessaryCheck.BOTH)));
+        softly, iClass, NOT_DETECTABLE_NUMBER_OF_ARRAY_ACCESS, not(equalTo(UnnecessaryCheck.BOTH)));
   }
 
   @Test
-  public void notInBound() {
+  public void notInBound(final SoftAssertions softly) {
     IClass iClass = getIClass(NOT_IN_BOUND_TESTDATA);
     assertAllSameNecessity(
-        iClass, NOT_IN_BOUND_TESTDATA_NUMBER_OF_ARRAY_ACCESS, not(equalTo(UnnecessaryCheck.BOTH)));
+        softly,
+        iClass,
+        NOT_IN_BOUND_TESTDATA_NUMBER_OF_ARRAY_ACCESS,
+        not(equalTo(UnnecessaryCheck.BOTH)));
   }
 
   public void assertAllSameNecessity(
-      IClass iClass, int expectedNumberOfArrayAccesses, Matcher<UnnecessaryCheck> matcher) {
+      final SoftAssertions softly,
+      IClass iClass,
+      int expectedNumberOfArrayAccesses,
+      Matcher<UnnecessaryCheck> matcher) {
+    final var condition = new HamcrestCondition<>(matcher);
     int numberOfArrayAccesses = 0;
     for (IMethod method : iClass.getAllMethods()) {
       if (method.getDeclaringClass().equals(iClass)) {
@@ -123,13 +131,15 @@ public void assertAllSameNecessity(
         for (SSAArrayReferenceInstruction key : analysis.getBoundsCheckNecessary().keySet()) {
           numberOfArrayAccesses++;
           UnnecessaryCheck unnecessary = analysis.getBoundsCheckNecessary().get(key);
-          collector.checkThat(
-              "Unexpected necessity for bounds check in "
-                  + identifyer
-                  + ":"
-                  + method.getLineNumber(key.iIndex()),
-              unnecessary,
-              matcher);
+          softly
+              .assertThat(unnecessary)
+              .as(
+                  () ->
+                      "Unexpected necessity for bounds check in "
+                          + identifyer
+                          + ":"
+                          + method.getLineNumber(key.iIndex()))
+              .satisfies(condition);
         }
       }
     }
@@ -145,13 +155,16 @@ public void assertAllSameNecessity(
      *
      * There is a bug, so not all array accesses are found.
      */
-    collector.checkThat(
-        "Number of found array accesses is not as expected for " + iClass.getName().toString(),
-        numberOfArrayAccesses,
-        equalTo(expectedNumberOfArrayAccesses));
+    softly
+        .assertThat(numberOfArrayAccesses)
+        .as(
+            () ->
+                "Number of found array accesses is not as expected for "
+                    + iClass.getName().toString())
+        .isEqualTo(expectedNumberOfArrayAccesses);
   }
 
-  @AfterClass
+  @AfterAll
   public static void free() {
     scope = null;
     cha = null;

core/src/test/java/com/ibm/wala/core/tests/arraybounds/PruneArrayOutOfBoundExceptionEdge.java

@@ -33,12 +33,14 @@
 import com.ibm.wala.util.collections.Pair;
 import java.io.IOException;
 import java.util.LinkedHashSet;
+import org.assertj.core.api.HamcrestCondition;
+import org.assertj.core.api.SoftAssertions;
+import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
 import org.hamcrest.Matcher;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ErrorCollector;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 /**
  * This test will check that:
@@ -58,6 +60,7 @@
  *
  * @author Stephan Gocht {@code <[email protected]>}
  */
+@ExtendWith(SoftAssertionsExtension.class)
 public class PruneArrayOutOfBoundExceptionEdge {
   private static final ClassLoader CLASS_LOADER =
       PruneArrayOutOfBoundExceptionEdge.class.getClassLoader();
@@ -88,9 +91,7 @@ public class PruneArrayOutOfBoundExceptionEdge {
   private static AnalysisScope scope;
   private static ClassHierarchy cha;
 
-  @Rule public ErrorCollector collector = new ErrorCollector();
-
-  @BeforeClass
+  @BeforeAll
   public static void init() throws IOException, ClassHierarchyException {
     scope =
         AnalysisScopeReader.instance.readJavaScope(TestConstants.WALA_TESTDATA, null, CLASS_LOADER);
@@ -131,24 +132,25 @@ private static IClass getIClass(String name) {
   }
 
   @Test
-  public void detectable() {
+  public void detectable(final SoftAssertions softly) {
     IClass iClass = getIClass(DETECTABLE_TESTDATA);
-    checkRemovedEdges(iClass, DETECTABLE_EXPECTED_COUNT);
+    checkRemovedEdges(softly, iClass, DETECTABLE_EXPECTED_COUNT);
   }
 
   @Test
-  public void notDetectable() {
+  public void notDetectable(final SoftAssertions softly) {
     IClass iClass = getIClass(NOT_DETECTABLE_TESTDATA);
-    checkRemovedEdges(iClass, NOT_DETECTABLE_EXPECTED_COUNT);
+    checkRemovedEdges(softly, iClass, NOT_DETECTABLE_EXPECTED_COUNT);
   }
 
   @Test
-  public void notInBound() {
+  public void notInBound(final SoftAssertions softly) {
     IClass iClass = getIClass(NOT_IN_BOUND_TESTDATA);
-    checkRemovedEdges(iClass, NOT_IN_BOUND_EXPECTED_COUNT);
+    checkRemovedEdges(softly, iClass, NOT_IN_BOUND_EXPECTED_COUNT);
   }
 
-  private void checkRemovedEdges(IClass iClass, int expectedNumberOfArrayAccesses) {
+  private void checkRemovedEdges(
+      final SoftAssertions softly, IClass iClass, int expectedNumberOfArrayAccesses) {
     int numberOfDeletedExceptionEdges = 0;
     for (IMethod method : iClass.getAllMethods()) {
       if (method.getDeclaringClass().equals(iClass)) {
@@ -160,9 +162,9 @@ private void checkRemovedEdges(IClass iClass, int expectedNumberOfArrayAccesses)
         PrunedCFG<SSAInstruction, ISSABasicBlock> prunedCfg = cfgs.snd;
 
         for (ISSABasicBlock block : cfg) {
-          checkNormalSuccessors(cfg, prunedCfg, block);
+          checkNormalSuccessors(softly, cfg, prunedCfg, block);
           boolean isEdgeRemoved =
-              checkExceptionalSuccessors(block, cfg, prunedCfg, method, identifyer);
+              checkExceptionalSuccessors(softly, block, cfg, prunedCfg, method, identifyer);
           numberOfDeletedExceptionEdges += isEdgeRemoved ? 1 : 0;
         }
       }
@@ -179,10 +181,10 @@ private void checkRemovedEdges(IClass iClass, int expectedNumberOfArrayAccesses)
      *
      * There is a bug, so not all edges are deleted.
      */
-    collector.checkThat(
-        "Number of deleted edges is not as expected for " + iClass.getName().toString(),
-        numberOfDeletedExceptionEdges,
-        equalTo(expectedNumberOfArrayAccesses));
+    softly
+        .assertThat(numberOfDeletedExceptionEdges)
+        .as(() -> "Number of deleted edges is not as expected for " + iClass.getName().toString())
+        .isEqualTo(expectedNumberOfArrayAccesses);
   }
 
   /**
@@ -193,6 +195,7 @@ private void checkRemovedEdges(IClass iClass, int expectedNumberOfArrayAccesses)
    * @return if an edge of block was removed
    */
   private boolean checkExceptionalSuccessors(
+      final SoftAssertions softly,
       ISSABasicBlock block,
       SSACFG cfg,
       PrunedCFG<SSAInstruction, ISSABasicBlock> prunedCfg,
@@ -218,44 +221,50 @@ private boolean checkExceptionalSuccessors(
         final Matcher<Iterable<? super TypeReference>> hasJLNPE = hasItem(isJLNPE);
         final Matcher<Iterable<? super TypeReference>> hasJLAIOOBE = hasItem(isJLAIOOBE);
         final Matcher<Iterable<? super TypeReference>> matcher1 = anyOf(hasJLNPE, hasJLAIOOBE);
-        collector.checkThat(
-            "Edge deleted but cause instruction can't throw NullPointerException"
-                + "nor ArrayIndexOutOfBoundsException: "
-                + identifyer
-                + ":"
-                + method.getLineNumber(lastInstruction.iIndex()),
-            lastInstruction.getExceptionTypes(),
-            matcher1);
+        softly
+            .<Iterable<TypeReference>>assertThat(lastInstruction.getExceptionTypes())
+            .as(
+                () ->
+                    "Edge deleted but cause instruction can't throw NullPointerException"
+                        + "nor ArrayIndexOutOfBoundsException: "
+                        + identifyer
+                        + ":"
+                        + method.getLineNumber(lastInstruction.iIndex()))
+            .satisfies(new HamcrestCondition<>(matcher1));
 
         final Matcher<TypeReference> itemMatcher = anyOf(isJLNPE, isJLAIOOBE);
-        collector.checkThat(
-            "Edge deleted but cause instruction throws other exceptions as NullPointerException"
-                + "and ArrayIndexOutOfBoundsException: "
-                + identifyer
-                + ":"
-                + method.getLineNumber(lastInstruction.iIndex()),
-            lastInstruction.getExceptionTypes(),
-            everyItem(itemMatcher));
+        softly
+            .<Iterable<TypeReference>>assertThat(lastInstruction.getExceptionTypes())
+            .as(
+                () ->
+                    "Edge deleted but cause instruction throws other exceptions as NullPointerException"
+                        + "and ArrayIndexOutOfBoundsException: "
+                        + identifyer
+                        + ":"
+                        + method.getLineNumber(lastInstruction.iIndex()))
+            .satisfies(new HamcrestCondition<>(everyItem(itemMatcher)));
 
       } else {
-        collector.addError(
-            new Throwable(
-                "Exceptional edge deleted, but no instruction as cause. - No last instruction."));
+        softly.fail(
+            "Exceptional edge deleted, but no instruction as cause. - No last instruction.");
       }
     }
     return isEdgeRemoved;
   }
 
   private void checkNormalSuccessors(
-      SSACFG cfg, PrunedCFG<SSAInstruction, ISSABasicBlock> prunedCfg, ISSABasicBlock block) {
+      final SoftAssertions softly,
+      SSACFG cfg,
+      PrunedCFG<SSAInstruction, ISSABasicBlock> prunedCfg,
+      ISSABasicBlock block) {
     LinkedHashSet<ISSABasicBlock> normalSuccessorCfg =
         new LinkedHashSet<>(cfg.getNormalSuccessors(block));
     LinkedHashSet<ISSABasicBlock> normalSuccessorPruned =
         new LinkedHashSet<>(prunedCfg.getNormalSuccessors(block));
-    collector.checkThat("", normalSuccessorPruned, equalTo(normalSuccessorCfg));
+    softly.assertThat(normalSuccessorPruned).isEqualTo(normalSuccessorCfg);
   }
 
-  @AfterClass
+  @AfterAll
   public static void free() {
     scope = null;
     cha = null;

core/src/test/java/com/ibm/wala/core/tests/exceptionpruning/ExceptionAnalysis2EdgeFilterTest.java

@@ -1,7 +1,7 @@
 package com.ibm.wala.core.tests.exceptionpruning;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.ibm.wala.analysis.exceptionanalysis.ExceptionAnalysis;
 import com.ibm.wala.analysis.exceptionanalysis.ExceptionAnalysis2EdgeFilter;
@@ -42,10 +42,8 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ErrorCollector;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
 
 /**
  * This Test checks, if the number of deleted edges is correct for TestPruning, it is also doing a
@@ -64,9 +62,7 @@ public class ExceptionAnalysis2EdgeFilterTest {
   private static PointerAnalysis<InstanceKey> pointerAnalysis;
   private static CombinedInterproceduralExceptionFilter<SSAInstruction> filter;
 
-  @Rule public ErrorCollector collector = new ErrorCollector();
-
-  @BeforeClass
+  @BeforeAll
   public static void init()
       throws IOException, ClassHierarchyException, IllegalArgumentException,
           CallGraphBuilderCancelException {
@@ -180,27 +176,27 @@ public void test() {
       }
     }
 
-    assertEquals("Number of normal edges deleted wrong:", 0, deletedNormal);
+    assertEquals(0, deletedNormal, "Number of normal edges deleted wrong:");
     for (Map.Entry<String, Integer> entry : deletedExceptional.entrySet()) {
       final String key = entry.getKey();
       final int value = entry.getValue();
       String text = "Number of exceptional edges deleted wrong for " + key + ":";
       switch (key) {
         case "testTryCatchMultipleExceptions":
-          assertEquals(text, 12, value);
+          assertEquals(12, value, text);
           break;
         case "testTryCatchOwnException":
         case "testTryCatchImplicitException":
-          assertEquals(text, 5, value);
+          assertEquals(5, value, text);
           break;
         case "testTryCatchSuper":
-          assertEquals(text, 3, value);
+          assertEquals(3, value, text);
           break;
         case "main":
-          assertEquals(text, 4, value);
+          assertEquals(4, value, text);
           break;
         default:
-          assertEquals(text, 0, value);
+          assertEquals(0, value, text);
           break;
       }
     }