Add git cl presubmit --files

Chromium's presubmits contain a lot of latent errors - presubmit errors
that will trigger the next time a file is modified, but have been
waiting for years. Flushing those out with "git cl presubmit --all"
works poorly because it exercises all presubmits simultaneously, which
is too slow and triggers too many failures. Adding a --files option lets
small areas of the tree or specific file types to be exercised in a
controlled manner.

Bug: 1311697
Change-Id: I36ec6a759a80000d6ed4a8cc218ece327d45f8d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3559174
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

git_cl.py

@@ -1333,8 +1333,17 @@ class Changelist(object):
 
     return args
 
-  def RunHook(self, committing, may_prompt, verbose, parallel, upstream,
-              description, all_files, resultdb=False, realm=None):
+  def RunHook(self,
+              committing,
+              may_prompt,
+              verbose,
+              parallel,
+              upstream,
+              description,
+              all_files,
+              files=None,
+              resultdb=False,
+              realm=None):
     """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
     args = self._GetCommonPresubmitArgs(verbose, upstream)
     args.append('--commit' if committing else '--upload')
@@ -1344,6 +1353,9 @@ class Changelist(object):
       args.append('--parallel')
     if all_files:
       args.append('--all_files')
+    if files:
+      args.extend(files.split(';'))
+      args.append('--source_controlled_only')
 
     if resultdb and not realm:
       # TODO (crbug.com/1113463): store realm somewhere and look it up so
@@ -4064,6 +4076,11 @@ def CMDpresubmit(parser, args):
                     help='Run checks even if tree is dirty')
   parser.add_option('--all', action='store_true',
                     help='Run checks against all files, not just modified ones')
+  parser.add_option('--files',
+                    nargs=1,
+                    help='Semicolon-separated list of files to be marked as '
+                    'modified when executing presubmit or post-upload hooks. '
+                    'fnmatch wildcards can also be used.')
   parser.add_option('--parallel', action='store_true',
                     help='Run all tests specified by input_api.RunTests in all '
                          'PRESUBMIT files in parallel.')
@@ -4089,16 +4106,16 @@ def CMDpresubmit(parser, args):
   else:
     description = _create_description_from_log([base_branch])
 
-  cl.RunHook(
-      committing=not options.upload,
-      may_prompt=False,
-      verbose=options.verbose,
-      parallel=options.parallel,
-      upstream=base_branch,
-      description=description,
-      all_files=options.all,
-      resultdb=options.resultdb,
-      realm=options.realm)
+  cl.RunHook(committing=not options.upload,
+             may_prompt=False,
+             verbose=options.verbose,
+             parallel=options.parallel,
+             upstream=base_branch,
+             description=description,
+             all_files=options.all,
+             files=options.files,
+             resultdb=options.resultdb,
+             realm=options.realm)
   return 0
 
 

presubmit_support.py

@@ -1831,7 +1831,17 @@ def _parse_change(parser, options):
     parser.error('<files> is not optional for unversioned directories.')
 
   if options.files:
-    change_files = _parse_files(options.files, options.recursive)
+    if options.source_controlled_only:
+      # Get the filtered set of files from SCM.
+      change_files = []
+      for name in scm.GIT.GetAllFiles(options.root):
+        for mask in options.files:
+          if fnmatch.fnmatch(name, mask):
+            change_files.append(('M', name))
+            break
+    else:
+      # Get the filtered set of files from a directory scan.
+      change_files = _parse_files(options.files, options.recursive)
   elif options.all_files:
     change_files = [('M', f) for f in scm.GIT.GetAllFiles(options.root)]
   else:
@@ -1956,6 +1966,8 @@ def main(argv=None):
                       help='List of files to be marked as modified when '
                       'executing presubmit or post-upload hooks. fnmatch '
                       'wildcards can also be used.')
+  parser.add_argument('--source_controlled_only', action='store_true',
+                      help='Constrain \'files\' to those in source control.')
   parser.add_argument('--use-python3', action='store_true',
                       help='Use python3 for presubmit checks by default')
   options = parser.parse_args(argv)

tests/git_cl_test.py

@@ -3375,6 +3375,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase):
         upstream='upstream',
         description='fetch description',
         all_files=None,
+        files=None,
         resultdb=None,
         realm=None)
 
@@ -3389,6 +3390,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase):
         upstream='upstream',
         description='get description',
         all_files=None,
+        files=None,
         resultdb=None,
         realm=None)
 
@@ -3402,6 +3404,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase):
         upstream='custom_branch',
         description='fetch description',
         all_files=None,
+        files=None,
         resultdb=None,
         realm=None)
 
@@ -3417,6 +3420,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase):
         upstream='upstream',
         description='fetch description',
         all_files=True,
+        files=None,
         resultdb=True,
         realm='chromium:public')
 

tests/presubmit_unittest.py

@@ -1007,7 +1007,7 @@ def CheckChangeOnCommit(input_api, output_api):
   def testParseChange_Files(self):
     presubmit._parse_files.return_value=[('M', 'random_file.txt')]
     scm.determine_scm.return_value = None
-    options = mock.Mock(all_files=False)
+    options = mock.Mock(all_files=False, source_controlled_only = False)
 
     change = presubmit._parse_change(None, options)
     self.assertEqual(presubmit.Change.return_value, change)
@@ -1049,7 +1049,7 @@ def CheckChangeOnCommit(input_api, output_api):
   def testParseChange_FilesAndGit(self):
     scm.determine_scm.return_value = 'git'
     presubmit._parse_files.return_value = [('M', 'random_file.txt')]
-    options = mock.Mock(all_files=False)
+    options = mock.Mock(all_files=False, source_controlled_only = False)
 
     change = presubmit._parse_change(None, options)
     self.assertEqual(presubmit.GitChange.return_value, change)