fix: resolve issue # -

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 the message is a status that supersedes the previous one: it
+    # then updates the existing status comment instead of posting a new one
+    updatable = False
 
     def __init__(self, **kwargs):
         self.kwargs = kwargs
@@ -59,6 +62,7 @@ class SilentException(BertE_Exception):
 # template for informative exceptions
 class InformationException(TemplateException):
     dont_repeat_if_in_history = NEVER_REPEAT
+    updatable = True
 
 
 # template exceptions
@@ -131,12 +135,14 @@ class IncorrectFixVersion(TemplateException):
     code = 112
     template = 'incorrect_fix_version.md'
     status = "failure"
+    updatable = True
 
 
 class BranchHistoryMismatch(TemplateException):
     code = 113
     template = 'history_mismatch.md'
     status = "failure"
+    updatable = True
 
 
 class Conflict(TemplateException):
@@ -226,6 +232,7 @@ class QueueOutOfOrder(TemplateException):
 class ResetComplete(TemplateException):
     code = 128
     template = "reset_complete.md"
+    updatable = 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, msg: str) -> None:
+        """Replace the comment's contents with `msg`.
+
+        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

@@ -1018,6 +1018,11 @@ def id(self) -> int:
     def delete(self) -> None:
         self.client.delete(self.data['url'])
 
+    def edit(self, msg: str) -> None:
+        self.data = self.update(
+            self.client, {'body': msg}, url=self.data['url']
+        ).data
+
 
 class CheckRun(base.AbstractGitHostObject):
     GET_URL = '/repos/{owner}/{repo}/check-runs/{id}'

bert_e/git_host/mock.py

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

bert_e/tests/unit/test_comment_update.py

@@ -0,0 +1,89 @@
+"""Status comments are edited in place instead of being re-posted."""
+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, editable=True):
+        self.author = author
+        self.text = text
+        self.editable = editable
+
+    def edit(self, msg):
+        if not self.editable:
+            raise NotImplementedError
+        self.text = msg
+
+
+class FakePR:
+    def __init__(self, editable=True):
+        self.comments = []
+        self.editable = editable
+
+    def add_comment(self, msg):
+        self.comments.append(FakeComment('bert-e', msg, self.editable))
+
+
+@pytest.fixture
+def settings():
+    return SimpleNamespace(no_comment=False, interactive=False,
+                           robot='bert-e')
+
+
+def _send(settings, pr, msg, exc_cls):
+    pr_utils._send_comment(settings, pr, msg,
+                           exc_cls.dont_repeat_if_in_history,
+                           updatable=exc_cls.updatable)
+
+
+def test_updatable_classes():
+    assert exceptions.ResetComplete.updatable
+    assert exceptions.BranchHistoryMismatch.updatable
+    assert exceptions.IncorrectFixVersion.updatable
+    assert exceptions.IntegrationDataCreated.updatable
+    assert not exceptions.HelpMessage.updatable
+    assert not exceptions.StatusReport.updatable
+
+
+def test_status_comment_is_updated(settings):
+    pr = FakePR()
+    _send(settings, pr, 'reset', exceptions.ResetComplete)
+    _send(settings, pr, 'history mismatch', exceptions.BranchHistoryMismatch)
+    assert len(pr.comments) == 1
+    assert pr.comments[0].text.startswith('history mismatch')
+    assert pr.comments[0].text.endswith(pr_utils.STATUS_MARKER)
+
+
+def test_identical_status_not_reposted(settings):
+    pr = FakePR()
+    _send(settings, pr, 'reset', exceptions.ResetComplete)
+    with pytest.raises(exceptions.CommentAlreadyExists):
+        _send(settings, pr, 'reset', exceptions.ResetComplete)
+    assert len(pr.comments) == 1
+
+
+def test_non_updatable_message_is_posted(settings):
+    pr = FakePR()
+    _send(settings, pr, 'reset', exceptions.ResetComplete)
+    _send(settings, pr, 'help', exceptions.HelpMessage)
+    assert len(pr.comments) == 2
+    assert pr_utils.STATUS_MARKER not in pr.comments[1].text
+
+
+def test_fallback_when_edit_unsupported(settings):
+    pr = FakePR(editable=False)
+    _send(settings, pr, 'reset', exceptions.ResetComplete)
+    _send(settings, pr, 'other', exceptions.BranchHistoryMismatch)
+    assert len(pr.comments) == 2
+
+
+def test_other_authors_comments_untouched(settings):
+    pr = FakePR()
+    pr.comments.append(FakeComment('someone', 'hi ' + pr_utils.STATUS_MARKER))
+    _send(settings, pr, 'reset', exceptions.ResetComplete)
+    assert len(pr.comments) == 2
+    assert pr.comments[0].text.startswith('hi')

bert_e/workflow/pr_utils.py

@@ -21,6 +21,9 @@
 
 LOG = logging.getLogger(__name__)
 
+# Hidden marker identifying the comment holding the bot's latest status.
+STATUS_MARKER = '<!-- bert-e:status -->'
+
 
 def find_comment(pull_request: AbstractPullRequest, username=None,
                  startswith=None, max_history=None) -> AbstractComment:
@@ -50,15 +53,26 @@ def find_comment(pull_request: AbstractPullRequest, username=None,
         return comment
 
 
+def find_status_comment(pull_request: AbstractPullRequest,
+                        username) -> AbstractComment:
+    """Return the latest status comment posted by the bot, if any."""
+    for comment in reversed(pull_request.comments):
+        if comment.author == username and comment.text.rstrip().endswith(
+                STATUS_MARKER):
+            return comment
+
+
 def _send_comment(settings, pull_request: AbstractPullRequest, msg: str,
-                  dont_repeat_if_in_history=10) -> None:
+                  dont_repeat_if_in_history=10, updatable=False) -> None:
     """Comment a pull request.
 
     Before posting:
         Check that the same comment was not already posted in the recent pull
         request comments history.
         Optionally (if settings.interactive is set) ask confirmation to the
         user.
+        If `updatable` is set, edit the bot's previous status comment in place
+        (when there is one) instead of posting a new comment.
 
     Raises:
         CommentAlreadyExists: if the comment was already posted.
@@ -80,6 +94,21 @@ def _send_comment(settings, pull_request: AbstractPullRequest, msg: str,
         if not confirm('Do you want to send this comment?'):
             return
 
+    if updatable:
+        status_comment = find_status_comment(pull_request, settings.robot)
+        msg = f'{msg}\n\n{STATUS_MARKER}'
+        if status_comment is not None:
+            if status_comment.text.strip() == msg.strip():
+                raise exceptions.CommentAlreadyExists(
+                    "The status comment is already up to date."
+                )
+            try:
+                LOG.debug('UPDATING STATUS COMMENT %s', msg)
+                status_comment.edit(msg)
+                return
+            except NotImplementedError:
+                LOG.debug('Comments cannot be edited, posting a new one.')
+
     LOG.debug('SENDING MESSAGE %s', msg)
     pull_request.add_comment(msg)
 
@@ -104,6 +133,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,
+                      updatable=comment.updatable)
     except exceptions.CommentAlreadyExists:
         LOG.info("Comment '%s' already posted", comment.__class__.__name__)