Use latest action comment on PR for require_comment

python/publish/publisher.py

@@ -10,6 +10,7 @@
 from github.CheckRun import CheckRun
 from github.CheckRunAnnotation import CheckRunAnnotation
 from github.PullRequest import PullRequest
+from github.IssueComment import IssueComment
 
 from publish import hide_comments_mode_orphaned, hide_comments_mode_all_but_latest, hide_comments_mode_off, \
     comment_mode_off, comment_mode_create, comment_mode_update, \
@@ -381,31 +382,80 @@ def publish_comment(self,
         all_tests, skipped_tests = get_all_tests_list(cases), get_skipped_tests_list(cases)
         test_changes = SomeTestChanges(before_all_tests, all_tests, before_skipped_tests, skipped_tests)
 
+        # we need fetch the latest comment if comment_condition != comment_condition_always
+        # or self._settings.comment_mode == comment_mode_update
+        latest_comment = None
+        if self._settings.comment_condition != comment_condition_always or self._settings.comment_mode == comment_mode_update:
+            latest_comment = self.get_latest_comment(pull_request)
+        latest_comment_body = latest_comment.body if latest_comment else None
+
         # are we required to create a comment on this PR?
-        if not self.require_comment(stats, stats_with_delta, test_changes):
+        if not self.require_comment(stats, stats_with_delta, test_changes, latest_comment_body):
             logger.info(f'No comment required as comment_on is {self._settings.comment_condition}')
             return
 
         details_url = check_run.html_url if check_run else None
         summary = get_long_summary_md(stats_with_delta, details_url, test_changes, self._settings.test_changes_limit)
         body = f'## {title}\n{summary}'
 
-        # reuse existing commend when comment_mode == comment_mode_update
-        # if none exists or comment_mode != comment_mode_update, create new comment
-        if self._settings.comment_mode != comment_mode_update or not self.reuse_comment(pull_request, body):
+        # reuse existing comment when comment_mode == comment_mode_update, otherwise create new comment
+        if self._settings.comment_mode == comment_mode_update and latest_comment is not None:
+            self.reuse_comment(latest_comment, body)
+            logger.info(f'edited comment for pull request #{pull_request.number}: {latest_comment.html_url}')
+        else:
             comment = pull_request.create_issue_comment(body)
             logger.info(f'created comment for pull request #{pull_request.number}: {comment.html_url}')
 
+    @staticmethod
+    def comment_has_changes(comment_body: Optional[str]) -> bool:
+        if comment_body is None:
+            return False
+        end = comment_body.lower().find('results for commit')
+        if end < 0:
+            return False
+        comment_body = comment_body[:end]
+
+        # remove links
+        comment_body = re.sub(r'\([^)]*\)', '<link>', comment_body)
+        # replace ' ' with some non-whitespace string, it separates columns in the comment
+        comment_body = comment_body.replace(' ', '⋯')
+
+        m = re.search(r'[-+](\s*[0-9]+)+\s+(([^0-9\s])|\n)', comment_body)
+        return m is not None
+
+    @classmethod
+    def comment_has_failures(cls, comment_body: Optional[str]) -> bool:
+        return cls._comment_has(comment_body, r'\[:x:]')
+
+    @classmethod
+    def comment_has_errors(cls, comment_body: Optional[str]) -> bool:
+        return cls._comment_has(comment_body, r'(\[:fire:]|errors)')
+
+    @staticmethod
+    def _comment_has(comment_body: Optional[str], symbol: str) -> bool:
+        if comment_body is None:
+            return False
+
+        end = comment_body.lower().find('results for commit')
+        if end < 0:
+            return False
+        comment_body = comment_body[:end]
+
+        # we assume '00 ...' indicates multiple failures / errors (more than 99)
+        m = re.search(r'([0-9][0-9]|[1-9])\s' + symbol, comment_body)
+        return m is not None
+
     def require_comment(self,
                         stats: UnitTestRunResults,
                         stats_with_delta: UnitTestRunDeltaResults,
-                        test_changes: SomeTestChanges) -> bool:
+                        test_changes: SomeTestChanges,
+                        comment_body: Optional[str]) -> bool:
         return (self._settings.comment_condition == comment_condition_always or
-                self._settings.comment_condition == comment_condition_changes and (stats_with_delta is None or stats_with_delta.has_changes or test_changes.has_changes) or
-                self._settings.comment_condition == comment_condition_failures and (stats.has_failures or stats.has_errors) or
-                self._settings.comment_condition == comment_condition_errors and stats.has_errors)
+                self._settings.comment_condition == comment_condition_changes and (self.comment_has_changes(comment_body) or stats_with_delta is None or stats_with_delta.has_changes or test_changes.has_changes) or
+                self._settings.comment_condition == comment_condition_failures and (self.comment_has_failures(comment_body) or self.comment_has_errors(comment_body) or stats.has_failures or stats.has_errors) or
+                self._settings.comment_condition == comment_condition_errors and (self.comment_has_errors(comment_body) or stats.has_errors))
 
