presubmit_support: Add --all-files options.

Will be set when --all is passed to git-cl presubmit to execute
presubmit checks over all files on a repo.

Bug: 1042324
Change-Id: I2ebb7c3ec633f9d3a3774314799aa056bc2919c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2003071Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

presubmit_support.py

@@ -1725,7 +1725,7 @@ def DoPresubmitChecks(change,
     os.environ = old_environ
 
 
-def ScanSubDirs(mask, recursive):
+def _scan_sub_dirs(mask, recursive):
   if not recursive:
     return [x for x in glob.glob(mask) if x not in ('.svn', '.git')]
 
@@ -1741,32 +1741,83 @@ def ScanSubDirs(mask, recursive):
   return results
 
 
-def ParseFiles(args, recursive):
+def _parse_files(args, recursive):
   logging.debug('Searching for %s', args)
   files = []
   for arg in args:
-    files.extend([('M', f) for f in ScanSubDirs(arg, recursive)])
+    files.extend([('M', f) for f in _scan_sub_dirs(arg, recursive)])
   return files
 
 
-def load_files(options):
-  """Tries to determine the SCM."""
-  files = []
-  if options.files:
-    files = ParseFiles(options.files, options.recursive)
+def _parse_change(parser, options):
+  """Process change options.
+
+  Args:
+    parser: The parser used to parse the arguments from command line.
+    options: The arguments parsed from command line.
+  Returns:
+    A GitChange if the change root is a git repository, or a Change otherwise.
+  """
+  if options.files and options.all_files:
+    parser.error('<files> cannot be specified when --all-files is set.')
+
   change_scm = scm.determine_scm(options.root)
-  if change_scm == 'git':
-    change_class = GitChange
-    upstream = options.upstream or None
-    if not files:
-      files = scm.GIT.CaptureStatus([], options.root, upstream)
+  if change_scm != 'git' and not options.files:
+    parser.error('<files> is not optional for unversioned directories.')
+
+  if options.files:
+    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:
-    logging.info(
-        'Doesn\'t seem under source control. Got %d files', len(options.files))
-    if not files:
-      return None, None
-    change_class = Change
-  return change_class, files
+    change_files = scm.GIT.CaptureStatus(
+        [], options.root, options.upstream or None)
+
+  logging.info('Found %d file(s).', len(change_files))
+
+  change_class = GitChange if change_scm == 'git' else Change
+  return change_class(
+      options.name,
+      options.description,
+      options.root,
+      change_files,
+      options.issue,
+      options.patchset,
+      options.author,
+      upstream=options.upstream)
+
+
+def _parse_gerrit_options(parser, options):
+  """Process gerrit options.
+
+  SIDE EFFECTS: Modifies options.author and options.description from Gerrit if
+  options.gerrit_fetch is set.
+
+  Args:
+    parser: The parser used to parse the arguments from command line.
+    options: The arguments parsed from command line.
+  Returns:
+    A GerritAccessor object if options.gerrit_url is set, or None otherwise.
+  """
+  gerrit_obj = None
+  if options.gerrit_url:
+    gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc)
+
+  if not options.gerrit_fetch:
+    return gerrit_obj
+
+  if not options.gerrit_url or not options.issue or not options.patchset:
+    parser.error(
+        '--gerrit_fetch requires --gerrit_url, --issue and --patchset.')
+
+  options.author = gerrit_obj.GetChangeOwner(options.issue)
+  options.description = gerrit_obj.GetChangeDescription(
+      options.issue, options.patchset)
+
+  logging.info('Got author: "%s"', options.author)
+  logging.info('Got description: """\n%s\n"""', options.description)
+
+  return gerrit_obj
 
 
 @contextlib.contextmanager
@@ -1824,6 +1875,8 @@ def main(argv=None):
                            'all PRESUBMIT files in parallel.')
   parser.add_argument('--json_output',
                       help='Write presubmit errors to json output.')
