[test+presubmit] Remove duplicate test status file entries

And add a presubmit check to guard against future duplicates.

R=machenbach@chromium.org

Review URL: https://codereview.chromium.org/1477403002

Cr-Commit-Position: refs/heads/master@{#32380}

test/mjsunit/mjsunit.status

@@ -302,7 +302,6 @@
   'array-feedback': [SKIP],
   'array-literal-feedback': [SKIP],
   'd8-performance-now': [SKIP],
-  'debug-stepout-scope-part8': [PASS, ['arch == arm ', FAIL]],
   'elements-kind': [SKIP],
   'elements-transition-hoisting': [SKIP],
   'fast-prototype': [SKIP],
@@ -411,11 +410,6 @@
   'regress/regress-91013': [SKIP],
   'regress/regress-99167': [SKIP],
 
-  # Long running tests.
-  'regress/regress-2185': [PASS, ['mode == debug', PASS, TIMEOUT]],
-  'regress/regress-2185-2': [PASS, TIMEOUT],
-  'whitespaces': [PASS, TIMEOUT, SLOW],
-
   # BUG(v8:3457).
   'deserialize-reference': [PASS, FAIL],
 
@@ -453,6 +447,7 @@
   'unicodelctest-no-optimization': [PASS, SLOW],
   'unicodelctest': [PASS, SLOW],
   'unicode-test': [PASS, SLOW],
+  'whitespaces': [PASS, TIMEOUT, SLOW],
 }],  # 'arch == arm64'
 
 ['arch == arm64 and mode == debug and simulator_run == True', {
@@ -501,7 +496,7 @@
   'try': [PASS, ['mode == debug', SKIP]],
   'debug-scripts-request': [PASS, ['mode == debug', SKIP]],
   'array-constructor': [PASS, ['mode == debug', SKIP]],
-  'regress/regress-1122': [PASS, ['mode == debug and arch == android_arm', SKIP]],
+  'regress/regress-1122': [PASS, SLOW, ['mode == debug and arch == android_arm', SKIP]],
 
   # Flaky test that can hit compilation-time stack overflow in debug mode.
   'unicode-test': [PASS, ['mode == debug', PASS, FAIL]],
@@ -510,10 +505,6 @@
   'compiler/regress-stacktrace-methods': [PASS, ['mode == release', TIMEOUT]],
   'array-splice': [PASS, TIMEOUT],
 
-  # Long running test.
-  'string-indexof-2': [PASS, TIMEOUT],
-  'mirror-object': [PASS, TIMEOUT],
-
   # Long running tests. Skipping because having them timeout takes too long on
   # the buildbot.
   'big-object-literal': [SKIP],
@@ -531,17 +522,16 @@
   # Currently always deopt on minus zero
   'math-floor-of-div-minus-zero': [SKIP],
 
-  ############################################################################
   # Slow tests.
-  'regress/regress-2185-2': [PASS, SLOW],
-  'mirror-object': [PASS, SLOW],
-  'compiler/osr-with-args': [PASS, SLOW],
   'array-sort': [PASS, SLOW],
+  'compiler/osr-with-args': [PASS, SLOW],
+  'mirror-object': [PASS, SLOW],
   'packed-elements': [PASS, SLOW],
-  'regress/regress-91008': [PASS, SLOW],
+  'regress/regress-2185-2': [PASS, SLOW],
   'regress/regress-2790': [PASS, SLOW],
+  'regress/regress-91008': [PASS, SLOW],
   'regress/regress-json-stringify-gc': [PASS, SLOW],
-  'regress/regress-1122': [PASS, SLOW],
+  'string-indexof-2': [PASS, TIMEOUT],
 }],  # 'arch == arm or arch == android_arm'
 
 ##############################################################################

test/test262/test262.status

@@ -64,7 +64,7 @@
 
   # https://code.google.com/p/v8/issues/detail?id=3305
   # This times out in sloppy mode because sloppy const assignment does not throw.
-  'language/statements/const/syntax/const-invalid-assignment-next-expression-for': [PASS, FAIL, TIMEOUT],
+  'language/statements/const/syntax/const-invalid-assignment-next-expression-for': [SKIP],
 
   # https://code.google.com/p/v8/issues/detail?id=1543
   'built-ins/Proxy/*': [FAIL],
@@ -399,10 +399,8 @@
   'intl402/Collator/10.1.2_a': [PASS, FAIL],
   'intl402/Collator/10.2.3_b': [PASS, FAIL],
   'intl402/Collator/prototype/10.3_a': [FAIL],
-  'intl402/Date/prototype/13.3.0_7': [FAIL],
   'intl402/DateTimeFormat/12.1.1': [FAIL],
   'intl402/DateTimeFormat/12.1.1_a': [FAIL],
-  'intl402/DateTimeFormat/12.1.1_1': [FAIL],
   'intl402/DateTimeFormat/12.1.2': [PASS, FAIL],
   'intl402/DateTimeFormat/12.1.2.1_4': [FAIL],
   'intl402/DateTimeFormat/12.2.3_b': [FAIL],
@@ -486,7 +484,6 @@
   'language/literals/regexp/S7.8.5_A1.4_T2': [SKIP],
   'language/literals/regexp/S7.8.5_A2.1_T2': [SKIP],
   'language/literals/regexp/S7.8.5_A2.4_T2': [SKIP],
