Rename --merge to --use-merge-strategy, make it mutualy exclusive wit…

…h --add-tested, remove the Tested-by trailer in job.push_merged_version()

marge/app.py

@@ -100,7 +100,17 @@ def regexp(s):
         metavar='INTERVAL[,..]',
         help='Time(s) during which no merging is to take place, e.g. "Friday 1pm - Monday 9am".\n',
     )
-    parser.add_argument(
+    merge_group = parser.add_mutually_exclusive_group(required=False)
+    merge_group.add_argument(
+        '--use-merge-strategy',
+        action='store_true',
+        help=(
+            'Use git merge instead of git rebase\n'
+            '(enable this is you use git merge as\n'
+            'git tends to misbehave when both are used)\n'
+        ),
+    )
+    merge_group.add_argument(
         '--add-tested',
         action='store_true',
         help='Add "Tested: marge-bot <$MR_URL>" for the final commit on branch after it passed CI.\n',
@@ -150,15 +160,6 @@ def regexp(s):
         default='.*',
         help='Only process MRs whose target branches match the given regular expression.\n',
     )
-    parser.add_argument(
-        '--merge',
-        action='store_true',
-        help=(
-            'Use git merge instead of git rebase\n'
-            '(enable this is you use git merge as\n'
-            'git tends to misbehave when both are used)\n'
-        ),
-    )
     parser.add_argument(
         '--debug',
         action='store_true',
@@ -223,7 +224,7 @@ def main(args=sys.argv[1:]):
                 reapprove=options.impersonate_approvers,
                 embargo=options.embargo,
                 ci_timeout=options.ci_timeout,
-                merge=options.merge,
+                use_merge_strategy=options.use_merge_strategy,
             )
         )
 

marge/job.py

@@ -108,7 +108,7 @@ def update_merge_request_and_accept(self, approvals):
             source_repo_url = None if source_project is self._project else source_project.ssh_url_to_repo
             # NB. this will be a no-op if there is nothing to rebase/rewrite
             target_sha, _rebased_sha, actual_sha = update_from_target_branch_and_push(
-                use_merge_strategy=self.opts.merge,
+                use_merge_strategy=self.opts.use_merge_strategy,
                 repo=self.repo,
                 source_branch=merge_request.source_branch,
                 target_branch=merge_request.target_branch,
@@ -336,13 +336,6 @@ def push_merged_version(
                 branch=source_branch,
                 start_commit='origin/' + target_branch,
             )
-        if tested_by is not None:
-            rewritten_sha = repo.tag_with_trailer(
-                trailer_name='Tested-by',
-                trailer_values=tested_by,
-                branch=source_branch,
-                start_commit=source_branch + '^'
-            )
         if part_of is not None:
             rewritten_sha = repo.tag_with_trailer(
                 trailer_name='Part-of',
@@ -482,7 +475,7 @@ def _get_reviewer_names_and_emails(approvals, api):
     'reapprove',
     'embargo',
     'ci_timeout',
-    'merge',
+    'use_merge_strategy',
 ]
 
 class MergeJobOptions(namedtuple('MergeJobOptions', _job_options)):
@@ -496,7 +489,7 @@ def requests_commit_tagging(self):
     def default(
             cls, *,
             add_tested=False, add_part_of=False, add_reviewers=False, reapprove=False,
-            embargo=None, ci_timeout=None, merge=False
+            embargo=None, ci_timeout=None, use_merge_strategy=False
     ):
         embargo = embargo or IntervalUnion.empty()
         ci_timeout = ci_timeout or timedelta(minutes=15)
@@ -507,7 +500,7 @@ def default(
             reapprove=reapprove,
             embargo=embargo,
             ci_timeout=ci_timeout,
-            merge=merge,
+            use_merge_strategy=use_merge_strategy,
         )
 
 

tests/test_app.py

@@ -101,12 +101,24 @@ def test_embargo():
                embargo=interval.IntervalUnion.from_human('Fri 1pm-Mon 7am'),
             )
 
+def test_use_merge_strategy():
+    with env(MARGE_AUTH_TOKEN="NON-ADMIN-TOKEN", MARGE_SSH_KEY="KEY", MARGE_GITLAB_URL='http://foo.com'):
+        with main('--use-merge-strategy') as bot:
+            assert bot.config.merge_opts != job.MergeJobOptions.default()
+            assert bot.config.merge_opts == job.MergeJobOptions.default(use_merge_strategy=True)
+
 def test_add_tested():
     with env(MARGE_AUTH_TOKEN="NON-ADMIN-TOKEN", MARGE_SSH_KEY="KEY", MARGE_GITLAB_URL='http://foo.com'):
         with main('--add-tested') as bot:
             assert bot.config.merge_opts != job.MergeJobOptions.default()
             assert bot.config.merge_opts == job.MergeJobOptions.default(add_tested=True)
 
+def test_use_merge_strategy_and_add_tested_are_mutualy_exclusive():
+    with env(MARGE_AUTH_TOKEN="NON-ADMIN-TOKEN", MARGE_SSH_KEY="KEY", MARGE_GITLAB_URL='http://foo.com'):
+        with pytest.raises(SystemExit):
+            with main('--use-merge-strategy --add-tested') as bot:
+                pass
+
 def test_add_part_of():
     with env(MARGE_AUTH_TOKEN="NON-ADMIN-TOKEN", MARGE_SSH_KEY="KEY", MARGE_GITLAB_URL='http://foo.com'):
         with main('--add-part-of') as bot:
@@ -165,13 +177,6 @@ def test_branch_regexp():
             assert bot.config.branch_regexp == re.compile('foo.*bar')
 
 
-def test_merge():
-    with env(MARGE_AUTH_TOKEN="NON-ADMIN-TOKEN", MARGE_SSH_KEY="KEY", MARGE_GITLAB_URL='http://foo.com'):
-        with main('--merge') as bot:
-            assert bot.config.merge_opts != job.MergeJobOptions.default()
-            assert bot.config.merge_opts == job.MergeJobOptions.default(merge=True)
-
-
 # FIXME: I'd reallly prefer this to be a doctest, but adding --doctest-modules
 # seems to seriously mess up the test run
 def test_time_interval():

tests/test_job.py

@@ -407,7 +407,7 @@ def test_default(self):
             reapprove=False,
             embargo=marge.interval.IntervalUnion.empty(),
             ci_timeout=timedelta(minutes=15),
-            merge=False,
+            use_merge_strategy=False,
         )
 
     def test_default_ci_time(self):