Merge branch 'feature/PTFE-583-prevent-creation-integration-branches' into q/3.9

bert-e · bert-e · commit 63363af7c476 · 2023-07-26T22:08:08.000Z
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,11 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+## [3.9.0] - 2023-07-20
+# Added
+- Introducing a new option that prevent the creation of
+  integration branches.
+
 ## [3.7.0] - 2022-08-08
 # Added
 - Support config settings through environment.
diff --git a/bert_e/docs/USER_DOC.md b/bert_e/docs/USER_DOC.md
@@ -119,6 +119,7 @@ __Bert-E__.
 | bypass_peer_approval      | Bypass the pull request peer's approval | yes | no
 | bypass_leader_approval    | Bypass the pull request leader's approval | yes | no
 | create_pull_requests      | Let __Bert-E__ create pull requests corresponding to integration branches | no | no
+| create_integration_branches | Request __Bert-E__ to create integration branches and move forward with the gitwaterflow | no | no
 | no_octopus                | Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead | yes | no
 | unanimity                 | Change review acceptance criteria from `one reviewer at least` to `all reviewers` (**this feature is not supported on GitHub**) | no | no
 | wait                      | Instruct __Bert-E__ not to run until further notice | no | no
@@ -170,6 +171,19 @@ target development branch. This code is then tested in the build pipeline,
 before any merge can happen. There are as many integration branches as there
 are target branches.
 
+The integration branches are mandatory in the GitWaterFlow process,
+and by default they are automatically created when a pull request is
+opened.
+This behaviour can be changed with the `always_create_integration_branches`
+parameter in the bot settings:
+
+* *true* (default): integration branches are created automatically when a
+  pull request is opened.
+* *false*: users will be required to explicitly request the creation
+  of integration branches by adding a `/create_integration_branches`
+  comment in their pull request (or it will be set for them during
+  other operations, like `approve` or `create_pull_requests`).
+
 On the Git project, the name of the integration branches follow the format:
 
 ```w/<version>/<name_of_source_branch>```
diff --git a/bert_e/exceptions.py b/bert_e/exceptions.py
@@ -229,6 +229,11 @@ class NotAuthor(TemplateException):
     template = "not_author.md"
 
 
+class RequestIntegrationBranches(TemplateException):
+    code = 135
+    template = "request_integration_branches.md"
+
+
 # internal exceptions
 class UnableToSendEmail(InternalException):
     code = 201
diff --git a/bert_e/settings.py b/bert_e/settings.py
@@ -129,6 +129,8 @@ class Meta:
     # Settings defined in config files
     always_create_integration_pull_requests = fields.Bool(
         required=False, load_default=True)
+    always_create_integration_branches = fields.Bool(
+        required=False, load_default=True)
 
     frontend_url = fields.Str(required=False, load_default='')
 
diff --git a/bert_e/templates/request_integration_branches.md b/bert_e/templates/request_integration_branches.md
@@ -0,0 +1,19 @@
+{% extends "message.md" %}
+
+{% block title -%}
+Request integration branches
+{% endblock %}
+
+{% block message %}
+Waiting for integration branch creation to be requested by the user.
+
+To request integration branches, please comment on this pull request with the following command:
+
+```
+/create_integration_branches
+```
+
+Alternatively, the `/approve` and `/create_pull_requests` commands will automatically
+create the integration branches.
+
+{% endblock %}
diff --git a/bert_e/tests/test_bert_e.py b/bert_e/tests/test_bert_e.py
@@ -1159,6 +1159,210 @@ def test_comments_without_integration_pull_requests(self):
         self.assertIn('Hello %s' % self.args.contributor_username,
                       self.get_last_pr_comment(pr))
 