+  parser.add_argument('--all-files', action='store_true',
+                      help='Mark all files under source control as modified.')
   parser.add_argument('files', nargs='*',
                       help='List of files to be marked as modified when '
                       'executing presubmit or post-upload hooks. fnmatch '
@@ -1831,21 +1884,6 @@ def main(argv=None):
 
   options = parser.parse_args(argv)
 
-  gerrit_obj = None
-  if options.gerrit_url:
-    gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc)
-  if options.gerrit_fetch:
-    if not options.gerrit_url or not options.issue or not options.patchset:
-      parser.error(
-          '--gerrit_fetch requires --gerrit_url, --issue and --patchset.')
-
-    options.author = gerrit_obj.GetChangeOwner(options.issue)
-    options.description = gerrit_obj.GetChangeDescription(
-        options.issue, options.patchset)
-
-    logging.info('Got author: "%s"', options.author)
-    logging.info('Got description: """\n%s\n"""', options.description)
-
   if options.verbose >= 2:
     logging.basicConfig(level=logging.DEBUG)
   elif options.verbose:
@@ -1853,22 +1891,13 @@ def main(argv=None):
   else:
     logging.basicConfig(level=logging.ERROR)
 
-  change_class, files = load_files(options)
-  if not change_class:
-    parser.error('For unversioned directory, <files> is not optional.')
-  logging.info('Found %d file(s).', len(files))
+  change = _parse_change(parser, options)
+  gerrit_obj = _parse_gerrit_options(parser, options)
 
   try:
     with canned_check_filter(options.skip_canned):
       results = DoPresubmitChecks(
-          change_class(options.name,
-                       options.description,
-                       options.root,
-                       files,
-                       options.issue,
-                       options.patchset,
-                       options.author,
-                       upstream=options.upstream),
+          change,
           options.commit,
           options.verbose,
           sys.stdout,

tests/presubmit_unittest.py

@@ -163,24 +163,25 @@ index fe3de7b..54ae6e1 100755
     mock.patch('gclient_utils.FileRead').start()
     mock.patch('gclient_utils.FileWrite').start()
     mock.patch('json.load').start()
-    mock.patch('os.path.abspath', lambda f: f).start()
-    mock.patch('os.getcwd', self.RootDir)
+    mock.patch('multiprocessing.cpu_count', lambda: 2)
     mock.patch('os.chdir').start()
+    mock.patch('os.getcwd', self.RootDir)
     mock.patch('os.listdir').start()
+    mock.patch('os.path.abspath', lambda f: f).start()
     mock.patch('os.path.isfile').start()
     mock.patch('os.remove').start()
-    mock.patch('random.randint').start()
-    mock.patch('presubmit_support.warn').start()
+    mock.patch('presubmit_support._parse_files').start()
     mock.patch('presubmit_support.sigint_handler').start()
     mock.patch('presubmit_support.time_time', return_value=0).start()
-    mock.patch('scm.determine_scm').start()
+    mock.patch('presubmit_support.warn').start()
+    mock.patch('random.randint').start()
     mock.patch('scm.GIT.GenerateDiff').start()
+    mock.patch('scm.determine_scm').start()
     mock.patch('subprocess2.Popen').start()
     mock.patch('sys.stderr', StringIO()).start()
     mock.patch('sys.stdout', StringIO()).start()
     mock.patch('tempfile.NamedTemporaryFile').start()
     mock.patch('threading.Timer').start()
-    mock.patch('multiprocessing.cpu_count', lambda: 2)
     if sys.version_info.major == 2:
       mock.patch('urllib2.urlopen').start()
     else:
@@ -878,10 +879,9 @@ def CheckChangeOnCommit(input_api, output_api):
                                                False, output))
 
   @mock.patch('presubmit_support.DoPresubmitChecks')
-  @mock.patch('presubmit_support.ParseFiles')
-  def testMainUnversioned(self, mockParseFiles, mockDoPresubmitChecks):
+  def testMainUnversioned(self, mockDoPresubmitChecks):
     scm.determine_scm.return_value = None