-    def reuse_comment(self, pull: PullRequest, body: str) -> bool:
+    def get_latest_comment(self, pull: PullRequest) -> Optional[IssueComment]:
         # get comments of this pull request
         comments = self.get_pull_request_comments(pull, order_by_updated=True)
 
@@ -414,23 +464,22 @@ def reuse_comment(self, pull: PullRequest, body: str) -> bool:
 
         # if there is no such comment, stop here
         if len(comments) == 0:
-            return False
+            return None
 
-        # edit last comment
+        # fetch latest action comment
         comment_id = comments[-1].get("databaseId")
+        return pull.get_issue_comment(comment_id)
+
+    def reuse_comment(self, comment: IssueComment, body: str):
         if ':recycle:' not in body:
             body = f'{body}\n:recycle: This comment has been updated with latest results.'
 
         try:
-            comment = pull.get_issue_comment(comment_id)
             comment.edit(body)
-            logger.info(f'edited comment for pull request #{pull.number}: {comment.html_url}')
         except Exception as e:
-            self._gha.warning(f'Failed to edit existing comment #{comment_id}')
+            self._gha.warning(f'Failed to edit existing comment #{comment.id}')
             logger.debug('editing existing comment failed', exc_info=e)
 
-        return True
-
     def get_base_commit_sha(self, pull_request: PullRequest) -> Optional[str]:
         if self._settings.pull_request_build == pull_request_build_mode_merge:
             if self._settings.event:

python/test/test_publisher.py

@@ -1,10 +1,12 @@
+import dataclasses
 import json
 import os
+import re
 import tempfile
 import unittest
 from collections.abc import Collection
 from datetime import datetime, timezone
-from typing import Optional, List, Mapping, Union, Any
+from typing import Optional, List, Mapping, Union, Any, Callable
 
 import github.CheckRun
 import mock
@@ -16,15 +18,27 @@
     get_error_annotation, digest_header, get_digest_from_stats, \
     all_tests_list, skipped_tests_list, none_list, \
     all_tests_label_md, skipped_tests_label_md, failed_tests_label_md, passed_tests_label_md, test_errors_label_md, \
-    duration_label_md, pull_request_build_mode_merge
+    duration_label_md, pull_request_build_mode_merge, \
+    get_long_summary_md
 from publish.github_action import GithubAction
 from publish.publisher import Publisher, Settings, PublishData
 from publish.unittestresults import UnitTestCase, ParseError, UnitTestRunResults, UnitTestRunDeltaResults, \
-    UnitTestCaseResults
+    UnitTestRunResultsOrDeltaResults, UnitTestCaseResults, get_diff_value
 
 errors = [ParseError('file', 'error', 1, 2)]
 
 
+@dataclasses.dataclass(frozen=True)
+class CommentConditionTest:
+    comment_has_changes: bool
+    comment_has_failures: bool
+    comment_has_errors: bool
+    stats_has_changes: bool
+    stats_has_failures: bool
+    stats_has_errors: bool
+    tests_has_changes: bool
+
+
 class TestPublisher(unittest.TestCase):
 
     @staticmethod
@@ -255,102 +269,135 @@ def call_mocked_publish(settings: Settings,
                       if not call[0].startswith('_logger.')]
         return mock_calls
 
