Add support for fully skipping tests

Some tests are incorrect and known to crash even on correct JDWP
implementations. This can lead to error logs being hard to parse since
there will be spurious crashes in them. To improve this situation we
add the ability to entirely suppress the running of specific tests by
passing them, in a comma-delimited list, with the
jpda.settings.badTestCases variable.

Test: ./art/tools/run-libjdwp-tests.sh --mode=host --skip-test 'org.apache.harmony.jpda.tests.jdwp.VirtualMachine_VersionTest#testVersion001'
      Examine output
Bug: 141907697

Change-Id: Icbac5f3cd82689e04660448fd2796159c5e11da2

2 files changed, 124 insertions, 11 deletions

diff --git a/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/AllTests.java b/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/AllTests.java
index e0ed276..55ce1ca 100644
--- a/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/AllTests.java
+++ b/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/AllTests.java
@@ -18,41 +18,141 @@
 
 package org.apache.harmony.jpda.tests.share;
 
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import org.apache.harmony.jpda.tests.jdwp.share.JDWPRawTestCase;
 
 public class AllTests {
+  private static JPDATestOptions TEST_OPTIONS = new JPDATestOptions();
+
   public static void main(String[] args) {
     junit.framework.TestResult result = junit.textui.TestRunner.run(suite());
     if (!result.wasSuccessful()) {
-        System.exit(1);
+      System.exit(1);
+    }
+  }
+
+  public static final class BadTestClassException extends IllegalArgumentException {
+    public BadTestClassException(String s) {
+      super(s);
+    }
+
+    public BadTestClassException(String s, Throwable t) {
+      super(s, t);
     }
   }
 
-  private static void addOptionalTestSuite(junit.framework.TestSuite suite, String classname) {
+  public static junit.framework.Test makeWarning(String name, String msg) {
+    return new junit.framework.TestCase(name) {
+      protected void runTest() {
+        JPDALogWriter lw = new JPDALogWriter(System.out, null, TEST_OPTIONS.isVerbose());
+        lw.printError("Skipping " + name + " due to: " + msg);
+      }
+    };
+  }
+  public static interface FilterSuite {
+    public void addTestSuite(Class<? extends JDWPRawTestCase> k);
+  }
+
+
+  public static class FilteredTestSuite extends junit.framework.TestSuite {
+    public FilteredTestSuite(Class<? extends JDWPRawTestCase> tc, Predicate<Method> is_good)
+        throws BadTestClassException {
+      super();
+
+      setName(tc.getName());
+
+      if (!Modifier.isPublic(tc.getModifiers())) {
+        throw new BadTestClassException(tc + " is not public");
+      }
+      Function<Method, junit.framework.Test> mktest =
+          (m) -> {
+            try {
+              junit.framework.TestCase res = (junit.framework.TestCase) tc.newInstance();
+              res.setName(m.getName());
+              return res;
+            } catch (Exception t) {
+              return makeWarning(
+                  m.getName(), "Unable to create test case for " + m + " because of " + t);
+            }
+          };
+      Class<?> curClass = tc;
+      Set<String> seen_names = new HashSet<>();
+      while (junit.framework.Test.class.isAssignableFrom(curClass)) {
+        for (Method m : tc.getDeclaredMethods()) {
+          addTestMethod(m, seen_names, is_good, mktest);
+        }
+        curClass = curClass.getSuperclass();
+      }
+    }
+
+    private void addTestMethod(
+        Method m,
+        Set<String> seen,
+        Predicate<Method> is_good,
+        Function<Method, junit.framework.Test> mkTest) {
+      if (seen.contains(m.getName())) {
+        return;
+      }
+      seen.add(m.getName());
+      if (Modifier.isPublic(m.getModifiers())
+          && m.getParameterCount() == 0
+          && m.getName().startsWith("test")
+          && m.getReturnType().equals(Void.TYPE)) {
+        if (is_good.test(m)) {
+          addTest(mkTest.apply(m));
+        } else {
+          addTest(makeWarning(m.getName(), "Skipping test " + m + " due to explicit skip"));
+        }
+      }
+    }
+  }
+
+  private static void addOptionalTestSuite(FilterSuite suite, String classname) {
     try {
-      suite.addTestSuite((Class<? extends junit.framework.TestCase>)Class.forName(classname));
-    } catch (ClassNotFoundException e) { }
+      suite.addTestSuite((Class<? extends JDWPRawTestCase>) Class.forName(classname));
+    } catch (ClassNotFoundException e) {
+    }
   }
 
   public static junit.framework.Test suite() {
-    junit.framework.TestSuite suite = new junit.framework.TestSuite();
+    junit.framework.TestSuite baseSuite = new junit.framework.TestSuite();
 
     // All of these tests can only be run using the full JDWP implementation. They weren't really
     // used by IDEs/aren't really applicable to android so were never supported by the
     // -XjdwpProvider:internal JDWP implementation. The new agent based implementation supports them
     // though.
-    JPDATestOptions to = new JPDATestOptions();
-    if (to.getSuiteType().equals("full") || to.getSuiteType().equals("libjdwp")) {
+    Set<String> bad_test_cases = new HashSet<>();
+    bad_test_cases.addAll(Arrays.asList(TEST_OPTIONS.getBadTestCases()));
+    FilterSuite suite = (k) -> {
+      try {
+        baseSuite.addTest(
+            new FilteredTestSuite(
+                k,
+                (Method m) -> {
+                  String test_desc = m.getDeclaringClass().getName() + "#" + m.getName();
+                  return !bad_test_cases.contains(test_desc);
+                }));
+      } catch (BadTestClassException e) {
+        baseSuite.addTest(makeWarning(k.getName(), "Could not add test " + k + " due to " + e));
+      }
+    };
+    if (TEST_OPTIONS.getSuiteType().equals("full")
+        || TEST_OPTIONS.getSuiteType().equals("libjdwp")) {
       // I haven't yet found an IDE that will use these, but we might want to implement them anyway.
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.Events.MonitorContendedEnteredTest.class);
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.Events.MonitorContendedEnterTest.class);
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.Events.MonitorWaitedTest.class);
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.Events.MonitorWaitTest.class);
-
       // I don't know when these are ever used, but they're not obviously useless.
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.ReferenceType.NestedTypesTest.class);
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.VirtualMachine.HoldEventsTest.class);
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.VirtualMachine.ReleaseEventsTest.class);
-
       // Internal JDWP implementation never supported this.
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.ThreadReference.StopTest.class);
     }