-    mockParseFiles.return_value = [('M', 'random_file.txt')]
+    presubmit._parse_files.return_value = [('M', 'random_file.txt')]
     mockDoPresubmitChecks().should_continue.return_value = False
 
     self.assertEqual(
@@ -898,8 +898,202 @@ def CheckChangeOnCommit(input_api, output_api):
     self.assertEqual(
         sys.stderr.getvalue(),
         'usage: presubmit_unittest.py [options] <files...>\n'
-        'presubmit_unittest.py: error: For unversioned directory, <files> is '
-        'not optional.\n')
+        'presubmit_unittest.py: error: <files> is not optional for unversioned '
+        'directories.\n')
+
+  @mock.patch('presubmit_support.Change', mock.Mock())
+  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)
+
+    change = presubmit._parse_change(None, options)
+    self.assertEqual(presubmit.Change.return_value, change)
+    presubmit.Change.assert_called_once_with(
+        options.name,
+        options.description,
+        options.root,
+        [('M', 'random_file.txt')],
+        options.issue,
+        options.patchset,
+        options.author,
+        upstream=options.upstream)
+    presubmit._parse_files.assert_called_once_with(
+        options.files, options.recursive)
+
+  def testParseChange_NoFilesAndNoScm(self):
+    presubmit._parse_files.return_value = []
+    scm.determine_scm.return_value = None
+    parser = mock.Mock()
+    parser.error.side_effect = [SystemExit]
+    options = mock.Mock(files=[], all_files=False)
+
+    with self.assertRaises(SystemExit):
+      presubmit._parse_change(parser, options)
+    parser.error.assert_called_once_with(
+        '<files> is not optional for unversioned directories.')
+
+  def testParseChange_FilesAndAllFiles(self):
+    parser = mock.Mock()
+    parser.error.side_effect = [SystemExit]
+    options = mock.Mock(files=['foo'], all_files=True)
+
+    with self.assertRaises(SystemExit):
+      presubmit._parse_change(parser, options)
+    parser.error.assert_called_once_with(
+        '<files> cannot be specified when --all-files is set.')
+
+  @mock.patch('presubmit_support.GitChange', mock.Mock())
+  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)
+
+    change = presubmit._parse_change(None, options)
+    self.assertEqual(presubmit.GitChange.return_value, change)
+    presubmit.GitChange.assert_called_once_with(
+        options.name,
+        options.description,
+        options.root,
+        [('M', 'random_file.txt')],
+        options.issue,
+        options.patchset,
+        options.author,
+        upstream=options.upstream)
+    presubmit._parse_files.assert_called_once_with(
+        options.files, options.recursive)
+
+  @mock.patch('presubmit_support.GitChange', mock.Mock())
+  @mock.patch('scm.GIT.CaptureStatus', mock.Mock())
+  def testParseChange_NoFilesAndGit(self):
+    scm.determine_scm.return_value = 'git'
+    scm.GIT.CaptureStatus.return_value = [('A', 'added.txt')]
+    options = mock.Mock(all_files=False, files=[])
+
+    change = presubmit._parse_change(None, options)
+    self.assertEqual(presubmit.GitChange.return_value, change)
+    presubmit.GitChange.assert_called_once_with(
+        options.name,
+        options.description,
+        options.root,
+        [('A', 'added.txt')],
+        options.issue,
+        options.patchset,
+        options.author,
+        upstream=options.upstream)
+    scm.GIT.CaptureStatus.assert_called_once_with(
+        [], options.root, options.upstream)
+
+  @mock.patch('presubmit_support.GitChange', mock.Mock())
+  @mock.patch('scm.GIT.GetAllFiles', mock.Mock())
+  def testParseChange_AllFilesAndGit(self):
+    scm.determine_scm.return_value = 'git'
+    scm.GIT.GetAllFiles.return_value = ['foo.txt', 'bar.txt']
+    options = mock.Mock(all_files=True, files=[])
+
+    change = presubmit._parse_change(None, options)
+    self.assertEqual(presubmit.GitChange.return_value, change)
+    presubmit.GitChange.assert_called_once_with(
+        options.name,
+        options.description,
+        options.root,
+        [('M', 'foo.txt'), ('M', 'bar.txt')],
+        options.issue,
+        options.patchset,
+        options.author,
+        upstream=options.upstream)
+    scm.GIT.GetAllFiles.assert_called_once_with(options.root)
+
+  def testParseGerritOptions_NoGerritUrl(self):
+    options = mock.Mock(
+        gerrit_url=None,
+        gerrit_fetch=False,
+        author='author',
+        description='description')
+    gerrit_obj = presubmit._parse_gerrit_options(None, options)
+
+    self.assertIsNone(gerrit_obj)
+    self.assertEqual('author', options.author)
+    self.assertEqual('description', options.description)
+
+  @mock.patch('presubmit_support.GerritAccessor', mock.Mock())
+  def testParseGerritOptions_NoGerritFetch(self):
+    options = mock.Mock(
+        gerrit_url='https://foo-review.googlesource.com/bar',
+        gerrit_fetch=False,
+        author='author',
+        description='description')
+
+    gerrit_obj = presubmit._parse_gerrit_options(None, options)
+
+    presubmit.GerritAccessor.assert_called_once_with(
+        'foo-review.googlesource.com')
+    self.assertEqual(presubmit.GerritAccessor.return_value, gerrit_obj)
+    self.assertEqual('author', options.author)
+    self.assertEqual('description', options.description)
+
+  @mock.patch('presubmit_support.GerritAccessor', mock.Mock())
+  def testParseGerritOptions_GerritFetch(self):
+    accessor = mock.Mock()
+    accessor.GetChangeOwner.return_value = 'new owner'
+    accessor.GetChangeDescription.return_value = 'new description'
+    presubmit.GerritAccessor.return_value = accessor
+
+    options = mock.Mock(
+        gerrit_url='https://foo-review.googlesource.com/bar',
+        gerrit_fetch=True,
+        issue=123,
+        patchset=4)
+
+    gerrit_obj = presubmit._parse_gerrit_options(None, options)
+
+    presubmit.GerritAccessor.assert_called_once_with(
+        'foo-review.googlesource.com')
+    self.assertEqual(presubmit.GerritAccessor.return_value, gerrit_obj)
+    self.assertEqual('new owner', options.author)
+    self.assertEqual('new description', options.description)
+
+  def testParseGerritOptions_GerritFetchNoUrl(self):
+    parser = mock.Mock()
+    parser.error.side_effect = [SystemExit]
+    options = mock.Mock(
+        gerrit_url=None,
+        gerrit_fetch=True,
+        issue=123,
+        patchset=4)
+
+    with self.assertRaises(SystemExit):
+      presubmit._parse_gerrit_options(parser, options)
+    parser.error.assert_called_once_with(
+        '--gerrit_fetch requires --gerrit_url, --issue and --patchset.')
+
+  def testParseGerritOptions_GerritFetchNoIssue(self):
+    parser = mock.Mock()
+    parser.error.side_effect = [SystemExit]
+    options = mock.Mock(
+        gerrit_url='https://example.com',
+        gerrit_fetch=True,
+        issue=None,
+        patchset=4)
+
+    with self.assertRaises(SystemExit):
+      presubmit._parse_gerrit_options(parser, options)
+    parser.error.assert_called_once_with(
+        '--gerrit_fetch requires --gerrit_url, --issue and --patchset.')
+
+  def testParseGerritOptions_GerritFetchNoPatchset(self):
+    parser = mock.Mock()
+    parser.error.side_effect = [SystemExit]
+    options = mock.Mock(
+        gerrit_url='https://example.com',
+        gerrit_fetch=True,
+        issue=123,
+        patchset=None)
+
+    with self.assertRaises(SystemExit):
+      presubmit._parse_gerrit_options(parser, options)
+    parser.error.assert_called_once_with(
+        '--gerrit_fetch requires --gerrit_url, --issue and --patchset.')
 
 
 class InputApiUnittest(PresubmitTestsBase):