-    def do_test_require_comment(self, comment_condition, tests):
+    def test_re(self):
+        m = re.search(r'[-+](\s*[0-9]+)+\s+(([^0-9\s])|\n)', '- 1 \n')
+        self.assertIsNotNone(m)
+
+    def test_comment_has_changes(self):
+        with self.subTest(comment=None):
+            self.assertEqual(Publisher.comment_has_changes(None), False)
+
+        for mag in [1, 10, 100, 1000]:
+            kwargs = {
+                'files': get_diff_value(1+mag, 1+mag),
+                'errors': [],
+                'suites': get_diff_value(2+mag, 2+mag),
+                'duration': get_diff_value(3+mag, 3+mag, 'duration'),
+
+                'tests': get_diff_value(22+mag, 22+mag),
+                'tests_succ': get_diff_value(4+mag, 4+mag),
+                'tests_skip': get_diff_value(5+mag, 5+mag),
+                'tests_fail': get_diff_value(6+mag, 6+mag),
+                'tests_error': get_diff_value(7+mag, 7+mag),
+
+                'runs': get_diff_value(38+mag, 38+mag),
+                'runs_succ': get_diff_value(8+mag, 8+mag),
+                'runs_skip': get_diff_value(9+mag, 9+mag),
+                'runs_fail': get_diff_value(10+mag, 10+mag),
+                'runs_error': get_diff_value(11+mag, 11+mag),
+
+                'commit': 'commit',
+                'reference_type': 'ref',
+                'reference_commit': 'base'
+            }
+
+            # comments with changes
+            for field in [None, 'files', 'suites', 'duration',
+                          'tests', 'tests_succ', 'tests_skip', 'tests_fail', 'tests_error',
+                          'runs', 'runs_succ', 'runs_skip', 'runs_fail', 'runs_error']:
+                for change in {-1, +1, mag, -mag}:
+                    with self.subTest(field=field, magnitude=mag,   change=change):
+                        kwargs2 = {k: v if k != field else {k2: v2 + change if k2 == 'delta' else v2
+                                                            for k2, v2 in v.items()}
+                                   for k, v in kwargs.items()}
+                        stats = UnitTestRunDeltaResults(**kwargs2)
+                        summary = get_long_summary_md(stats, 'http://details')
+                        comment = f'## title\n{summary}'
+                        self.assertEqual(Publisher.comment_has_changes(comment), stats.has_changes, msg=comment)
+
+    def test_comment_has_failures(self):
+        self.do_test_comment_has(lambda comment: Publisher.comment_has_failures(comment), lambda stats: stats.has_failures)
+
+    def test_comment_has_errors(self):
+        self.do_test_comment_has(lambda comment: Publisher.comment_has_errors(comment), lambda stats: stats.has_errors)
+
+    def do_test_comment_has(self,
+                            actual: Callable[[str], bool],
+                            expected: Callable[[UnitTestRunResultsOrDeltaResults], bool]):
+        with self.subTest(comment=None):
+            self.assertEqual(Publisher.comment_has_failures(None), False)
+
+        kwargs = {
+            'errors': [],
+            'commit': 'commit'
+        }
+        fields = ['files', 'suites', 'duration',
+                  'tests', 'tests_succ', 'tests_skip', 'tests_fail', 'tests_error',
+                  'runs', 'runs_succ', 'runs_skip', 'runs_fail', 'runs_error']
+        kwargs.update(**{field: 0 for field in fields})
+
+        for mag in [1, 10, 100, 1000]:
+            for field in fields + ['errors']:
+                with self.subTest(field=field, magnitude=mag):
+                    kwargs2 = {k: mag if k == field and field != 'errors' else v for k, v in kwargs.items()}
+                    stats = UnitTestRunResults(**kwargs2)
+                    if field == 'errors':
+                        stats = stats.with_errors(errors * mag)
+                    summary = get_long_summary_md(stats, 'http://details')
+                    comment = f'## title\n{summary}'
+                    print(f'field={field} stats={stats} summary={summary}')
+                    self.assertEqual(actual(comment), expected(stats), msg=comment)
+
+    def do_test_require_comment(self, comment_condition, test_expectation: Callable[["CommentConditionTest"], bool]):
+        tests = [(test, test_expectation(test)) for test in self.comment_condition_tests]
+
         publisher = mock.MagicMock(Publisher)
         publisher._settings = self.create_settings(comment_condition=comment_condition)
 
-        for has_failures, has_errors, stats_has_changes, tests_has_changes, expected in tests:
-            with self.subTest(has_failures=has_failures, has_errors=has_errors,
-                              stats_has_changes=stats_has_changes, tests_has_changes=tests_has_changes):
-                stats = mock.MagicMock(has_failures=has_failures, has_errors=has_errors)
-                stats_with_delta = mock.MagicMock(has_changes=stats_has_changes)
-                test_changes = mock.MagicMock(has_changes=tests_has_changes)
-                required = Publisher.require_comment(publisher, stats, stats_with_delta, test_changes)
+        for test, expected in tests:
+            with self.subTest(test):
+                publisher.comment_has_changes = mock.Mock(return_value=test.comment_has_changes)
+                publisher.comment_has_failures = mock.Mock(return_value=test.comment_has_failures)
+                publisher.comment_has_errors = mock.Mock(return_value=test.comment_has_errors)
+                stats = mock.MagicMock(has_failures=test.stats_has_failures, has_errors=test.stats_has_errors)
+                stats_with_delta = mock.MagicMock(has_changes=test.stats_has_changes)
+                test_changes = mock.MagicMock(has_changes=test.tests_has_changes)
+                required = Publisher.require_comment(publisher, stats, stats_with_delta, test_changes, 'body')
                 self.assertEqual(required, expected)
 