@@ -60,7 +160,7 @@ public class AllTests {
     //
     // These tests are not worth fixing or fundamentally do not make sense on android.
     //
-    if (to.getSuiteType().equals("full")) {
+    if (TEST_OPTIONS.getSuiteType().equals("full")) {
       // It's not obvious how to translate this into our world, or what debuggers would do with it.
       suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.ReferenceType.ClassFileVersionTest.class);
       // TODO The test suite itself seems to send incorrect commands when this is run.
@@ -250,6 +350,7 @@ public class AllTests {
     addOptionalTestSuite(suite, "org.apache.harmony.jpda.tests.jdwp.DDM.DDMTest");
     suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.VMDebug.VMDebugTest.class);
     suite.addTestSuite(org.apache.harmony.jpda.tests.jdwp.VMDebug.VMDebugTest002.class);
-    return suite;
+
+    return baseSuite;
   }
 }
diff --git a/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/JPDATestOptions.java b/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/JPDATestOptions.java
index e6825b3..0d7841a 100644
--- a/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/JPDATestOptions.java
+++ b/jdwp/src/test/java/org/apache/harmony/jpda/tests/share/JPDATestOptions.java
@@ -25,6 +25,7 @@
  */
 package org.apache.harmony.jpda.tests.share;
 
+import java.util.Arrays;
 import org.apache.harmony.jpda.tests.framework.TestOptions;
 
 /**
@@ -40,6 +41,17 @@ import org.apache.harmony.jpda.tests.framework.TestOptions;
 public class JPDATestOptions extends TestOptions {
 
     /**
+     * Returns what test methods have been marked as bad and to be skipped.
+     *
+     * @return comma-split option "jpda.settings.badTestCases" or [] by default
+     */
+    public String[] getBadTestCases() {
+        return Arrays.stream(System.getProperty("jpda.settings.badTestCases", "").split(","))
+                     .filter((v) -> !v.equals(""))
+                     .toArray((i) -> new String[i]);
+    }
+
+    /**
      * Returns what test suite we should use if running AllTests. May be "internal" (for the test
      * suite corresponding to -XjdwpProvider:internal), "libjdwp" (for the test suite corresponding
      * to -XjdwpProvider:adbconnection) or "full" for everything.