Add check for path regexps

This CL adds a check to //infra/config/branch/cq_cfg_presubmit.py to
check the path regexps in cq.cfg to make sure there is at least one file
that they could match with.

This CL also fixes a few typos which already exist in cq.cfg, as a
result of this missing checking.

Bug: 902507
Change-Id: I535d6671e4598eda27a0919ef14404a228398ea7
Reviewed-on: https://chromium-review.googlesource.com/c/1334940
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612756}

docs/infra/cq_builders.md

@@ -120,18 +120,20 @@ builder.
 * [linux-blink-gen-property-trees](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-blink-gen-property-trees) ([`cq.cfg` entry](https://cs.chromium.org/search/?q=package:%5Echromium$+file:cq.cfg+linux-blink-gen-property-trees)) ([matching builders](https://cs.chromium.org/search/?q=file:trybots.py+linux-blink-gen-property-trees))
 
   Path regular expressions:
-    * [`//third_party/blink/web_tests/FlagExpectations/enable-blink-gen-property-trees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-blink-gen-property-trees)
-    * [`//third_party/blink/web_tests/flag-specific/enable-blink-gen-property-trees/.+`](https://cs.chromium.org/chromium/src/third_party/blink/web_tests/flag-specific/enable-blink-gen-property-trees/)
+    * [`//third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees)
+    * [`//third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees)
 
 * [linux-blink-rel](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-blink-rel) ([`cq.cfg` entry](https://cs.chromium.org/search/?q=package:%5Echromium$+file:cq.cfg+linux-blink-rel)) ([matching builders](https://cs.chromium.org/search/?q=file:trybots.py+linux-blink-rel))
 
   Path regular expressions:
     * [`//cc/.+`](https://cs.chromium.org/chromium/src/cc/)
     * [`//third_party/blink/renderer/core/(svg|paint)/.+`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/renderer/core/(svg|paint)/)
-    * [`//third_party/blink/renderer/core/layout/compositing/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/compositing/)
+    * [`//third_party/blink/renderer/core/paint/compositing/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/compositing/)
     * [`//third_party/blink/renderer/platform/graphics/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/)
-    * [`//third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees)`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees))
-    * [`//third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/.+`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/)
+    * [`//third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees)
+    * [`//third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees)
+    * [`//third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2)
+    * [`//third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/.+`](https://cs.chromium.org/chromium/src/third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/)
 
 * [linux_chromium_dbg_ng](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_dbg_ng) ([`cq.cfg` entry](https://cs.chromium.org/search/?q=package:%5Echromium$+file:cq.cfg+linux_chromium_dbg_ng)) ([matching builders](https://cs.chromium.org/search/?q=file:trybots.py+linux_chromium_dbg_ng))
 
@@ -150,11 +152,13 @@ builder.
 * [linux_layout_tests_slimming_paint_v2](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_layout_tests_slimming_paint_v2) ([`cq.cfg` entry](https://cs.chromium.org/search/?q=package:%5Echromium$+file:cq.cfg+linux_layout_tests_slimming_paint_v2)) ([matching builders](https://cs.chromium.org/search/?q=file:trybots.py+linux_layout_tests_slimming_paint_v2))
 
   Path regular expressions:
-    * [`//third_party/blink/renderer/core/layout/compositing/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/compositing/)
+    * [`//third_party/blink/renderer/core/paint/compositing/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/compositing/)
     * [`//third_party/blink/renderer/core/(svg|paint)/.+`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/renderer/core/(svg|paint)/)
     * [`//third_party/blink/renderer/platform/graphics/.+`](https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/)
-    * [`//third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees)`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees))
-    * [`//third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/.+`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/)
+    * [`//third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees)
+    * [`//third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees)
+    * [`//third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2`](https://cs.chromium.org/search/?q=package:%5Echromium$+file:third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2)
+    * [`//third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/.+`](https://cs.chromium.org/chromium/src/third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/)
 
 * [linux_mojo](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_mojo) ([`cq.cfg` entry](https://cs.chromium.org/search/?q=package:%5Echromium$+file:cq.cfg+linux_mojo)) ([matching builders](https://cs.chromium.org/search/?q=file:trybots.py+linux_mojo))
 

infra/config/PRESUBMIT.py

@@ -3,30 +3,34 @@
 # found in the LICENSE file.
 
 
-def _CheckForTranslations(input_api, output_api):
-  del output_api
-
-  return []
-
 def _CommonChecks(input_api, output_api):
   commands = []
   # TODO(martiniss): Move this check to the root presubmit. It checks to ensure
   # that path regexps in cq.cfg reference something locally.
+  touches_cq = False
   for f in input_api.AffectedFiles():
     local_path = f.LocalPath()
     if local_path.endswith('cq.cfg'):
-      commands.append(
-        input_api.Command(
-          name='cq.cfg presubmit', cmd=[
-              input_api.python_executable, 'branch/cq_cfg_presubmit.py',
-              '--check'],
-          kwargs={}, message=output_api.PresubmitError),
-      )
+      touches_cq = True
+
+  if touches_cq:
+    commands.append(
+      input_api.Command(
+        name='cq.cfg presubmit', cmd=[
+            input_api.python_executable, 'branch/cq_cfg_presubmit.py',
+            '--check'],
+        kwargs={}, message=output_api.PresubmitError),
+    )
 
   results = []
 
   results.extend(input_api.canned_checks.CheckChangedLUCIConfigs(
       input_api, output_api))
+
+  commands.extend(input_api.canned_checks.GetUnitTestsRecursively(
+      input_api, output_api,
+      input_api.os_path.join(input_api.PresubmitLocalPath()),
+      whitelist=[r'.+_unittest\.py$'], blacklist=[]))
   results.extend(input_api.RunTests(commands))
   return results
 

infra/config/branch/cq.cfg

@@ -129,17 +129,19 @@ verifiers {
       }
       builders {
         name: "linux-blink-gen-property-trees"
-        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-blink-gen-property-trees"
-        path_regexp: "third_party/blink/web_tests/flag-specific/enable-blink-gen-property-trees/.+"
+        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees"
+        path_regexp: "third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees"
       }
       builders {
         name: "linux-blink-rel"
         path_regexp: "cc/.+"
         path_regexp: "third_party/blink/renderer/core/(svg|paint)/.+"
-        path_regexp: "third_party/blink/renderer/core/layout/compositing/.+"
+        path_regexp: "third_party/blink/renderer/core/paint/compositing/.+"
         path_regexp: "third_party/blink/renderer/platform/graphics/.+"
-        path_regexp: "third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees)"
-        path_regexp: "third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/.+"
+        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees"
+        path_regexp: "third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees"
+        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2"
+        path_regexp: "third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/.+"
       }
       builders {
         name: "linux_chromium_dbg_ng"
@@ -155,11 +157,13 @@ verifiers {
       }
       builders {
         name: "linux_layout_tests_slimming_paint_v2"
-        path_regexp: "third_party/blink/renderer/core/layout/compositing/.+"
+        path_regexp: "third_party/blink/renderer/core/paint/compositing/.+"
         path_regexp: "third_party/blink/renderer/core/(svg|paint)/.+"
         path_regexp: "third_party/blink/renderer/platform/graphics/.+"
-        path_regexp: "third_party/blink/web_tests/FlagExpectations/(enable-slimming-paint-v2|enable-blink-gen-property-trees)"
-        path_regexp: "third_party/blink/web_tests/flag-specific/(enable-slimming-paint-v2|enable-blink-gen-property-trees)/.+"
+        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees"
+        path_regexp: "third_party/blink/web_tests/flag-specific/enable-features=BlinkGenPropertyTrees"
+        path_regexp: "third_party/blink/web_tests/FlagExpectations/enable-slimming-paint-v2"
+        path_regexp: "third_party/blink/web_tests/flag-specific/enable-slimming-paint-v2/.+"
       }
       builders {
         name: "linux_mojo"

infra/config/branch/cq_cfg_presubmit.py

@@ -5,6 +5,7 @@
 
 import argparse
 import difflib
+import re
 import os
 import string
 import sys
@@ -181,6 +182,16 @@ class CQConfig(object):
 
     return CQConfig(lines)
 
+  def get_path_regexps(self):
+    _, opt, _ = self.builder_list().by_section()
+    for b in opt:
+      if 'path_regexp' in b:
+        for reg in b['path_regexp']:
+          yield reg
+      if 'path_regexp_exclude' in b:
+        for reg in b['path_regexp_exclude']:
+          yield reg
+
   @property
   def version(self):
     return int(self._value['version'][0])
@@ -253,6 +264,50 @@ class CQConfig(object):
 
     return '\n'.join(lines)
 
+def verify_path_regexps(regexps, verbose=True):
+  # Verify that all the regexps listed in the file have files which they could
+  # be triggered by. Failing this usually means they're old, and the code was
+  # moved somewhere, like the webkit->blink rename.
+  invalid_regexp = False
+  for regexp in regexps:
+    regexp = regexp.replace('\\\\', '')
+    # Split by path name, so that we don't have to run os.walk on the entire
+    # source tree. cq.cfg always uses '/' as the path separator.
+    parts = regexp.split('/')
+    # Dash and equal sign are used by layout tests.
+    simple_name_re = re.compile(r'^[a-zA-Z0-9_\-=]*$')
+    last_normal_path = 0
+    while last_normal_path < len(parts):
+      itm = parts[last_normal_path]
+      if not simple_name_re.match(itm):
+        break
+      last_normal_path += 1
+    path_to_search = os.path.sep.join(parts[:last_normal_path])
+    # Simple case. Regexp is just referencing a single file. Just check if the
+    # file exists.
+    if path_to_search == regexp and os.path.exists(os.path.join(
+        CHROMIUM_DIR, path_to_search)):
+      continue
+
+    compiled_regexp = re.compile(regexp)
+    found = False
+    for root, _, files in os.walk(os.path.join(CHROMIUM_DIR, path_to_search)):
+      for fname in files:
+        fullname = os.path.relpath(os.path.join(root, fname), CHROMIUM_DIR)
+        if compiled_regexp.match(fullname):
+          found = True
+          break
+      if found:
+        break
+    if not found:
+      if verbose:
+        print (
+            'Regexp %s appears to have no valid files which could match it.' % (
+                regexp))
+      invalid_regexp = True
+
+  return not invalid_regexp
+
 
 def main():
   parser = argparse.ArgumentParser()
@@ -288,6 +343,9 @@ def main():
     exit_code = 1
 
   if args.check:
+    if not verify_path_regexps(cfg.get_path_regexps()):
+      exit_code = 1
+
     # TODO(martiniss): Add a check for path_regexp, to make sure they're valid
     # paths.
     with open(os.path.join(
@@ -295,7 +353,7 @@ def main():
       if cfg.get_markdown_doc() != f.read():
         print (
             'Markdown file is out of date. Please run '
-            '`//infra/config/branch/cq_cfg_presubmit.py to regenerate the '
+            '`//infra/config/branch/cq_cfg_presubmit.py` to regenerate the '
             'docs.')
         exit_code = 1
   else:

infra/config/branch/cq_cfg_presubmit_unittest.py

+#!/usr/bin/env python
+# Copyright (c) 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""Tests for cq_cfg_presubmit."""
+
+import mock
+import os
+import unittest
+
+import cq_cfg_presubmit
+
+
+class CqCfgPresubmitTest(unittest.TestCase):
+  def test_verify_path_regexp_exists(self):
+    with mock.patch('cq_cfg_presubmit.os.path.exists') as exists:
+      exists.side_effect = [True]
+      self.assertTrue(cq_cfg_presubmit.verify_path_regexps([
+          'simple/file',
+      ]))
+
+  def test_verify_path_regexp_os_walk_found(self):
+    with mock.patch('cq_cfg_presubmit.os.walk') as walk:
+      walk.side_effect = [(
+          (os.path.join(cq_cfg_presubmit.CHROMIUM_DIR, 'random'),
+            None, ['test.txt'],),
+          (os.path.join(cq_cfg_presubmit.CHROMIUM_DIR, 'simple/file'),
+           None, ['test.txt'],),
+      )]
+      with mock.patch('cq_cfg_presubmit.os.path.exists') as exists:
+        exists.side_effect = [False]
+        self.assertTrue(cq_cfg_presubmit.verify_path_regexps([
+            'simple/file/.+',
+        ], False))
+
+  def test_verify_path_regexp_os_walk_not_found(self):
+    with mock.patch('cq_cfg_presubmit.os.walk') as walk:
+      walk.side_effect = [(
+          (os.path.join(cq_cfg_presubmit.CHROMIUM_DIR, 'random'),
+            None, ['test.txt'],),
+      )]
+      with mock.patch('cq_cfg_presubmit.os.path.exists') as exists:
+        exists.side_effect = [False]
+        self.assertFalse(cq_cfg_presubmit.verify_path_regexps([
+            'simple/file/.+',
+        ], False))
+
+
+if __name__ == '__main__':
+  unittest.main()