+    comment_condition_tests = [CommentConditionTest(comment_has_changes, comment_has_failures, comment_has_errors,
+                                                    stat_has_changes, stat_has_failures, stat_has_errors, test_has_changes)
+                               for comment_has_changes in [False, True]
+                               for comment_has_failures in [False, True]
+                               for comment_has_errors in [False, True]
+                               for stat_has_changes in [False, True]
+                               for stat_has_failures in [False, True]
+                               for stat_has_errors in [False, True]
+                               for test_has_changes in [False, True]]
+
     def test_require_comment_always(self):
-        self.do_test_require_comment(comment_condition_always, [
-            # failures, errors, changes, test changes
-            (False, False, False, False, True),
-            (False, False, False, True, True),
-            (False, False, True, False, True),
-            (False, False, True, True, True),
-            (False, True, False, False, True),
-            (False, True, False, True, True),
-            (False, True, True, False, True),
-            (False, True, True, True, True),
-            (True, False, False, False, True),
-            (True, False, False, True, True),
-            (True, False, True, False, True),
-            (True, False, True, True, True),
-            (True, True, False, False, True),
-            (True, True, False, True, True),
-            (True, True, True, False, True),
-            (True, True, True, True, True)
-        ])
+        self.do_test_require_comment(
+            comment_condition_always,
+            lambda _: True
+        )
 
     def test_require_comment_changes(self):
-        self.do_test_require_comment(comment_condition_changes, [
-            # failures, errors, changes, test changes
-            (False, False, False, False, False),
-            (False, False, False, True, True),
-            (False, False, True, False, True),
-            (False, False, True, True, True),
-            (False, True, False, False, False),
-            (False, True, False, True, True),
-            (False, True, True, False, True),
-            (False, True, True, True, True),
-            (True, False, False, False, False),
-            (True, False, False, True, True),
-            (True, False, True, False, True),
-            (True, False, True, True, True),
-            (True, True, False, False, False),
-            (True, True, False, True, True),
-            (True, True, True, False, True),
-            (True, True, True, True, True)
-        ])
+        self.do_test_require_comment(
+            comment_condition_changes,
+            lambda test: test.comment_has_changes or test.stats_has_changes or test.test_has_changes
+        )
 
     def test_require_comment_failures(self):
-        self.do_test_require_comment(comment_condition_failures, [
-            # failures, errors, changes, test changes
-            (False, False, False, False, False),
-            (False, False, False, True, False),
-            (False, False, True, False, False),
-            (False, False, True, True, False),
-            (False, True, False, False, True),
-            (False, True, False, True, True),
-            (False, True, True, False, True),
-            (False, True, True, True, True),
-            (True, False, False, False, True),
-            (True, False, False, True, True),
-            (True, False, True, False, True),
-            (True, False, True, True, True),
-            (True, True, False, False, True),
-            (True, True, False, True, True),
-            (True, True, True, False, True),
-            (True, True, True, True, True)
-        ])
+        self.do_test_require_comment(
+            comment_condition_failures,
+            lambda test: test.comment_has_failures or test.comment_has_errors or test.stats_has_failures or test.stats_has_errors
+        )
 
     def test_require_comment_errors(self):
-        self.do_test_require_comment(comment_condition_errors, [
-            # failures, errors, changes, test changes
-            (False, False, False, False, False),
-            (False, False, False, True, False),
-            (False, False, True, False, False),
-            (False, False, True, True, False),
-            (False, True, False, False, True),
-            (False, True, False, True, True),
-            (False, True, True, False, True),
-            (False, True, True, True, True),
-            (True, False, False, False, False),
-            (True, False, False, True, False),
-            (True, False, True, False, False),
-            (True, False, True, True, False),
-            (True, True, False, False, True),
-            (True, True, False, True, True),
-            (True, True, True, False, True),
-            (True, True, True, True, True)
-        ])
+        self.do_test_require_comment(
+            comment_condition_errors,
+            lambda test: test.comment_has_errors or test.stats_has_errors
+        )
 
     def test_publish_without_comment(self):
         settings = self.create_settings(comment_mode=comment_mode_off, hide_comment_mode=hide_comments_mode_off)