[git-webkit revert] Update pr arguments and usability

https://bugs.webkit.org/show_bug.cgi?id=267955 rdar://121379784 Reviewed by Jonathan Bedard. Set automatic PR creation to 'true' in git-webkit revert. Added additional logging and clearer prompts. * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py: (Revert.parser): Added --pr and --no-pr options. (Revert.get_issue_info): (Revert.main): * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/revert_unittest.py: (TestRevert.test_github): (TestRevert.test_github_two_step): (TestRevert.test_args): (test_update): (test_pr): Canonical link: https://commits.webkit.org/273569@main
WebKit · Jan 26, 2024 · a4de8ff · a4de8ff
1 parent b66901d
commit a4de8ff
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 20 deletions.
diff --git a/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py b/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py
@@ -66,10 +66,10 @@ def parser(cls, parser, loggers=None):
         )
 
         parser.add_argument(
-            '--pr',
-            default=False,
-            action='store_true',
-            help='Create a pull request at the same time'
+            '--pr', '--no-pr',
+            default=True,
+            action=arguments.NoAction,
+            help='Create a pull request (or do not) after reverting'
         )
 
     @classmethod
@@ -92,7 +92,8 @@ def get_commit_info(cls, args, repository, **kwargs):
     def get_issue_info(cls, args, repository, commit_objects, commit_bugs, **kwargs):
         # Can give either a bug URL or a title of the new issue
         if not args.issue and not args.reason:
-            prompt = 'Enter issue URL or reason for the revert: '
+            print('This issue will track the revert and should not be the issue of the commit(s) to be reverted.')
+            prompt = 'Enter issue URL or title of new issue (reason for the revert): '
             args.issue = Terminal.input(prompt, alert_after=2 * Terminal.RING_INTERVAL)
         elif args.reason:
             args.issue = args.reason
@@ -251,6 +252,6 @@ def main(cls, args, repository, **kwargs):
 
         if not args.pr:
             return result
-        else:
-            return PullRequest.create_pull_request(repository, args, branch_point)
+
+        return PullRequest.create_pull_request(repository, args, branch_point)
 
diff --git a/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/revert_unittest.py b/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/revert_unittest.py
@@ -64,7 +64,8 @@ def test_github(self):
 
         self.assertEqual(
             captured.stdout.getvalue(),
-            "Enter issue URL or reason for the revert: \n"
+            "This issue will track the revert and should not be the issue of the commit(s) to be reverted.\n"
+            "Enter issue URL or title of new issue (reason for the revert): \n"
             "Created the local development branch 'eng/Example-feature-1'\n"
             "Created 'PR 1 | Unreviewed, reverting 5@main'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n",
@@ -100,20 +101,22 @@ def test_github_two_step(self):
             ),
         ), mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', [bugzilla.Tracker(self.BUGZILLA)]):
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-v'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '--no-pr', '-v'),
                 path=self.path,
             )
             self.assertEqual(0, result)
             self.assertDictEqual(repo.modified, dict())
             self.assertDictEqual(repo.staged, dict())
             self.assertEqual(True, 'Unreviewed, reverting 5@main' in repo.head.message)
-            result = program.main(args=('pull-request', '-v', '--no-history'), path=self.path)
+            with MockTerminal.input('{}/show_bug.cgi?id=2'.format(self.BUGZILLA)):
+                result = program.main(args=('pull-request', '-v', '--no-history'), path=self.path)
             self.assertEqual(0, result)
             self.assertEqual(local.Git(self.path).remote().pull_requests.get(1).draft, False)
 
         self.assertEqual(
             captured.stdout.getvalue(),
-            "Enter issue URL or reason for the revert: \n"
+            "This issue will track the revert and should not be the issue of the commit(s) to be reverted.\n"
+            "Enter issue URL or title of new issue (reason for the revert): \n"
             "Created the local development branch 'eng/Example-feature-1'\n"
             "Created 'PR 1 | Unreviewed, reverting 5@main'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n",
@@ -138,7 +141,7 @@ def test_github_two_step(self):
         )
 
     def test_args(self):
