fix: resolve issue #295 - bert-e: pr status should be displayed at all time

bert_e/git_host/base.py

@@ -257,6 +257,10 @@
     def delete(self) -> None:
         """Delete the comment."""
 
+    def update(self, msg: str) -> None:
+        """Replace the comment's contents (optional feature)."""
+        raise NotImplementedError('"update" feature is not available')
+
     @property
     @abstractmethod
     def author(self) -> str:

bert_e/git_host/bitbucket/__init__.py

@@ -348,12 +348,15 @@ def full_name(self):
         return self['destination']['repository']['full_name']
 
     def add_comment(self, msg):
-        return Comment.create(
+        comment = Comment.create(
             self.client,
             data=msg,
             full_name=self.full_name(),
             pull_request_id=self['id']
         )
+        # invalidate the cache so that the new comment is visible
+        self._comments = None
+        return comment
 
     def set_bot_status(self, status: str | None, title: str, summary: str):
         raise NotImplementedError('"set_bot_status" feature '
@@ -489,6 +492,9 @@ def delete(self):
                               pull_request_id=self.data['pullrequest']['id'],
                               comment_id=self.id)
 
+    def update(self, msg):
+        raise NotImplementedError('"update" feature is not available')
+
     @classmethod
     def create(cls, client, data, **kwargs):
         return super().create(client, {'content': {'raw': data}}, **kwargs)

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,12 @@ def id(self) -> int:
     def delete(self) -> None:
         self.client.delete(self.data['url'])
 
+    def update(self, msg: str) -> None:
+        data = self.client.patch(self.data['url'],
+                                 data=json.dumps({'body': msg}))
+        # go through the schema so that e.g. dates stay datetime objects
+        self.data = self.load(data, self.SCHEMA).data
+
 
 class CheckRun(base.AbstractGitHostObject):
     GET_URL = '/repos/{owner}/{repo}/check-runs/{id}'

bert_e/git_host/mock.py

@@ -406,6 +406,10 @@
 
 class CommentController(Controller, base.AbstractComment):
 
+    def update(self, msg):
+        self.controlled.content = {"raw": msg, "markup": "markdown",
+                                   "html": msg}
+
     def delete(self):
         self.controlled.delete()
 

bert_e/settings.py

@@ -195,6 +195,8 @@ class Meta:
     github_installation_id = fields.Int(required=False, load_default='')
 
     send_bot_status = fields.Bool(required=False, load_default=False)
+    # keep a single, always up-to-date status comment on each pull request
+    status_comment = fields.Bool(required=False, load_default=False)
 
     @pre_load(pass_many=True)
     def load_env(self, data, **kwargs):

bert_e/tests/unit/test_status_comment.py

@@ -0,0 +1,241 @@
+import pytest
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+from bert_e import exceptions
+from bert_e.workflow import pr_utils
+
+
+class FakeComment:
+    def __init__(self, author, text):
+        self.author = author
+        self.text = text
+
+    def update(self, msg):
+        self.text = msg
+
+
+class FakePR:
+    def __init__(self):
+        self.comments = []
+        self.description = 'original description'
+
+    def add_comment(self, msg):
+        self.comments.append(FakeComment('bert-e', msg))
+
+    def set_bot_status(self, *args, **kwargs):
+        pass
+
+
+def settings(**kw):
+    base = dict(robot='bert-e', no_comment=False, interactive=False,
+                send_bot_status=False, status_comment=True)
+    base.update(kw)
+    return SimpleNamespace(**base)
+
+
+def exc(cls=exceptions.QueueConflict, **kw):
+    e = MagicMock(spec=cls)
+    e.title = cls.__name__
+    e.status = None
+    e.dont_repeat_if_in_history = 0
+    e.__str__ = lambda self: 'details ' + kw.get('t', '')
+    return e
+
+
+def test_status_comment_single_and_updated():
+    pr = FakePR()
+    s = settings()
+    pr_utils.notify_user(s, pr, exc(t='one'))
+    pr_utils.notify_user(s, pr, exc(exceptions.Conflict, t='two'))
+    status = [c for c in pr.comments
+              if c.text.startswith(pr_utils.STATUS_COMMENT_MARKER)]
+    assert len(status) == 1
+    assert 'details two' in status[0].text
+    assert pr.description == 'original description'
+
+
+def test_status_comment_disabled_by_default():
+    pr = FakePR()
+    pr_utils.notify_user(settings(status_comment=False), pr, exc())
+    assert not any(c.text.startswith(pr_utils.STATUS_COMMENT_MARKER)
+                   for c in pr.comments)
+
+
+def test_status_comment_ignores_other_authors():
+    pr = FakePR()
+    pr.comments.append(FakeComment(
+        'someone', pr_utils.STATUS_COMMENT_MARKER + ' fake'))
+    pr_utils.notify_user(settings(), pr, exc())
+    assert sum(c.author == 'bert-e' and
+               c.text.startswith(pr_utils.STATUS_COMMENT_MARKER)
+               for c in pr.comments) == 1
+
+
+# --- real GitHub / Bitbucket comment classes -------------------------------
+
+def _github_comment(client, body='old'):
+    from bert_e.git_host.github import Comment
+    return Comment(
+        client=client, _validate=False,
+        id=1, body=body, url='https://api.github.com/c/1',
+        user={'login': 'Bert-E'}, created_at='2020-01-01T00:00:00Z')
+
+
+def test_github_comment_update_sends_patch_request():
+    import json
+    from bert_e.git_host.github import Client
+    client = Client(login='l', password='p', email='e@o.com')
+    client.session = MagicMock()
+    client.session.patch.return_value = MagicMock(
+        text=json.dumps({'id': 1, 'body': 'new',
+                         'url': 'https://api.github.com/c/1',
+                         'user': {'id': 1, 'login': 'Bert-E'},
+                         'created_at': '2020-01-01T00:00:00Z'}))
+    comment = _github_comment(client)
+    comment.update('new')
+    client.session.post.assert_not_called()
+    args, kwargs = client.session.patch.call_args
+    assert args[0] == 'https://api.github.com/c/1'
+    assert json.loads(kwargs['data']) == {'body': 'new'}
+    assert comment.text == 'new'
+    assert comment.author == 'bert-e'
+
+
+def test_bitbucket_comment_has_no_update():
+    from bert_e.git_host.bitbucket import Comment
+    comment = Comment(client=None, _validate=False)
+    with pytest.raises(NotImplementedError):
+        comment.update('x')
+
+
+def test_notify_user_without_update_support_replaces_status():
+    class NoUpdateComment(FakeComment):
+        def update(self, msg):
+            raise NotImplementedError
+
+    pr = FakePR()
+    old = NoUpdateComment('bert-e', pr_utils.STATUS_COMMENT_MARKER + ' stale')
+    old.delete = lambda: pr.comments.remove(old)
+    pr.comments.append(old)
+    pr_utils.notify_user(settings(), pr, exc(t='one'))
+    status = [c for c in pr.comments
+              if c.text.startswith(pr_utils.STATUS_COMMENT_MARKER)]
+    assert len(status) == 1 and 'stale' not in status[0].text
+    assert pr.comments[-1].text == 'details one'
+
+
+def test_status_comment_skipped_in_interactive_mode():
+    pr = FakePR()
+    pr_utils._update_status_comment(settings(interactive=True), pr,
+                                    exc(t='one'))
+    assert pr.comments == []
+
+
+@pytest.mark.parametrize('cls', [exceptions.StatusReport,
+                                 exceptions.UnknownCommand,
+                                 exceptions.HelpMessage,
+                                 exceptions.InitMessage,
+                                 exceptions.CommandNotImplemented,
+                                 exceptions.ResetComplete,
+                                 exceptions.LossyResetWarning,
+                                 exceptions.IncorrectCommandSyntax,
+                                 exceptions.NotEnoughCredentials,
+                                 exceptions.NotAuthor,
+                                 exceptions.IntegrationDataCreated,
+                                 exceptions.PendingHotfixVersionReminder])
+def test_status_comment_not_replaced_by_command_replies(cls):
+    pr = FakePR()
+    pr_utils._update_status_comment(settings(), pr, exc(cls, t='one'))
+    assert pr.comments == []
+
+
+def test_default_dedupe_minus_one_does_not_duplicate():
+    pr = FakePR()
+    s = settings()
+    e = exc(t='one')
+    e.dont_repeat_if_in_history = -1
+    pr_utils.notify_user(s, pr, e)
+    pr_utils.notify_user(s, pr, e)
+    assert len([c for c in pr.comments if c.text == 'details one']) == 1
+
+
+def test_bitbucket_add_comment_invalidates_cache():
+    from bert_e.git_host.bitbucket import PullRequest, Comment
+    pr = PullRequest.__new__(PullRequest)
+    pr._comments = ['stale']
+    pr.client = None
+    pr.full_name = lambda: 'o/r'
+    pr._json_data = {'id': 1}
+    orig = Comment.create
+    Comment.create = classmethod(lambda cls, *a, **k: 'new')
+    try:
+        assert pr.add_comment('x') == 'new'
+    finally:
+        Comment.create = orig
+    assert not pr._comments
+
+
+def test_github_update_keeps_datetime():
+    import json
+    import datetime
+    from bert_e.git_host.github import Client
+    client = Client(login='l', password='p', email='e@o.com')
+    client.session = MagicMock()
+    client.session.patch.return_value = MagicMock(
+        text=json.dumps({'id': 1, 'body': 'new',
+                         'url': 'https://api.github.com/c/1',
+                         'user': {'id': 1, 'login': 'Bert-E'},
+                         'created_at': '2020-01-01T00:00:00Z'}))
+    comment = _github_comment(client)
+    comment.update('new')
+    assert isinstance(comment.created_on, datetime.datetime)
+
+
+# --- interaction with comment deduplication --------------------------------
+
+def test_status_comment_does_not_trigger_dedupe_of_regular_comment():
+    pr = FakePR()
+    s = settings()
+    e = exc(t='one')
+    e.dont_repeat_if_in_history = 10
+    pr_utils.notify_user(s, pr, e)
+    regular = [c for c in pr.comments if c.text == 'details one']
+    assert len(regular) == 1
+    # repeated notification: regular comment deduped, status stays single
+    pr_utils.notify_user(s, pr, e)
+    assert len([c for c in pr.comments if c.text == 'details one']) == 1
+    assert len([c for c in pr.comments if c.text.startswith(
+        pr_utils.STATUS_COMMENT_MARKER)]) == 1
+
+
+def test_status_comment_updated_even_when_regular_comment_deduped():
+    pr = FakePR()
+    s = settings()
+    e = exc(t='one')
+    e.dont_repeat_if_in_history = 10
+    pr_utils.notify_user(s, pr, e)
+    e2 = exc(exceptions.Conflict, t='one')
+    e2.dont_repeat_if_in_history = 10
+    pr_utils.notify_user(s, pr, e2)
+    status = [c for c in pr.comments
+              if c.text.startswith(pr_utils.STATUS_COMMENT_MARKER)]
+    assert len(status) == 1
+    assert 'Conflict' in status[0].text
+
+
+def test_find_comment_skips_status_comment_for_regular_lookup():
+    pr = FakePR()
+    pr_utils.notify_user(settings(), pr, exc(t='one'))
+    found = pr_utils.find_comment(pr, 'bert-e', 'details one', 10)
+    assert found is not None and found.text == 'details one'
+
+
+def test_find_comment_include_status_finds_status_comment():
+    pr = MagicMock()
+    c = MagicMock()
+    c.author = 'bert-e'
+    c.text = pr_utils.STATUS_COMMENT_MARKER + '\nstatus'
+    pr.comments = [c]
+    assert pr_utils.find_comment(pr, 'bert-e') is None
+    assert pr_utils.find_comment(pr, 'bert-e', include_status=True) is c

bert_e/workflow/gitwaterflow/__init__.py

@@ -276,7 +276,8 @@ def send_greetings(job):
 
     """
     username = job.settings.robot
-    if find_comment(job.pull_request, username=username):
+    if find_comment(job.pull_request, username=username,
+                    include_status=True):
         return
 
     init_message = messages.InitMessage(