fix: resolve issue #290 - Reduce Bert-e's verbosity by updating comment

bert_e/exceptions.py

@@ -31,6 +31,9 @@ class TemplateException(BertE_Exception):
     template = None
     # whether to re-publish if the message is already in the history
     dont_repeat_if_in_history = -1
+    # whether to edit the previous status comment of the bot instead of
+    # posting a new one
+    update_status_comment = False
 
     def __init__(self, **kwargs):
         self.kwargs = kwargs
@@ -130,18 +133,21 @@ class MismatchPrefixIssueType(TemplateException):
 class IncorrectFixVersion(TemplateException):
     code = 112
     template = 'incorrect_fix_version.md'
+    update_status_comment = True
     status = "failure"
 
 
 class BranchHistoryMismatch(TemplateException):
     code = 113
     template = 'history_mismatch.md'
+    update_status_comment = True
     status = "failure"
 
 
 class Conflict(TemplateException):
     code = 114
     template = 'conflict.md'
+    update_status_comment = True
     status = "failure"
 
 
@@ -166,6 +172,7 @@ class AfterPullRequest(TemplateException):
 class IntegrationDataCreated(InformationException):
     code = 121
     template = 'integration_data_created.md'
+    update_status_comment = True
 
 
 class UnknownCommand(TemplateException):
@@ -226,6 +233,7 @@ class QueueOutOfOrder(TemplateException):
 class ResetComplete(TemplateException):
     code = 128
     template = "reset_complete.md"
+    update_status_comment = True
 
 
 class LossyResetWarning(TemplateException):

bert_e/git_host/base.py

@@ -277,6 +277,13 @@ def text(self) -> str:
     def id(self) -> int:
         """The comment's ID"""
 
+    def edit(self, text: str) -> None:
+        """Replace the comment's contents with the given text.
+
+        Git hosts that cannot edit comments raise NotImplementedError.
+        """
+        raise NotImplementedError('Comment edition is not supported')
+
 
 class AbstractPullRequest(metaclass=ABCMeta):
     @abstractmethod

bert_e/git_host/github/__init__.py