-        with OutputCapture(level=logging.INFO) as captured, mocks.remote.GitHub() as remote, mocks.local.Git(
+        with MockTerminal.input('{}/show_bug.cgi?id=2'.format(self.BUGZILLA)), OutputCapture(level=logging.INFO) as captured, mocks.remote.GitHub() as remote, mocks.local.Git(
             self.path, remote='https://{}'.format(remote.remote),
             remotes=dict(fork='https://{}/Contributor/WebKit'.format(remote.hosts[0])),
         ) as repo, bmocks.Bugzilla(
@@ -150,14 +153,15 @@ def test_args(self):
             ),
         ), mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', [bugzilla.Tracker(self.BUGZILLA)]):
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '--issue', '{}/show_bug.cgi?id=2'.format(self.BUGZILLA), '-v'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '--issue', '{}/show_bug.cgi?id=2'.format(self.BUGZILLA), '-v', '--no-pr'),
                 path=self.path,
             )
             self.assertEqual(0, result)
             self.assertDictEqual(repo.modified, dict())
             self.assertDictEqual(repo.staged, dict())
             self.assertEqual(True, 'Unreviewed, reverting 5@main' in repo.head.message)
-            result = program.main(args=('pull-request', '-v', '--no-history'), path=self.path)
+            with MockTerminal.input('{}/show_bug.cgi?id=2'.format(self.BUGZILLA)):
+                result = program.main(args=('pull-request', '-v', '--no-history'), path=self.path)
             self.assertEqual(0, result)
             self.assertEqual(local.Git(self.path).remote().pull_requests.get(1).draft, False)
 
@@ -224,19 +228,21 @@ def test_update(self):
             ),
         ), mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', [bugzilla.Tracker(self.BUGZILLA)]):
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-v', '--pr'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-v'),
                 path=self.path,
             )
             self.assertEqual(0, result)
-            result = program.main(
-                args=('pull-request', '-v', '--no-history'),
-                path=self.path,
-            )
+            with MockTerminal.input('{}/show_bug.cgi?id=2'.format(self.BUGZILLA)):
+                result = program.main(
+                    args=('pull-request', '-v', '--no-history'),
+                    path=self.path,
+                )
             self.assertEqual(0, result)
 
         self.assertEqual(
             captured.stdout.getvalue(),
-            "Enter issue URL or reason for the revert: \n"
+            "This issue will track the revert and should not be the issue of the commit(s) to be reverted.\n"
+            "Enter issue URL or title of new issue (reason for the revert): \n"
             "Created the local development branch 'eng/Example-feature-1'\n"
             "Created 'PR 1 | Unreviewed, reverting 5@main'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n"
@@ -272,3 +278,30 @@ def test_update(self):
                 "Updating pull-request for 'eng/Example-feature-1'..."
             ],
         )
+
+    def test_pr(self):
+        with MockTerminal.input('{}/show_bug.cgi?id=2'.format(self.BUGZILLA)), OutputCapture(level=logging.INFO) as captured, mocks.remote.GitHub() as remote, mocks.local.Git(
+            self.path, remote='https://{}'.format(remote.remote),
+            remotes=dict(fork='https://{}/Contributor/WebKit'.format(remote.hosts[0])),
+        ) as repo, bmocks.Bugzilla(
+            self.BUGZILLA.split('://')[-1],
+            issues=bmocks.ISSUES,
+            environment=Environment(
+                BUGS_EXAMPLE_COM_USERNAME='tcontributor@example.com',
+                BUGS_EXAMPLE_COM_PASSWORD='password',
+            ),
+        ), mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', [bugzilla.Tracker(self.BUGZILLA)]):
+            result = program.main(
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '--issue', '{}/show_bug.cgi?id=2'.format(self.BUGZILLA), '-v'),
+                path=self.path,
+            )
+            self.assertEqual(0, result)
+            self.assertDictEqual(repo.modified, dict())
+            self.assertDictEqual(repo.staged, dict())
+            self.assertEqual(True, 'Unreviewed, reverting 5@main' in repo.head.message)
+            self.assertEqual(
+                captured.stdout.getvalue(),
+                "Created the local development branch 'eng/Example-feature-1'\n"
+                "Created 'PR 1 | Unreviewed, reverting 5@main'!\n"
+                "https://github.example.com/WebKit/WebKit/pull/1\n"
+            )