Merge "Add verification of knownfailures in the testrunner"

2 files changed, 56 insertions, 47 deletions

diff --git a/test/knownfailures.json b/test/knownfailures.json
index 5923ebd6c6..b3a4e1be40 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1,12 +1,12 @@
 [
     {
-        "test": "153-reference-stress",
+        "tests": "153-reference-stress",
         "description": ["Disable 153-reference-stress temporarily until a fix",
                         "arrives."],
         "bug": "http://b/33389022"
     },
     {
-        "test": "080-oom-fragmentation",
+        "tests": "080-oom-fragmentation",
         "description": "Disable 080-oom-fragmentation due to flakes.",
         "bug": "http://b/33795328"
     },
@@ -21,7 +21,7 @@
         "bug": "http://b/34193123"
     },
     {
-        "test": "149-suspend-all-stress",
+        "tests": "149-suspend-all-stress",
         "description": "Disable 149-suspend-all-stress, its output is flaky",
         "bug": "http://b/28988206"
     },
@@ -34,13 +34,13 @@
                         "loaded systems."]
     },
     {
-        "test": "147-stripped-dex-fallback",
+        "tests": "147-stripped-dex-fallback",
         "variant": "target",
         "description": ["147-stripped-dex-fallback isn't supported on device",
                         "because --strip-dex  requires the zip command."]
     },
     {
-        "test": "569-checker-pattern-replacement",
+        "tests": "569-checker-pattern-replacement",
         "variant": "target",
         "description": ["569-checker-pattern-replacement tests behaviour",
                         "present only on host."]
@@ -54,13 +54,7 @@
                         "doesn't (and isn't meant to) work with --prebuild."]
     },
     {
-        "test": "554-jit-profile-file",
-        "variant": "no-prebuild | interpreter",
-        "description": ["554-jit-profile-file is disabled because it needs a",
-                        "primary oat file to know what it should save."]
-    },
-    {
-        "tests": ["529-checker-unresolved", "555-checker-regression-x86const"],
+        "tests": ["529-checker-unresolved"],
         "variant": "no-prebuild",
         "bug": "http://b/27784033"
     },
