Add a presubmit check to ensure sytnax of test filters files is valid.

This prints an error if:
- A line begins with '//' or '/*'
- A non-comment contains a whitespace
And prints a warning if:
- A filter file contains both inclusions and exclusions

Bug: 868000
Change-Id: I2a04504d3a17f4f7536b529a0e2e626eb6e61f66
Reviewed-on: https://chromium-review.googlesource.com/1153427
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578784}

testing/buildbot/filters/PRESUBMIT.py

@@ -7,6 +7,84 @@ See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into depot_tools.
 """
 
+import os
+import re
+
+
+def _CheckFilterFileFormat(input_api, output_api):
+  """This ensures all modified filter files are free of common syntax errors.
+
+  See the following for the correct syntax of these files:
+  https://chromium.googlesource.com/chromium/src/+/master/testing/buildbot/filters/README.md#file-syntax
+  As well as:
+  https://bit.ly/chromium-test-list-format
+  """
+  errors = []
+  warnings = []
+  for f in input_api.AffectedFiles():
+    filename = os.path.basename(f.LocalPath())
+    if not filename.endswith('.filter'):
+      # Non-filter files. Ignore these.
+      continue
+
+    inclusions = 0
+    exclusions = 0
+    for line_num, line in enumerate(f.NewContents()):
+      # Implicitly allow for trailing (but not leading) whitespace.
+      line = line.rstrip()
+      if not line:
+        # Empty line. Ignore these.
+        continue
+      if line.startswith('#'):
+        # A comment. Ignore these.
+        continue
+      if line.startswith('//') or line.startswith('/*'):
+        errors.append(
+            '%s:%d Not a valid comment syntax. Use "#" instead: "%s"' % (
+                filename, line_num, line))
+        continue
+      if not re.match(r'^\S+$', line):
+        errors.append(
+            '%s:%d Line must not contain whitespace: "%s"' % (
+                filename, line_num, line))
+        continue
+      if line[0] == '-':
+        exclusions += 1
+      else:
+        inclusions += 1
+
+    # If we have a mix of exclusions and inclusions, print a warning with a
+    # Y/N prompt to the author. Though this is a valid syntax, it's possible
+    # that such a combination will lead to a situation where zero tests are run.
+    if exclusions and inclusions:
+      warnings.append(
+          '%s: Contains both inclusions (%d) and exclusions (%d). This may '
+          'result in no tests running. Are you sure this is correct?' % (
+              filename, inclusions, exclusions))
+
+  res = []
+  if errors:
+    res.append(output_api.PresubmitError(
+        'Filter files do not follow the correct format:',
+        long_text='\n'.join(errors)))
+  if warnings:
+    res.append(output_api.PresubmitPromptWarning(
+        'Filter files may be incorrect:\n%s' % '\n'.join(warnings)))
+  return res
+
+
+def CommonChecks(input_api, output_api):
+  return _CheckFilterFileFormat(input_api, output_api)
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return CommonChecks(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return CommonChecks(input_api, output_api)
+
+
 def PostUploadHook(cl, change, output_api):
   """git cl upload will call this hook after the issue is created/modified.
 
@@ -23,4 +101,3 @@ def PostUploadHook(cl, change, output_api):
       'luci.chromium.try:linux_mojo'
     ],
     'Automatically added network service trybots to run tests on CQ.')
-

testing/buildbot/filters/README.md

@@ -82,4 +82,4 @@ Please use the following conventions when naming the new file:
 
 When adding a new file, please update `//testing/buildbot/filters/BUILD.gn`.
 
-[gtest_filter]: https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-a-subset-of-the-tests
+[gtest_filter]: https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#running-a-subset-of-the-tests

testing/buildbot/filters/chromeos.chromeos_unittests.filter

-TODO(crbug.com/865693): Fix this.
+# TODO(crbug.com/865693): Fix this.
 -NetworkConnectionHandlerImplTest.ConnectWithCertificateSuccess

testing/legion/lib/rpc/jsonrpclib.py

@@ -15,6 +15,9 @@ change to a minimum.
 This module only depends on default Python modules. No third party code is
 required to use this module.
 """
+
+# pylint: disable=no-value-for-parameter
+
 import json
 import urllib
 import xmlrpclib as _base