+    def test_request_integration_branch_creation(self):
+        """Test comments to request integration branches creation.
+
+        1. Create a PR and ensure the proper message is sent regarding
+           the creation of integration branches.
+        2. Request the creation of integration branches and ensure the
+           branches are created.
+        3. Once the integration branches are created,
+           ensure the bot is able to merge the PR.
+
+        """
+        settings = """
+repository_owner: {owner}
+repository_slug: {slug}
+repository_host: {host}
+robot: {robot}
+robot_email: nobody@nowhere.com
+pull_request_base_url: https://bitbucket.org/{owner}/{slug}/bar/pull-requests/{{pr_id}}
+commit_base_url: https://bitbucket.org/{owner}/{slug}/commits/{{commit_id}}
+build_key: pre-merge
+required_leader_approvals: 0
+required_peer_approvals: 1
+always_create_integration_branches: false
+always_create_integration_pull_requests: false
+admins:
+  - {admin}
+""" # noqa
+        options = self.bypass_all_but(['bypass_build_status'])
+        pr = self.create_pr('feature/TEST-0069', 'development/4.3')
+        with self.assertRaises(exns.RequestIntegrationBranches):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+        self.assertEqual(len(list(pr.get_comments())), 2)
+        self.assertIn(
+            'Request integration branches', self.get_last_pr_comment(pr))
+        self.assertIn(
+            '/create_integration_branches', self.get_last_pr_comment(pr))
+
+        pr.add_comment('/create_integration_branches')
+        with self.assertRaises(exns.BuildNotStarted):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+        self.assertEqual(len(list(pr.get_comments())), 4)
+        self.assertIn('Integration data created', self.get_last_pr_comment(pr))
+        self.assertIn(
+            'create_integration_branches', self.get_last_pr_comment(pr))
+
+        options = self.bypass_all
+        with self.assertRaises(exns.SuccessMessage):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+
+    def test_request_integration_branch_by_creating_pull_requests(self):
+        """Test creating integration branches by creating pull requests
+
+        1. Create a PR and verify that the appropriate message is sent
+           regarding its creation
+        2. Type `/create_integration_branches` and ensure the
+           branches are created.
+        3. Once the integration branches are created,
+           ensure the bot is able to merge the PR.
+
+        """
+        settings = """
+repository_owner: {owner}
+repository_slug: {slug}
+repository_host: {host}
+robot: {robot}
+robot_email: nobody@nowhere.com
+pull_request_base_url: https://bitbucket.org/{owner}/{slug}/bar/pull-requests/{{pr_id}}
+commit_base_url: https://bitbucket.org/{owner}/{slug}/commits/{{commit_id}}
+build_key: pre-merge
+required_leader_approvals: 0
+required_peer_approvals: 1
+always_create_integration_branches: false
+always_create_integration_pull_requests: false
+admins:
+  - {admin}
+""" # noqa
+        options = self.bypass_all_but(['bypass_build_status'])
+        pr = self.create_pr('feature/TEST-0069', 'development/4.3')
+        with self.assertRaises(exns.RequestIntegrationBranches):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+
+        pr.add_comment('/create_pull_requests')
+        with self.assertRaises(exns.BuildNotStarted):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+        self.assertEqual(len(list(pr.get_comments())), 4)
+        self.assertIn('Integration data created', self.get_last_pr_comment(pr))
+
+        options = self.bypass_all
+        with self.assertRaises(exns.SuccessMessage):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+
+        self.assertIn(
+            'I have successfully merged the changeset',
+            self.get_last_pr_comment(pr))
+
+    def test_creation_integration_branch_by_approve(self):
+        """Test pr.approve() to request integration branches creation.
+
+        1. Create a PR and verify that the appropriate message is sent
+           regarding its creation
+        2. Ensure that author approval is required for the PR
+        3. Approve the PR from the author's perspective and check if
+           the integration branches are created.
+        4. Once the integration branches are created,
+           ensure the bot is able to merge the PR.
+
+        """
+        settings = """
+repository_owner: {owner}
+repository_slug: {slug}
+repository_host: {host}
+robot: {robot}
+robot_email: nobody@nowhere.com
+pull_request_base_url: https://bitbucket.org/{owner}/{slug}/bar/pull-requests/{{pr_id}}
+commit_base_url: https://bitbucket.org/{owner}/{slug}/commits/{{commit_id}}
+build_key: pre-merge
+required_leader_approvals: 0
+required_peer_approvals: 0
+always_create_integration_branches: false
+admins:
+  - {admin}
+""" # noqa
+        pr_1 = self.create_pr('feature/TEST-0069', 'development/4.3')
+        pr_2 = self.create_pr('feature/TEST-0070', 'development/4.3')
+        prs = [pr_1, pr_2]
+
+        for pr in prs:
+            options = self.bypass_all_but(['bypass_build_status',
+                                           'bypass_author_approval'])
+            with self.assertRaises(exns.ApprovalRequired):
+                self.handle(pr.id, options=options, backtrace=True)
+
+            self.assertEqual(len(list(pr.get_comments())), 3)
+
+            self.assertIn(
+                'Integration data created', list(pr.get_comments())[-2].text)
+
+            self.assertIn(
+                'Waiting for approval', self.get_last_pr_comment(pr))
+            self.assertIn(
+                'The following approvals are needed',
+                self.get_last_pr_comment(pr))
+
+            if pr.src_branch == "feature/TEST-0069":
+                pr.approve()
+            elif pr.src_branch == "feature/TEST-0070":
+                pr.add_comment('/approve')
+
+            with self.assertRaises(exns.BuildNotStarted):
+                self.handle(
+                    pr.id, settings=settings, options=options, backtrace=True)
+
+            options = self.bypass_all
+            with self.assertRaises(exns.SuccessMessage):
+                self.handle(
+                    pr.id, settings=settings, options=options, backtrace=True)
+
+            self.assertIn(
+                'I have successfully merged the changeset',
+                self.get_last_pr_comment(pr))
+
+    def test_integration_branch_creation_latest_branch(self):
+        """Test there is no comment to request integration branches creation.
+
+        1. Create a PR with the latest branch and check if there is no comment
+           to request integration branches creation.
+        2. Then, ensure the bot is able to merge the PR.
+
+        """
+        settings = """
+repository_owner: {owner}
+repository_slug: {slug}
+repository_host: {host}
+robot: {robot}
+robot_email: nobody@nowhere.com
+pull_request_base_url: https://bitbucket.org/{owner}/{slug}/bar/pull-requests/{{pr_id}}
+commit_base_url: https://bitbucket.org/{owner}/{slug}/commits/{{commit_id}}
+build_key: pre-merge
+required_leader_approvals: 0
+required_peer_approvals: 1
+always_create_integration_branches: false
+admins:
+  - {admin}
+""" # noqa
+        options = self.bypass_all_but(['bypass_build_status'])
+        pr = self.create_pr('feature/TEST-0069', 'development/10.0')
+        self.handle(pr.id, settings=settings, options=options)
+        self.assertEqual(len(list(pr.get_comments())), 1)
+
+        with self.assertRaises(exns.BuildNotStarted):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+
+        options = self.bypass_all
+        with self.assertRaises(exns.SuccessMessage):
+            self.handle(
+                pr.id, settings=settings, options=options, backtrace=True)
+
     def test_comments_for_manual_integration_pr_creation(self):
         """Test comments when integration data is created.
 
diff --git a/bert_e/workflow/gitwaterflow/__init__.py b/bert_e/workflow/gitwaterflow/__init__.py
@@ -34,7 +34,8 @@
     bypass_author_approval, bypass_leader_approval, bypass_build_status
 )
 from .commands import setup  # noqa
-from .integration import (create_integration_branches,
+from .integration import (check_integration_branches,
+                          create_integration_branches,
                           create_integration_pull_requests,
                           merge_integration_branches,
                           notify_integration_data,
@@ -158,6 +159,7 @@ def _handle_pull_request(job: PullRequestJob):
     check_branch_compatibility(job)
     jira_checks(job)
 
+    check_integration_branches(job)
     wbranches = list(create_integration_branches(job))
     use_queue = job.settings.use_queue
     if use_queue and queueing.already_in_queue(job, wbranches):
diff --git a/bert_e/workflow/gitwaterflow/commands.py b/bert_e/workflow/gitwaterflow/commands.py
@@ -209,6 +209,11 @@ def setup(defaults={}):
         "Allow the creation of integration pull requests.",
         privileged=False,
         default=defaults.get("create_pull_requests", False))
+    Reactor.add_option(
+        "create_integration_branches",
+        "Allow the creation of integration branches.",
+        privileged=False,
+        default=defaults.get("create_integration_branches", False))
     Reactor.add_option(
         "no_octopus",
         "Prevent Wall-E from doing any octopus merge and use multiple "
diff --git a/bert_e/workflow/gitwaterflow/integration.py b/bert_e/workflow/gitwaterflow/integration.py
@@ -136,6 +136,29 @@ def update(wbranch, source):
         prev = branch
 
 
+def check_integration_branches(job):
+    """Check if the integration branches can be created."""
+
+    approvals = set(job.pull_request.get_approvals())
+    if job.settings.approve:
+        approvals.add(job.pull_request.author)
+    approved_by_author = job.pull_request.author in approvals
+
+    create_integration = (job.settings.always_create_integration_branches or
+                          job.settings.create_integration_branches)
+    create_prs = (job.settings.always_create_integration_pull_requests or
+                  job.settings.create_pull_requests)
+    multiple_dst_branches = len(job.git.cascade.dst_branches) <= 1
+
+    if not (create_integration or
+            create_prs or
+            approved_by_author or
+            multiple_dst_branches):
+        raise exceptions.RequestIntegrationBranches(
+            active_options=job.active_options,
+        )
+
+
 def check_conflict(job, dst: git.Branch, src: git.Branch):
     """Check conflict between the source and destination branches of a PR."""
     # Create a temporary branch starting off from the destination branch, only
diff --git a/charts/bert-e/templates/settings.yaml b/charts/bert-e/templates/settings.yaml
@@ -21,6 +21,7 @@ stringData:
     robot_email: {{ .Values.bertE.robot.email | quote }}
     organization: {{ .Values.bertE.oauth.organization }}
     always_create_integration_pull_requests: {{ .Values.bertE.gating.alwaysCreateIntegrationPullRequests }}
+    always_create_integration_branches: {{ .Values.bertE.gating.alwaysCreateIntegrationBranches }}
     {{- if eq .Values.bertE.repository.gitHost "bitbucket" }}
     pull_request_base_url: https://bitbucket.org/{{ .Values.bertE.repository.owner }}/{{ .Values.bertE.repository.slug }}/pull-requests/{pr_id}
     commit_base_url: https://bitbucket.org/{{ .Values.bertE.repository.owner }}/{{ .Values.bertE.repository.slug }}/commits/{commit_id}
diff --git a/charts/bert-e/values.yaml b/charts/bert-e/values.yaml
@@ -130,6 +130,21 @@ bertE:
     ##
     alwaysCreateIntegrationPullRequests: true
 
+    ## always_create_integration_branches decides whether integration
+    ## branches are created in automatically when a pull request is opened.
+    ##
+    ## Bert-e will by default automatically create them.
+    ##
+    ##   Set this setting to false to require users to explicitly request the creation
+    ##   of integration branches by adding a `/create_integration_branches`
+    ##   comment in their pull request.
+    ##
+    ##   The above allow to temporize the creation of those branches as
+    ##   in the review process requested changes as expected.
+    ##
+    ##   default value: true
+    ##
+    alwaysCreateIntegrationBranches: true
     ## buildKey is the label of the key to look for in commit statuses.
     ##
     ##   default value: pre-merge
diff --git a/settings.sample.yml b/settings.sample.yml