@@ -73,27 +67,26 @@
     {
         "tests": ["117-nopatchoat",
                   "118-noimage-dex2oat",
-                  "119-noimage-patchoat",
-                  "554-jit-profile-file"],
+                  "119-noimage-patchoat"],
         "variant": "no-relocate",
         "description": ["117-nopatchoat is not broken per-se it just doesn't",
                         "work (and isn't meant to) without --prebuild",
                         "--relocate"]
     },
     {
-        "test": "137-cfi",
+        "tests": "137-cfi",
         "variant": "interp-ac",
         "description": ["Temporarily disable some broken tests when forcing",
                         "access checks in interpreter"],
         "bug": "http://b/22414682"
     },
     {
-        "test" : "629-vdex-speed",
+        "tests" : "629-vdex-speed",
         "variant": "interp-ac | no-dex2oat | interpreter | jit | relocate-npatchoat",
         "description": "629 requires compilation."
     },
     {
-        "test": "137-cfi",
+        "tests": "137-cfi",
         "variant": "gcstress",
         "description": ["137-cfi needs to unwind a second forked process. We're",
                         "using a primitive sleep to wait till we hope the",
@@ -101,7 +94,7 @@
                         "slowness of gcstress makes this bad."]
     },
     {
-        "test": "152-dead-large-object",
+        "tests": "152-dead-large-object",
         "variant": "gcstress",
         "description": ["152-dead-large-object requires a heap larger than what gcstress uses."],
         "bug": "http://b/35800768"
@@ -115,7 +108,7 @@
                         "non-deterministic. Same for 913."]
     },
     {
-        "test": "961-default-iface-resolution-gen",
+        "tests": "961-default-iface-resolution-gen",
         "variant": "gcstress",
         "description": ["961-default-iface-resolution-gen and",
                         "964-default-iface-init-genare very long tests that",
@@ -125,25 +118,25 @@
                         "lot."]
     },
     {
-        "test": "964-default-iface-init-gen",
+        "tests": "964-default-iface-init-gen",
         "variant": "gcstress"
     },
     {
-        "test": "154-gc-loop",
+        "tests": "154-gc-loop",
         "variant": "gcstress | jit & debug",
         "description": ["154-gc-loop depends GC not happening too often"],
         "bug": "http://b/35917229"
     },
     {
-        "test": "115-native-bridge",
+        "tests": "115-native-bridge",
         "variant": "target",
         "description": ["115-native-bridge setup is complicated. Need to",
                         "implement it correctly for the target."]
     },
     {
-        "test": "130-hprof",
+        "tests": "130-hprof",
         "variant": "target",
-        "desription": ["130-hprof dumps the heap and runs hprof-conv to check",
+        "description": ["130-hprof dumps the heap and runs hprof-conv to check",
                        "whether the file is somewhat readable. Thi is only",
                        "possible on the host. TODO: Turn off all the other",
                        "combinations, this is more about testing actual ART",
@@ -151,7 +144,7 @@
                        "complete test) JDWP must be set up."]
     },
     {
-        "test": "131-structural-change",
+        "tests": "131-structural-change",
         "variant": "debug",
         "description": ["131 is an old test. The functionality has been",
                         "implemented at an earlier stage and is checked",
@@ -160,25 +153,19 @@
                         "punt to interpreter"]
     },
     {
-        "test": "138-duplicate-classes-check",
+        "tests": "138-duplicate-classes-check",
         "variant": "ndebug",
         "description": ["Turned on for debug builds since debug builds have",
                         "duplicate classes checks enabled"],
         "bug": "http://b/2133391"
     },
     {
-        "test": "147-stripped-dex-fallback",
+        "tests": "147-stripped-dex-fallback",
         "variant": "no-dex2oat | no-image | relocate-npatchoat",
         "description": ["147-stripped-dex-fallback is disabled because it",
                         "requires --prebuild."]
     },
     {
-        "test": "554-jit-profile-file",
-        "variant": "no-dex2oat | no-image | relocate-npatchoat",
-        "description": ["554-jit-profile-file is disabled because it needs a",
-                        "primary oat file to know what it should save."]
-    },
-    {
         "tests": ["116-nodex2oat",
                   "117-nopatchoat",
                   "118-noimage-dex2oat",
@@ -197,14 +184,14 @@
                   "138-duplicate-classes-check",
                   "018-stack-overflow",
                   "961-default-iface-resolution-gen",
-                  "964-default-iface-init"],
+                  "964-default-iface-init-gen"],
         "variant": "no-image",
         "description": ["This test fails without an image. 018, 961, 964 often",
                         "time out."],
         "bug": "http://b/34369284"
     },
     {
-        "test": "137-cfi",
+        "tests": "137-cfi",
         "description": ["This test unrolls and expects managed frames, but",
                         "tracing means we run the interpreter."],
         "variant": "trace | stream"
@@ -219,7 +206,7 @@
         "variant": "trace | stream"
     },
     {
-        "test": "130-hprof",
+        "tests": "130-hprof",
         "description": "130 occasional timeout",
         "bug": "http://b/32383962",
         "variant": "trace | stream"
@@ -240,14 +227,14 @@
                         "suppressed when tracing."]
     },
     {
-        "test": "137-cfi",
+        "tests": "137-cfi",
         "description": ["CFI unwinding expects managed frames, and the test",
                         "does not iterate enough to even compile. JIT also",
                         "uses Generic JNI instead of the JNI compiler."],
         "variant": "interpreter | jit"
     },
     {
-        "test": "906-iterate-heap",
+        "tests": "906-iterate-heap",
         "description": ["Test 906 iterates the heap filtering with different",
                         "options. No instances should be created between those",
                         "runs to be able to have precise checks."],
@@ -275,22 +262,22 @@
         "variant": "optimizing &  ndebuggable | regalloc_gc & ndebuggable"
     },
     {
-        "test": "596-app-images",
+        "tests": "596-app-images",
         "variant": "npictest"
     },
     {
-        "test": "055-enum-performance",
+        "tests": "055-enum-performance",
         "variant": "optimizing | regalloc_gc",
         "description": ["055: Exceeds run time limits due to heap poisoning ",
                         "instrumentation (on ARM and ARM64 devices)."]
     },
     {
-        "test": "909-attach-agent",
+        "tests": "909-attach-agent",
         "variant": "debuggable",
         "description": "Tests that check semantics for a non-debuggable app."
     },
     {
-        "test": "137-cfi",
+        "tests": "137-cfi",
         "variant": "debuggable",
         "description": ["The test relies on AOT code and debuggable makes us",
                         "JIT always."]
@@ -330,7 +317,7 @@
         "variant": "optimizing | regalloc_gc"
     },
     {
-        "test": "055-enum-performance",
+        "tests": "055-enum-performance",
         "description": ["The test tests performance which degrades during",
                         "bisecting."],
         "env_vars": {"ART_TEST_BISECTION": "true"},
diff --git a/test/testrunner/testrunner.py b/test/testrunner/testrunner.py
index be84f898b2..a5504098d8 100755
--- a/test/testrunner/testrunner.py
+++ b/test/testrunner/testrunner.py
@@ -547,6 +547,22 @@ def print_test_info(test_name, result, failed_test_info=""):
   finally:
     print_mutex.release()
 
+def verify_knownfailure_entry(entry):
+  supported_field = {
+      'tests' : (list, unicode),
+      'description' : (list, unicode),
+      'bug' : (unicode,),
+      'variant' : (unicode,),
+      'env_vars' : (dict,),
+  }
+  for field in entry:
+    field_type = type(entry[field])
+    if field_type not in supported_field[field]:
+      raise ValueError('%s is not supported type for %s\n%s' % (
+          str(field_type),
+          field,
+          str(entry)))
+
 def get_disabled_test_info():
   """Generate set of known failures.
 
@@ -563,15 +579,18 @@ def get_disabled_test_info():
 
   disabled_test_info = {}
   for failure in known_failures_info:
-    tests = failure.get('test')
-    if tests:
+    verify_knownfailure_entry(failure)
+    tests = failure.get('tests', [])
+    if isinstance(tests, unicode):
       tests = [tests]
-    else:
-      tests = failure.get('tests', [])
     variants = parse_variants(failure.get('variant'))
     env_vars = failure.get('env_vars')
+
     if check_env_vars(env_vars):
       for test in tests:
+        if test not in RUN_TEST_SET:
+          raise ValueError('%s is not a valid run-test' % (
+              test))
         if test in disabled_test_info:
           disabled_test_info[test] = disabled_test_info[test].union(variants)
         else:
@@ -635,6 +654,9 @@ def parse_variants(variants):
     variant = set()
     for and_variant in and_variants:
       and_variant = and_variant.strip()
+      if and_variant not in TOTAL_VARIANTS_SET:
+        raise ValueError('%s is not a valid variant' % (
+            and_variant))
       variant.add(and_variant)
     variant_list.add(frozenset(variant))
   return variant_list