Handle incomplete / none JSON elements

python/publish/__init__.py

@@ -5,7 +5,7 @@
 import re
 from collections import defaultdict
 from dataclasses import dataclass
-from typing import List, Any, Union, Optional, Tuple, Mapping, Iterator, Set, Iterable
+from typing import List, Any, Union, Optional, Tuple, Mapping, Iterator, Set, Iterable, Dict
 
 from publish.unittestresults import Numeric, UnitTestSuite, UnitTestCaseResults, UnitTestRunResults, \
     UnitTestRunDeltaResults, UnitTestRunResultsOrDeltaResults, ParseError
@@ -151,6 +151,24 @@ def removed_skips(self) -> Optional[Set[str]]:
         return skipped_before.intersection(removed)
 
 
+def get_json_path(json: Dict[str, Any], path: Union[str, List[str]]) -> Any:
+    if isinstance(path, str):
+        path = path.split('.')
+
+    if path[0] not in json:
+        return None
+
+    elem = json[path[0]]
+
+    if len(path) > 1:
+        if isinstance(elem, dict):
+            return get_json_path(elem, path[1:])
+        else:
+            return None
+    else:
+        return elem
+
+
 def utf8_character_length(c: int) -> int:
     if c >= 0x00010000:
         return 4

python/publish/publisher.py

@@ -13,7 +13,7 @@
 from github.PullRequest import PullRequest
 from github.IssueComment import IssueComment
 
-from publish import __version__, comment_mode_off, digest_prefix, restrict_unicode_list, \
+from publish import __version__, get_json_path, comment_mode_off, digest_prefix, restrict_unicode_list, \
     comment_mode_always, comment_mode_changes, comment_mode_changes_failures, comment_mode_changes_errors, \
     comment_mode_failures, comment_mode_errors, \
     get_stats_from_digest, digest_header, get_short_summary, get_long_summary_md, \