@@ -269,7 +269,7 @@ def patch(self, url, data, **kwargs):
 
         """
         url = self._patch_url(url)
-        response = self.session.post(url, data=data, **kwargs)
+        response = self.session.patch(url, data=data, **kwargs)
         response.raise_for_status()
         return json.loads(response.text)
 
@@ -1018,6 +1018,11 @@ def id(self) -> int:
     def delete(self) -> None:
         self.client.delete(self.data['url'])
 
+    def edit(self, text: str) -> None:
+        updated = type(self).update(
+            self.client, {'body': text}, url=self.data['url'])
+        self.data = updated.data
+
 
 class CheckRun(base.AbstractGitHostObject):
     GET_URL = '/repos/{owner}/{repo}/check-runs/{id}'

bert_e/git_host/mock.py

@@ -409,6 +409,10 @@ class CommentController(Controller, base.AbstractComment):
     def delete(self):
         self.controlled.delete()
 
+    def edit(self, text):
+        self.controlled.content = {"raw": text, "markup": "markdown",
+                                   "html": text}
+
     @property
     def author(self):
         return self['user']['username'].lower()

bert_e/tests/unit/test_comment_update.py

@@ -0,0 +1,85 @@
+from types import SimpleNamespace
+
+import pytest
+
+from bert_e import exceptions
+from bert_e.workflow import pr_utils
+
+
+class FakeComment:
+    def __init__(self, author, text, can_update=True):
+        self.author = author
+        self.text = text
+        self.can_update = can_update
+
+    def edit(self, text):
+        if not self.can_update:
+            raise NotImplementedError
+        self.text = text
+
+
+class FakePR:
+    def __init__(self, can_update=True):
+        self.comments = []
+        self.can_update = can_update
+
+    def add_comment(self, msg):
+        self.comments.append(FakeComment('bert-e', msg, self.can_update))
+
+
+@pytest.fixture
+def settings():
+    return SimpleNamespace(no_comment=False, interactive=False,
+                           robot='bert-e')
+
+
+def send(settings, pr, msg, update=True):
+    pr_utils._send_comment(settings, pr, msg, 0, update)
+
+
+def test_status_comment_is_updated_in_place(settings):
+    pr = FakePR()
+    send(settings, pr, 'history conflict')
+    send(settings, pr, 'reset complete')
+    assert len(pr.comments) == 1
+    assert pr.comments[0].text.endswith('reset complete')
+    assert pr.comments[0].text.startswith(pr_utils.STATUS_MARKER)
+
+
+def test_status_comment_updated_even_after_other_comments(settings):
+    pr = FakePR()
+    send(settings, pr, 'status 1')
+    pr.comments.append(FakeComment('bert-e', 'approved'))
+    send(settings, pr, 'status 2')
+    assert len(pr.comments) == 2
+    assert pr.comments[0].text.endswith('status 2')
+
+
+def test_identical_status_raises(settings):
+    pr = FakePR()
+    send(settings, pr, 'same')
+    with pytest.raises(exceptions.CommentAlreadyExists):
+        send(settings, pr, 'same')
+
+
+def test_other_authors_comments_are_not_edited(settings):
+    pr = FakePR()
+    pr.comments.append(
+        FakeComment('someone', pr_utils.STATUS_MARKER + '\nfoo'))
+    send(settings, pr, 'status')
+    assert len(pr.comments) == 2
+    assert pr.comments[0].text.endswith('foo')
+
+
+def test_regular_comments_are_not_updated(settings):
+    pr = FakePR()
+    send(settings, pr, 'a', update=False)
+    send(settings, pr, 'b', update=False)
+    assert [c.text for c in pr.comments] == ['a', 'b']
+
+
+def test_fallback_when_host_cannot_edit(settings):
+    pr = FakePR(can_update=False)
+    send(settings, pr, 'a')
+    send(settings, pr, 'b')
+    assert len(pr.comments) == 2

bert_e/tests/unit/test_github_comment_api.py

@@ -0,0 +1,28 @@
+from unittest import mock
+
+from bert_e.git_host.github import Client, Comment
+
+
+def test_client_patch_uses_http_patch():
+    client = Client.__new__(Client)
+    client.session = mock.Mock()
+    client.session.patch.return_value.text = '{}'
+    client._patch_url = lambda url: url
+    client.patch('https://api.github.com/c/1', data='{}')
+    client.session.patch.assert_called_once()
+    client.session.post.assert_not_called()
+
+
+def test_comment_edit_sends_patch_and_refreshes_data():
+    client = mock.Mock()
+    client.patch.return_value = {
+        'id': 1, 'url': 'https://api.github.com/c/1', 'body': 'new',
+        'user': {'login': 'bert-e', 'id': 1},
+        'created_at': '2020-01-01T00:00:00Z'}
+    comment = Comment(client, _validate=False, id=1,
+                      url='https://api.github.com/c/1', body='old',
+                      user={'login': 'bert-e', 'id': 1},
+                      created_at='2020-01-01T00:00:00Z')
+    comment.edit('new')
+    assert client.patch.call_args[0][0] == 'https://api.github.com/c/1'
+    assert comment.text == 'new'

bert_e/workflow/pr_utils.py

@@ -50,8 +50,38 @@ def find_comment(pull_request: AbstractPullRequest, username=None,
         return comment
 
 
+STATUS_MARKER = '<!-- bert-e-status -->'
+
+
+def _update_status_comment(settings, pull_request: AbstractPullRequest,
+                           msg: str) -> bool:
+    """Edit the bot's status comment in place, or create it.
+
+    Returns True if the message was handled (edited or posted), False if
+    the git host cannot edit comments and a regular comment should be sent.
+
+    Raises:
+        CommentAlreadyExists: if the status comment already has this text.
+
+    """
+    body = f'{STATUS_MARKER}\n{msg}'
+    previous = find_comment(pull_request, settings.robot, STATUS_MARKER)
+    if previous is None:
+        pull_request.add_comment(body)
+        return True
+    if previous.text == body:
+        raise exceptions.CommentAlreadyExists(
+            "The status comment is already up to date.")
+    try:
+        previous.edit(body)
+    except NotImplementedError:
+        return False
+    return True
+
+
 def _send_comment(settings, pull_request: AbstractPullRequest, msg: str,
-                  dont_repeat_if_in_history=10) -> None:
+                  dont_repeat_if_in_history=10,
+                  update_status_comment=False) -> None:
     """Comment a pull request.
 
     Before posting:
@@ -81,6 +111,9 @@ def _send_comment(settings, pull_request: AbstractPullRequest, msg: str,
             return
 
     LOG.debug('SENDING MESSAGE %s', msg)
+    if update_status_comment and _update_status_comment(
+            settings, pull_request, msg):
+        return
     pull_request.add_comment(msg)
 
 
@@ -104,6 +137,7 @@ def notify_user(settings, pull_request: AbstractPullRequest,
     try:
         _send_bot_status(settings, pull_request, comment)
         _send_comment(settings, pull_request, str(comment),
-                      comment.dont_repeat_if_in_history)
+                      comment.dont_repeat_if_in_history,
+                      comment.update_status_comment)
     except exceptions.CommentAlreadyExists:
         LOG.info("Comment '%s' already posted", comment.__class__.__name__)