-  'language/statements/const/syntax/const-invalid-assignment-next-expression-for': [SKIP],
   'built-ins/Array/prototype/slice/S15.4.4.10_A3_T1': [SKIP],
   'built-ins/Array/prototype/slice/S15.4.4.10_A3_T2': [SKIP],
 }],  # ALWAYS

tools/presubmit.py

@@ -35,6 +35,7 @@ except ImportError, e:
   md5er = md5.new
 
 
+import json
 import optparse
 import os
 from os.path import abspath, join, dirname, basename, exists
@@ -407,16 +408,60 @@ def CheckExternalReferenceRegistration(workspace):
       [sys.executable, join(workspace, "tools", "external-reference-check.py")])
   return code == 0
 
+
+def _CheckStatusFileForDuplicateKeys(filepath):
+  comma_space_bracket = re.compile(", *]")
+  lines = []
+  with open(filepath) as f:
+    for line in f.readlines():
+      # Skip all-comment lines.
+      if line.lstrip().startswith("#"): continue
+      # Strip away comments at the end of the line.
+      comment_start = line.find("#")
+      if comment_start != -1:
+        line = line[:comment_start]
+      line = line.strip()
+      # Strip away trailing commas within the line.
+      line = comma_space_bracket.sub("]", line)
+      if len(line) > 0:
+        lines.append(line)
+
+  # Strip away trailing commas at line ends. Ugh.
+  for i in range(len(lines) - 1):
+    if (lines[i].endswith(",") and len(lines[i + 1]) > 0 and
+        lines[i + 1][0] in ("}", "]")):
+      lines[i] = lines[i][:-1]
+
+  contents = "\n".join(lines)
+  # JSON wants double-quotes.
+  contents = contents.replace("'", '"')
+  # Fill in keywords (like PASS, SKIP).
+  for key in statusfile.KEYWORDS:
+    contents = re.sub(r"\b%s\b" % key, "\"%s\"" % key, contents)
+
+  status = {"success": True}
+  def check_pairs(pairs):
+    keys = {}
+    for key, value in pairs:
+      if key in keys:
+        print("%s: Error: duplicate key %s" % (filepath, key))
+        status["success"] = False
+      keys[key] = True
+
+  json.loads(contents, object_pairs_hook=check_pairs)
+  return status["success"]
+
 def CheckStatusFiles(workspace):
+  success = True
   suite_paths = utils.GetSuitePaths(join(workspace, "test"))
   for root in suite_paths:
     suite_path = join(workspace, "test", root)
     status_file_path = join(suite_path, root + ".status")
     suite = testsuite.TestSuite.LoadTestSuite(suite_path)
     if suite and exists(status_file_path):
-      if not statusfile.PresubmitCheck(status_file_path):
+      success &= statusfile.PresubmitCheck(status_file_path)
-        return False
+      success &= _CheckStatusFileForDuplicateKeys(status_file_path)
-  return True
+  return success
 
 def CheckAuthorizedAuthor(input_api, output_api):
   """For non-googler/chromites committers, verify the author's email address is
@@ -460,12 +505,12 @@ def Main():
   success = True
   print "Running C++ lint check..."
   if not options.no_lint:
-    success = CppLintProcessor().Run(workspace) and success
+    success &= CppLintProcessor().Run(workspace)
   print "Running copyright header, trailing whitespaces and " \
         "two empty lines between declarations check..."
-  success = SourceProcessor().Run(workspace) and success
+  success &= SourceProcessor().Run(workspace)
-  success = CheckExternalReferenceRegistration(workspace) and success
+  success &= CheckExternalReferenceRegistration(workspace)
-  success = CheckStatusFiles(workspace) and success
+  success &= CheckStatusFiles(workspace)
   if success:
     return 0
   else:

tools/testrunner/local/statusfile.py

@@ -156,20 +156,25 @@ def ReadStatusFile(path, variables):
 def PresubmitCheck(path):
   contents = ReadContent(path)
   root_prefix = os.path.basename(os.path.dirname(path)) + "/"
-
+  status = {"success": True}
+  def _assert(check, message):  # Like "assert", but doesn't throw.
+    if not check:
+      print("%s: Error: %s" % (path, message))
+      status["success"] = False
   try:
     for section in contents:
-      assert type(section) == list
+      _assert(type(section) == list, "Section must be a list")
-      assert len(section) == 2
+      _assert(len(section) == 2, "Section list must have exactly 2 entries")
       section = section[1]
-      assert type(section) == dict
+      _assert(type(section) == dict,
+              "Second entry of section must be a dictionary")
       for rule in section:
-        assert type(rule) == str
+        _assert(type(rule) == str, "Rule key must be a string")
-        assert not rule.startswith(root_prefix), (
+        _assert(not rule.startswith(root_prefix),
-            "Suite name prefix must not be used in status files")
+                "Suite name prefix must not be used in rule keys")
-        assert not rule.endswith('.js'), (
+        _assert(not rule.endswith('.js'),
-            ".js extension must not be used in status files.")
+                ".js extension must not be used in rule keys.")
-    return True
+    return status["success"]
   except Exception as e:
     print e
     return False