@@ -206,7 +206,7 @@ def publish(self,
             check_run = None
             before_check_run = None
             if self._settings.compare_earlier:
-                before_commit_sha = self._settings.event.get('before')
+                before_commit_sha = get_json_path(self._settings.event, 'before')
                 logger.debug(f'comparing against before={before_commit_sha}')
                 before_check_run = self.get_check_run(before_commit_sha)
         else:
@@ -227,8 +227,8 @@ def publish(self,
                 logger.info('Commenting on pull requests disabled')
 
     def get_pull_from_event(self) -> Optional[PullRequest]:
-        number = self._settings.event.get('pull_request', {}).get('number')
-        repo = self._settings.event.get('pull_request', {}).get('base', {}).get('repo', {}).get('full_name')
+        number = get_json_path(self._settings.event, 'pull_request.number')
+        repo = get_json_path(self._settings.event, 'pull_request.base.repo.full_name')
         if number is None or repo is None or repo != self._settings.repo:
             return None
 
@@ -390,7 +390,7 @@ def publish_check(self,
         before_stats = None
         before_check_run = None
         if self._settings.compare_earlier:
-            before_commit_sha = self._settings.event.get('before')
+            before_commit_sha = get_json_path(self._settings.event, 'before')
             logger.debug(f'comparing against before={before_commit_sha}')
             before_check_run = self.get_check_run(before_commit_sha)
             before_stats = self.get_stats_from_check_run(before_check_run) if before_check_run is not None else None
@@ -686,7 +686,7 @@ def get_base_commit_sha(self, pull_request: PullRequest) -> Optional[str]:
             if self._settings.event:
                 # for pull request events we take the other parent of the merge commit (base)
                 if self._settings.event_name == 'pull_request':
-                    return self._settings.event.get('pull_request', {}).get('base', {}).get('sha')
+                    return get_json_path(self._settings.event, 'pull_request.base.sha')
                 # for workflow run events we should take the same as for pull request events,
                 # but we have no way to figure out the actual merge commit and its parents
                 # we do not take the base sha from pull_request as it is not immutable
@@ -728,18 +728,13 @@ def get_pull_request_comments(self, pull: PullRequest, order_by_updated: bool) -
             "POST", self._settings.graphql_url, input=query
         )
 
-        return data \
-            .get('data', {}) \
-            .get('repository', {}) \
-            .get('pullRequest', {}) \
-            .get('comments', {}) \
-            .get('nodes')
+        return get_json_path(data, 'data.repository.pullRequest.comments.nodes')
 
     def get_action_comments(self, comments: List[Mapping[str, Any]], is_minimized: Optional[bool] = False):
         comment_body_start = f'## {self._settings.comment_title}\n'
         comment_body_indicators = ['\nresults for commit ', '\nResults for commit ']
         return list([comment for comment in comments
-                     if comment.get('author', {}).get('login') == self._settings.actor
+                     if get_json_path(comment, 'author.login') == self._settings.actor
                      and (is_minimized is None or comment.get('isMinimized') == is_minimized)
                      and comment.get('body', '').startswith(comment_body_start)
                      and any(indicator in comment.get('body', '') for indicator in comment_body_indicators)])

python/test/test_publish.py

@@ -5,7 +5,7 @@
 import mock
 
 from publish import __version__, Annotation, UnitTestSuite, UnitTestRunResults, UnitTestRunDeltaResults, CaseMessages, \
-    get_error_annotation, get_digest_from_stats, \
+    get_json_path, get_error_annotation, get_digest_from_stats, \
     all_tests_label_md, skipped_tests_label_md, failed_tests_label_md, passed_tests_label_md, test_errors_label_md, \
     duration_label_md, SomeTestChanges, abbreviate, abbreviate_bytes, get_test_name, get_formatted_digits, \
     get_magnitude, get_delta, as_short_commit, as_delta, as_stat_number, as_stat_duration, get_stats_from_digest, \
@@ -29,6 +29,22 @@ class PublishTest(unittest.TestCase):
     old_locale = None
     details = [UnitTestSuite('suite', 7, 3, 2, 1, 'std-out', 'std-err')]
 
+    def test_get_json_path(self):
+        detail = {'a': 'A', 'b': 'B', 'c': ['d'], 'e': {}, 'f': None}
+        json = {'id': 1, 'name': 'Name', 'detail': detail}
+
+        self.assertEqual(None, get_json_path(json, 'not there'))
+        self.assertEqual(1, get_json_path(json, 'id'))
+        self.assertEqual('Name', get_json_path(json, 'name'))
+        self.assertEqual(detail, get_json_path(json, 'detail'))
+        self.assertEqual('A', get_json_path(json, 'detail.a'))
+        self.assertEqual(None, get_json_path(json, 'detail.a.g'))
+        self.assertEqual(['d'], get_json_path(json, 'detail.c'))
+        self.assertEqual({}, get_json_path(json, 'detail.e'))
+        self.assertEqual(None, get_json_path(json, 'detail.e.g'))
+        self.assertEqual(None, get_json_path(json, 'detail.f'))
+        self.assertEqual(None, get_json_path(json, 'detail.f.g'))
+
     def test_test_changes(self):
         changes = SomeTestChanges(['removed-test', 'removed-skip', 'remain-test', 'remain-skip', 'skip', 'unskip'],
                                   ['remain-test', 'remain-skip', 'skip', 'unskip', 'add-test', 'add-skip'],

python/test/test_publisher.py

@@ -13,7 +13,7 @@
 import mock
 from github import Github, GithubException
 
-from publish import __version__, comment_mode_off, comment_mode_always, \
+from publish import __version__, get_json_path, comment_mode_off, comment_mode_always, \
     comment_mode_changes, comment_mode_changes_failures, comment_mode_changes_errors, \
     comment_mode_failures, comment_mode_errors, Annotation, default_annotations, \
     get_error_annotation, digest_header, get_digest_from_stats, \
@@ -888,6 +888,22 @@ def test_get_pull_from_event(self):
         actual = publisher.get_pull_from_event()
         self.assertIs(actual, pr)
         repo.get_pull.assert_called_once_with(1234)
+        repo.get_pull.reset_mock()
+
+        # test with none in pull request
+        for event in [
+            {},
+            {'pull_request': None},
+            {'pull_request': {'number': 1234, 'base': None}},
+            {'pull_request': {'number': 1234, 'base': {'repo': None}}},
+            {'pull_request': {'number': 1234, 'base': {'repo': {}}}},
+        ]:
+            settings = self.create_settings(event=event)
+            publisher = Publisher(settings, gh, gha)
+
+            actual = publisher.get_pull_from_event()
+            self.assertIsNone(actual)
+            repo.get_pull.assert_not_called()
 
     def do_test_get_pulls(self,
                           settings: Settings,
@@ -911,7 +927,7 @@ def do_test_get_pulls(self,
         else:
             gh.search_issues.assert_not_called()
             if event_pull_request is not None and \
-                    settings.repo == settings.event.get('pull_request', {}).get('base', {}).get('repo', {}).get('full_name'):
+                    settings.repo == get_json_path(settings.event, 'pull_request.base.repo.full_name'):
                 repo.get_pull.assert_called_once_with(event_pull_request.number)
                 commit.get_pulls.assert_not_called()
             else:
@@ -2621,6 +2637,15 @@ def test_get_pull_request_comments_order_updated(self):
                     'Results for commit dee59820.\u2003± Comparison against base commit 70b5dd18.\n',
             'isMinimized': False
         },
+        # malformed comments
+        {
+            'id': 'comment nine',
+            'author': None,
+        },
+        {
+            'id': 'comment ten',
+            'author': {},
+        },
     ]
 
     def test_get_action_comments(self):