split_cl: Make description file optional during dry runs

I frequently find myself running `git cl split` just to see how the
files are going to be split up, without actually intending to submit the
resulting CLs immediately. Doing so requires creating a file, putting a
dummy description in it, and passing that to the command line, which is
unnecessary work. This CL simply allows dry runs to use a dummy
description if none is provided, to make checking the tool's output
easier.

Bug: None
Change-Id: I47e06c6e6da26701e07dcae81ab605edac2e2ca6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6163904
Reviewed-by: Andy Perelson <ajp@google.com>
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Brian Ryner <bryner@google.com>

git_cl.py

@@ -5679,7 +5679,8 @@ def CMDsplit(parser, args):
                       '--description',
                       dest='description_file',
                       help='A text file containing a CL description in which '
-                      '$directory will be replaced by each CL\'s directory.')
+                      '$directory will be replaced by each CL\'s directory. '
+                      'Mandatory except in dry runs.')
     parser.add_option('-c',
                       '--comment',
                       dest='comment_file',
@@ -5731,7 +5732,7 @@ def CMDsplit(parser, args):
                       help='Topic to specify when uploading')
     options, _ = parser.parse_args(args)
 
-    if not options.description_file:
+    if not options.description_file and not options.dry_run:
         parser.error('No --description flag specified.')
 
     def WrappedCMDupload(args):

split_cl.py

@@ -218,6 +218,14 @@ def PrintClInfo(cl_index, num_cls, directories, file_paths, description,
     print()
 
 
+def LoadDescription(description_file):
+    if not description_file:
+        return ('Dummy description for dry run.\n'
+                'directory = $directory')
+
+    return gclient_utils.FileRead(description_file)
+
+
 def SplitCl(description_file, comment_file, changelist, cmd_upload, dry_run,
             cq_dry_run, enable_auto_submit, max_depth, topic, repository_root):
     """"Splits a branch into smaller branches and uploads CLs.
@@ -237,8 +245,10 @@ def SplitCl(description_file, comment_file, changelist, cmd_upload, dry_run,
     Returns:
         0 in case of success. 1 in case of error.
     """
-    description = AddUploadedByGitClSplitToDescription(
-        gclient_utils.FileRead(description_file))
+
+    description = LoadDescription(description_file, dry_run)
+    description = AddUploadedByGitClSplitToDescription(description)
+
     comment = gclient_utils.FileRead(comment_file) if comment_file else None
 
     try:
@@ -262,7 +272,7 @@ def SplitCl(description_file, comment_file, changelist, cmd_upload, dry_run,
         assert refactor_branch_upstream, \
             "Branch %s must have an upstream." % refactor_branch
 
-        if not CheckDescriptionBugLink(description):
+        if not dry_run and not CheckDescriptionBugLink(description):
             return 0
 
         files_split_by_reviewers = SelectReviewersForFiles(

tests/git_cl_test.py

@@ -175,6 +175,10 @@ class SystemExitMock(Exception):
     pass
 
 
+class ParserErrorMock(Exception):
+    pass
+
+
 class TestGitClBasic(unittest.TestCase):
     def setUp(self):
         mock.patch('sys.exit', side_effect=SystemExitMock).start()
@@ -5549,6 +5553,31 @@ Change-Id: I25699146b24c7ad8776f17775f489b9d41499595
                          expected_message)
 
 
+@unittest.skipIf(gclient_utils.IsEnvCog(),
+                 'not supported in non-git environment')
+class CMDSplitTestCase(CMDTestCaseBase):
+
+    def setUp(self):
+        super(CMDTestCaseBase, self).setUp()
+        mock.patch('git_cl.Settings.GetRoot', return_value='root').start()
+
+    @mock.patch("split_cl.SplitCl", return_value=0)
+    @mock.patch("git_cl.OptionParser.error", side_effect=ParserErrorMock)
+    def testDescriptionFlagRequired(self, _, mock_split_cl):
+        # --description-file is mandatory...
+        self.assertRaises(ParserErrorMock, git_cl.main, ['split'])
+        self.assertEqual(mock_split_cl.call_count, 0)
+
+        self.assertEqual(git_cl.main(['split', '--description=SomeFile.txt']),
+                         0)
+        self.assertEqual(mock_split_cl.call_count, 1)
+
+        # Unless we're doing a dry run
+        mock_split_cl.reset_mock()
+        self.assertEqual(git_cl.main(['split', '-n']), 0)
+        self.assertEqual(mock_split_cl.call_count, 1)
+
+
 if __name__ == '__main__':
     logging.basicConfig(
         level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)

tests/split_cl_test.py

@@ -193,6 +193,17 @@ class SplitClTest(unittest.TestCase):
         self.assertTrue(split_cl.CheckDescriptionBugLink("Description"))
         self.assertEqual(mock_ask_for_data.call_count, 1)
 
+    @mock.patch("gclient_utils.FileRead", return_value="Description")
+    def testLoadDescription(self, mock_file_read):
+        # No description provided, use the dummy:
+        self.assertTrue(split_cl.LoadDescription(None).startswith("Dummy"))
+        self.assertEqual(mock_file_read.call_count, 0)
+
+        # Description file provided, load it
+        self.assertEqual(split_cl.LoadDescription("SomeFile.txt"),
+                         "Description")
+        self.assertEqual(mock_file_read.call_count, 1)
+
 
 if __name__ == '__main__':
     unittest.main()