diff --git a/README.md b/README.md index 3b4f13e8..31c98fad 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,9 @@ The list of most notable options: |:-----|:-----:|:----------| |`files`|`*.xml`|File patterns to select the test result XML files, e.g. `"test-results/**/*.xml"`. Use multiline string for multiple patterns. Supports `*`, `**`, `?`, `[]`. Excludes files when starting with `!`. | |`check_name`|`"Unit Test Results"`|An alternative name for the check result.| -|`comment_title`|same as `check_name`|An alternative name for the pull request comment.| +|`comment_title`|same as `check_name`|An alternative title for the pull request comment.| +|`comment_mode`|`always`|The action posts comments to pull requests that are associated with the commit. Set to:
`always` - always comment
`changes` - comment only when changes to the target branch exist
`failures` - when failures or errors exist
`errors` - when (only) errors exist
`off` - to not create pull request comments
With `changes in failures` and `changes in errors`, it only comments when changes in the number of failures and errors exist, respectively.| +|`ignore_runs`|`false`|Does not process test run information by ignoring `` elements in the XML files, which is useful for very large XML files. This disables any check run annotations.|
Options related to Git and GitHub @@ -198,15 +200,13 @@ The list of most notable options: |:-----|:-----:|:----------| |`time_unit`|`seconds`|Time values in the XML files have this unit. Supports `seconds` and `milliseconds`.| |`job_summary`|`true`| Set to `true`, the results are published as part of the [job summary page](https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/) of the workflow run.| -|`comment_mode`|`update last`|The action posts comments to a pull request that is associated with the commit. Set to `create new` to create a new comment on each commit, `update last` to create only one comment and update later on, `off` to not create pull request comments.| |`hide_comments`|`"all but latest"`|Configures which earlier comments in a pull request are hidden by the action:
`"orphaned commits"` - comments for removed commits
`"all but latest"` - all comments but the latest
`"off"` - no hiding| |`compare_to_earlier_commit`|`true`|Test results are compared to results of earlier commits to show changes:
`false` - disable comparison, `true` - compare across commits.'| -|`test_changes_limit`|`10`|Limits the number of removed or skipped tests reported on PR comments. This report can be disabled with a value of `0`.| +|`test_changes_limit`|`10`|Limits the number of removed or skipped tests reported on pull request comments. This report can be disabled with a value of `0`.| |`report_individual_runs`|`false`|Individual runs of the same test may see different failures. Reports all individual failures when set `true`, and the first failure only otherwise.| |`deduplicate_classes_by_file_name`|`false`|De-duplicates classes with same name by their file name when set `true`, combines test results for those classes otherwise.| |`check_run_annotations`|`all tests, skipped tests`|Adds additional information to the check run. This is a comma-separated list of any of the following values:
`all tests` - list all found tests,
`skipped tests` - list all skipped tests
Set to `none` to add no extra annotations at all.| -|`check_run_annotations_branch`|`event.repository.default_branch` or `"main, master"`|Adds check run annotations only on given branches. If not given, this defaults to the default branch of your repository, e.g. `main` or `master`. Comma separated list of branch names allowed, asterisk `"*"` matches all branches. Example: `main, master, branch_one`| -|`ignore_runs`|`false`|Does not process test run information by ignoring `` elements in the XML files, which is useful for very large XML files. This disables any check run annotations.| +|`check_run_annotations_branch`|`event.repository.default_branch` or `"main, master"`|Adds check run annotations only on given branches. If not given, this defaults to the default branch of your repository, e.g. `main` or `master`. Comma separated list of branch names allowed, asterisk `"*"` matches all branches. Example: `main, master, branch_one`.| |`json_file`|no file|Results are written to this JSON file.| |`json_thousands_separator`|`" "`|Formatted numbers in JSON use this character to separate groups of thousands. Common values are "," or ".". Defaults to punctuation space (\u2008).| |`fail_on`|`"test failures"`|Configures the state of the created test result check run. With `"test failures"` it fails if any test fails or test errors occur. It never fails when set to `"nothing"`, and fails only on errors when set to `"errors"`.| diff --git a/action.yml b/action.yml index 9d4f090a..a17e0406 100644 --- a/action.yml +++ b/action.yml @@ -19,7 +19,11 @@ inputs: default: 'Unit Test Results' required: false comment_title: - description: 'Title of PR comments, defaults to value of check_name input' + description: 'An alternative title for the pull request comment. Defaults to value of check_name input' + required: false + comment_mode: + description: 'The action posts comments to pull requests that are associated with the commit. Set to: "always" - always comment, "changes" - comment only when changes to the target branch exist, "failures" - when failures or errors exist, "errors" - when (only) errors exist, "off" - to not create pull request comments. With "changes in failures" and "changes in errors", it only comments when changes in the number of failures and errors exist, respectively' + default: 'always' required: false fail_on: description: 'The created test result check run has failure state if any test fails or test errors occur. Never fails when set to "nothing", fails only on errors when set to "errors". Default is "test failures".' @@ -49,10 +53,6 @@ inputs: comment_on_pr: description: 'Deprecated, please use comment_mode instead!' required: false - comment_mode: - description: 'Control PR comments: off - disable PR comments, create new - create comments on PRs, each time a new one, update last - create comment on PR, reuse an existing one' - default: 'update last' - required: false job_summary: description: 'Set to `true`, the results are published as part of the job summary page of the workflow run' default: 'true' @@ -72,14 +72,14 @@ inputs: description: 'An alternative event name to use. Useful to replace a "workflow_run" event name with the actual source event name: github.event.workflow_run.event.' required: false test_changes_limit: - description: 'Limits the number of removed or skipped tests reported on PR comments. This report can be disabled with a value of 0. The default is 10.' + description: 'Limits the number of removed or skipped tests reported on pull request comments. This report can be disabled with a value of 0. The default is 10.' required: false check_run_annotations: description: 'Adds additional information to the check run. This is a comma-separated list of any of the following values: "all tests" - list all found tests, "skipped tests" - list all skipped tests. Set to "none" to add no extra annotations at all' default: 'all tests, skipped tests' required: false check_run_annotations_branch: - description: 'Adds check run annotations only on given branches. Comma separated list of branch names allowed, asterisk "*" matches all branches. Defaults to event.repository.default_branch or "main, master".' + description: 'Adds check run annotations only on given branches. Comma-separated list of branch names allowed, asterisk "*" matches all branches. Defaults to event.repository.default_branch or "main, master".' required: false seconds_between_github_reads: description: 'Sets the number of seconds the action waits between concurrent read requests to the GitHub API. This throttles the API usage to avoid abuse rate limits: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#abuse-rate-limits' diff --git a/composite/action.yml b/composite/action.yml index dc171219..b0d9f10c 100644 --- a/composite/action.yml +++ b/composite/action.yml @@ -19,7 +19,11 @@ inputs: default: 'Unit Test Results' required: false comment_title: - description: 'Title of PR comments, defaults to value of check_name input' + description: 'An alternative title for the pull request comment. Defaults to value of check_name input' + required: false + comment_mode: + description: 'The action posts comments to pull requests that are associated with the commit. Set to: "always" - always comment, "changes" - comment only when changes to the target branch exist, "failures" - when failures or errors exist, "errors" - when (only) errors exist, "off" - to not create pull request comments. With "changes in failures" and "changes in errors", it only comments when changes in the number of failures and errors exist, respectively' + default: 'always' required: false fail_on: description: 'The created test result check run has failure state if any test fails or test errors occur. Never fails when set to "nothing", fails only on errors when set to "errors". Default is "test failures".' @@ -49,10 +53,6 @@ inputs: comment_on_pr: description: 'Deprecated, please use comment_mode instead!' required: false - comment_mode: - description: 'Control PR comments: off - disable PR comments, create new - create comments on PRs, each time a new one, update last - create comment on PR, reuse an existing one' - default: 'update last' - required: false job_summary: description: 'Set to `true`, the results are published as part of the job summary page of the workflow run' default: 'true' @@ -72,14 +72,14 @@ inputs: description: 'An alternative event name to use. Useful to replace a "workflow_run" event name with the actual source event name: github.event.workflow_run.event.' required: false test_changes_limit: - description: 'Limits the number of removed or skipped tests reported on PR comments. This report can be disabled with a value of 0. The default is 10.' + description: 'Limits the number of removed or skipped tests reported on pull request comments. This report can be disabled with a value of 0. The default is 10.' required: false check_run_annotations: description: 'Adds additional information to the check run. This is a comma-separated list of any of the following values: "all tests" - list all found tests, "skipped tests" - list all skipped tests. Set to "none" to add no extra annotations at all' default: 'all tests, skipped tests' required: false check_run_annotations_branch: - description: 'Adds check run annotations only on given branches. Comma separated list of branch names allowed, asterisk "*" matches all branches. Defaults to event.repository.default_branch or "main, master".' + description: 'Adds check run annotations only on given branches. Comma-separated list of branch names allowed, asterisk "*" matches all branches. Defaults to event.repository.default_branch or "main, master".' required: false seconds_between_github_reads: description: 'Sets the number of seconds the action waits between concurrent read requests to the GitHub API. This throttles the API usage to avoid abuse rate limits: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#abuse-rate-limits' @@ -149,6 +149,7 @@ runs: COMMIT: ${{ inputs.commit }} CHECK_NAME: ${{ inputs.check_name }} COMMENT_TITLE: ${{ inputs.comment_title }} + COMMENT_MODE: ${{ inputs.comment_mode }} FAIL_ON: ${{ inputs.fail_on }} FILES: ${{ inputs.files }} TIME_UNIT: ${{ inputs.time_unit }} @@ -157,7 +158,6 @@ runs: IGNORE_RUNS: ${{ inputs.ignore_runs }} HIDE_COMMENTS: ${{ inputs.hide_comments }} COMMENT_ON_PR: ${{ inputs.comment_on_pr }} - COMMENT_MODE: ${{ inputs.comment_mode }} COMPARE_TO_EARLIER_COMMIT: ${{ inputs.compare_to_earlier_commit }} PULL_REQUEST_BUILD: ${{ inputs.pull_request_build }} EVENT_FILE: ${{ inputs.event_file }} diff --git a/python/publish/__init__.py b/python/publish/__init__.py index a615fd7d..767cd9a4 100644 --- a/python/publish/__init__.py +++ b/python/publish/__init__.py @@ -19,13 +19,27 @@ punctuation_space = ' ' comment_mode_off = 'off' -comment_mode_create = 'create new' -comment_mode_update = 'update last' +comment_mode_create = 'create new' # deprecated +comment_mode_update = 'update last' # deprecated +comment_mode_always = 'always' +comment_mode_changes = 'changes' +comment_mode_changes_failures = 'changes in failures' # includes comment_mode_changes_errors +comment_mode_changes_errors = 'changes in errors' +comment_mode_failures = 'failures' # includes comment_mode_errors +comment_mode_errors = 'errors' comment_modes = [ comment_mode_off, - comment_mode_create, - comment_mode_update + comment_mode_always, + comment_mode_changes, + comment_mode_changes_failures, + comment_mode_changes_errors, + comment_mode_failures, + comment_mode_errors ] +comment_modes_deprecated = { + comment_mode_create: comment_mode_always, + comment_mode_update: comment_mode_always +} fail_on_mode_nothing = 'nothing' fail_on_mode_errors = 'errors' @@ -80,6 +94,11 @@ def __init__(self, self._skipped_tests_before = set(skipped_tests_before) if skipped_tests_before is not None else None self._skipped_tests_current = set(skipped_tests_current) if skipped_tests_current is not None else None + @property + def has_changes(self) -> bool: + return (self.adds() is not None and self.removes() is not None and len(self.adds().union(self.removes())) > 0 or + self.skips() is not None and self.un_skips() is not None and len(self.skips().union(self.un_skips())) > 0) + def adds(self) -> Optional[Set[str]]: if self._all_tests_before is None or self._all_tests_current is None: return None @@ -701,7 +720,7 @@ def get_long_summary_with_digest_md(stats: UnitTestRunResultsOrDeltaResults, raise ValueError('stats must be UnitTestRunResults when no digest_stats is given') summary = get_long_summary_md(stats, details_url, test_changes, test_list_changes_limit) digest = get_digest_from_stats(stats if digest_stats is None else digest_stats) - return f'{summary}\n{digest_header}{digest}' + return f'{summary}\n{digest_header}{digest}\n' def get_case_messages(case_results: UnitTestCaseResults) -> CaseMessages: diff --git a/python/publish/publisher.py b/python/publish/publisher.py index a17d9e16..e299a52c 100644 --- a/python/publish/publisher.py +++ b/python/publish/publisher.py @@ -4,16 +4,19 @@ import os import re from dataclasses import dataclass -from typing import List, Any, Optional, Tuple, Mapping, Dict, Union +from typing import List, Set, Any, Optional, Tuple, Mapping, Dict, Union, Callable from copy import deepcopy from github import Github, GithubException 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, 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, \ get_long_summary_with_digest_md, get_error_annotations, get_case_annotations, \ get_all_tests_list_annotation, get_skipped_tests_list_annotation, get_all_tests_list, \ @@ -21,7 +24,8 @@ Annotation, SomeTestChanges from publish import logger from publish.github_action import GithubAction -from publish.unittestresults import UnitTestCaseResults, UnitTestRunResults, UnitTestRunDeltaResults, get_stats_delta +from publish.unittestresults import UnitTestCaseResults, UnitTestRunResults, UnitTestRunDeltaResults, \ + UnitTestRunResultsOrDeltaResults, get_stats_delta @dataclass(frozen=True) @@ -135,7 +139,7 @@ def publish(self, stats: UnitTestRunResults, cases: UnitTestCaseResults, conclusion: str): - logger.info(f'publishing {conclusion} results for commit {self._settings.commit}') + logger.info(f'Publishing {conclusion} results for commit {self._settings.commit}') check_run, before_check_run = self.publish_check(stats, cases, conclusion) if self._settings.job_summary: @@ -151,11 +155,11 @@ def publish(self, elif self._settings.hide_comment_mode == hide_comments_mode_all_but_latest: self.hide_all_but_latest_comments(pull) if self._settings.hide_comment_mode == hide_comments_mode_off: - logger.info('hide_comments disabled, not hiding any comments') + logger.info('Hiding comments disabled (hide_comments)') else: - logger.info(f'there is no pull request for commit {self._settings.commit}') + logger.info(f'There is no pull request for commit {self._settings.commit}') else: - logger.info('comment_on_pr disabled, not commenting on any pull requests') + logger.info('Commenting on pull requests disabled (comment_on_pr)') def get_pulls(self, commit: str) -> List[PullRequest]: # totalCount calls the GitHub API just to get the total number @@ -263,9 +267,16 @@ def get_stats_from_check_run(check_run: CheckRun) -> Optional[UnitTestRunResults for line in summary.split('\n'): logger.debug(f'summary: {line}') - pos = summary.index(digest_header) if digest_header in summary else None - if pos: - digest = summary[pos + len(digest_header):] + return Publisher.get_stats_from_summary_md(summary) + + @staticmethod + def get_stats_from_summary_md(summary: str) -> Optional[UnitTestRunResults]: + start = summary.index(digest_header) if digest_header in summary else None + if start: + digest = summary[start + len(digest_header):] + end = digest.index('\n') if '\n' in digest else None + if end: + digest = digest[:end] logger.debug(f'digest: {digest}') stats = get_stats_from_digest(digest) logger.debug(f'stats: {stats}') @@ -330,7 +341,7 @@ def publish_check(self, status='completed', conclusion=conclusion, output=output) - logger.info(f'created check {check_run.html_url}') + logger.info(f'Created check {check_run.html_url}') else: logger.debug(f'updating check with {len(annotations)} more annotations') check_run.edit(output=output) @@ -364,7 +375,7 @@ def publish_job_summary(self, summary = get_long_summary_md(stats_with_delta, details_url) markdown = f'## {title}\n{summary}' self._gha.add_to_job_summary(markdown) - logger.info(f'created job summary') + logger.info(f'Created job summary') @staticmethod def get_test_lists_from_check_run(check_run: Optional[CheckRun]) -> Tuple[Optional[List[str]], Optional[List[str]]]: @@ -420,7 +431,7 @@ def publish_comment(self, stats: UnitTestRunResults, pull_request: PullRequest, check_run: Optional[CheckRun] = None, - cases: Optional[UnitTestCaseResults] = None) -> PullRequest: + cases: Optional[UnitTestCaseResults] = None): # compare them with earlier stats base_check_run = None if self._settings.compare_earlier: @@ -443,19 +454,110 @@ def publish_comment(self, all_tests, skipped_tests = restrict_unicode_list(all_tests), restrict_unicode_list(skipped_tests) test_changes = SomeTestChanges(before_all_tests, all_tests, before_skipped_tests, skipped_tests) + # we need to fetch the latest comment unless comment mode is off (this method would have been called) or deprecated create + latest_comment = None + if self._settings.comment_mode not in [comment_mode_off, comment_mode_create]: + 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? + earlier_stats = self.get_stats_from_summary_md(latest_comment_body) if latest_comment_body else None + if not self.require_comment(stats_with_delta, earlier_stats): + logger.info(f'No pull request comment required as comment mode is {self._settings.comment_mode} (comment_mode)') + return + details_url = check_run.html_url if check_run else None summary = get_long_summary_with_digest_md(stats_with_delta, stats, 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): + # only create new comment when comment_mode == comment_mode_create, or no comment exists + if self._settings.comment_mode == comment_mode_create or latest_comment is None: comment = pull_request.create_issue_comment(body) - logger.info(f'created comment for pull request #{pull_request.number}: {comment.html_url}') + logger.info(f'Created comment for pull request #{pull_request.number}: {comment.html_url}') + else: + self.reuse_comment(latest_comment, body) + logger.info(f'Edited comment for pull request #{pull_request.number}: {latest_comment.html_url}') + + def require_comment(self, + stats: UnitTestRunResultsOrDeltaResults, + earlier_stats: Optional[UnitTestRunResults]) -> bool: + # SomeTestChanges.has_changes cannot be used here as changes between earlier comment + # and current results cannot be identified + + if self._settings.comment_mode in [comment_mode_always, comment_mode_create, comment_mode_update]: + logger.debug(f'Comment required as comment mode is {self._settings.comment_mode}') + return True + + # helper method to detect if changes require a comment + def do_changes_require_comment(earlier_stats_is_different_to: Optional[Callable[[UnitTestRunResultsOrDeltaResults], bool]], + stats_has_changes: bool, + flavour: str = '') -> bool: + in_flavour = '' + if flavour: + flavour = f'{flavour} ' + in_flavour = f'in {flavour}' + + if earlier_stats is not None and earlier_stats_is_different_to(stats): + logger.info(f'Comment required as comment mode is "{self._settings.comment_mode}" ' + f'and {flavour}statistics are different to earlier comment') + logger.debug(f'earlier: {earlier_stats}') + logger.debug(f'current: {stats.without_delta() if stats.is_delta else stats}') + return True + if not stats.is_delta: + logger.info(f'Comment required as comment mode is "{self._settings.comment_mode}" ' + f'but no delta statistics to target branch available') + return True + if stats_has_changes: + logger.info(f'Comment required as comment mode is "{self._settings.comment_mode}" ' + f'and changes {in_flavour} to target branch exist') + logger.debug(f'current: {stats}') + return True + return False + + if self._settings.comment_mode == comment_mode_changes and \ + do_changes_require_comment(earlier_stats.is_different if earlier_stats else None, + stats.has_changes): + return True + + if self._settings.comment_mode == comment_mode_changes_failures and \ + do_changes_require_comment(earlier_stats.is_different_in_failures if earlier_stats else None, + stats.has_failure_changes, + 'failures'): + return True + + if self._settings.comment_mode in [comment_mode_changes_failures, comment_mode_changes_errors] and \ + do_changes_require_comment(earlier_stats.is_different_in_errors if earlier_stats else None, + stats.has_error_changes, + 'errors'): + return True + + # helper method to detect if stats require a comment + def do_stats_require_comment(earlier_stats_require: Optional[bool], stats_require: bool, flavour: str) -> bool: + if earlier_stats is not None and earlier_stats_require: + logger.info(f'Comment required as comment mode is {self._settings.comment_mode} ' + f'and {flavour} existed in earlier comment') + return True + if stats_require: + logger.info(f'Comment required as comment mode is {self._settings.comment_mode} ' + f'and {flavour} exist in current comment') + return True + return False + + if self._settings.comment_mode == comment_mode_failures and \ + do_stats_require_comment(earlier_stats.has_failures if earlier_stats else None, + stats.has_failures, + 'failures'): + return True - return pull_request + if self._settings.comment_mode in [comment_mode_failures, comment_mode_errors] and \ + do_stats_require_comment(earlier_stats.has_errors if earlier_stats else None, + stats.has_errors, + 'errors'): + return True - def reuse_comment(self, pull: PullRequest, body: str) -> bool: + return False + + 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) @@ -464,23 +566,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: @@ -588,7 +689,7 @@ def hide_orphaned_commit_comments(self, pull: PullRequest) -> None: # hide all those comments for node_id, comment_commit_sha in comment_ids: - logger.info(f'hiding unit test result comment for commit {comment_commit_sha}') + logger.info(f'Hiding unit test result comment for commit {comment_commit_sha}') self.hide_comment(node_id) def hide_all_but_latest_comments(self, pull: PullRequest) -> None: @@ -605,5 +706,5 @@ def hide_all_but_latest_comments(self, pull: PullRequest) -> None: # hide all those comments for node_id in comment_ids: - logger.info(f'hiding unit test result comment {node_id}') + logger.info(f'Hiding unit test result comment {node_id}') self.hide_comment(node_id) diff --git a/python/publish/unittestresults.py b/python/publish/unittestresults.py index 8bd54508..335e6b22 100644 --- a/python/publish/unittestresults.py +++ b/python/publish/unittestresults.py @@ -1,7 +1,7 @@ from collections import defaultdict import dataclasses from dataclasses import dataclass -from typing import Optional, List, Mapping, Any, Union, Dict +from typing import Optional, List, Mapping, Any, Union, Dict, Callable from xml.etree.ElementTree import ParseError as XmlParseError @@ -131,6 +131,7 @@ def without_cases(self): tests_errors=self.suite_errors, ) + @dataclass(frozen=True) class UnitTestResults(ParsedUnitTestResultsWithCommit): cases: int @@ -167,6 +168,46 @@ class UnitTestRunResults: commit: str + @property + def is_delta(self) -> bool: + return False + + @property + def has_failures(self): + return self.tests_fail > 0 or self.runs_fail > 0 + + @property + def has_errors(self): + return len(self.errors) > 0 or self.tests_error > 0 or self.runs_error > 0 + + @staticmethod + def _change_fields(results: 'UnitTestRunResults') -> List[int]: + return [results.files, results.suites, + results.tests, results.tests_succ, results.tests_skip, results.tests_fail, results.tests_error, + results.runs, results.runs_succ, results.runs_skip, results.runs_fail, results.runs_error] + + @staticmethod + def _failure_fields(results: 'UnitTestRunResults') -> List[int]: + return [results.tests_fail, results.runs_fail] + + @staticmethod + def _error_fields(results: 'UnitTestRunResults') -> List[int]: + return [results.tests_error, results.runs_error] + + def is_different(self, + other: 'UnitTestRunResultsOrDeltaResults', + fields_func: Callable[['UnitTestRunResults'], List[int]] = _change_fields.__func__): + if other.is_delta: + other = other.without_delta() + + return any([left != right for left, right in zip(fields_func(self), fields_func(other))]) + + def is_different_in_failures(self, other: 'UnitTestRunResultsOrDeltaResults'): + return self.is_different(other, self._failure_fields) + + def is_different_in_errors(self, other: 'UnitTestRunResultsOrDeltaResults'): + return self.is_different(other, self._error_fields) + def with_errors(self, errors: List[ParseError]): return UnitTestRunResults( files=self.files, @@ -243,9 +284,51 @@ class UnitTestRunDeltaResults: reference_type: str reference_commit: str + @property + def is_delta(self) -> bool: + return True + + @staticmethod + def _has_changes(fields: List[Numeric]) -> bool: + return any([field.get('delta') for field in fields]) + + @property + def has_changes(self) -> bool: + return self._has_changes([self.files, self.suites, + self.tests, self.tests_succ, self.tests_skip, self.tests_fail, self.tests_error, + self.runs, self.runs_succ, self.runs_skip, self.runs_fail, self.runs_error]) + + @property + def has_failure_changes(self) -> bool: + return self._has_changes([self.tests_fail, self.runs_fail]) + + @property + def has_error_changes(self) -> bool: + return self._has_changes([self.tests_error, self.runs_error]) + + @property + def has_failures(self): + return self.tests_fail.get('number') > 0 or self.runs_fail.get('number') > 0 + + @property + def has_errors(self): + return len(self.errors) > 0 or self.tests_error.get('number') > 0 or self.runs_error.get('number') > 0 + def to_dict(self) -> Dict[str, Any]: return dataclasses.asdict(self) + def without_delta(self) -> UnitTestRunResults: + def v(value: Numeric) -> int: + return value['number'] + + def d(value: Numeric) -> int: + return value['duration'] + + return UnitTestRunResults(files=v(self.files), errors=self.errors, suites=v(self.suites), duration=d(self.duration), + tests=v(self.tests), tests_succ=v(self.tests_succ), tests_skip=v(self.tests_skip), tests_fail=v(self.tests_fail), tests_error=v(self.tests_error), + runs=v(self.runs), runs_succ=v(self.runs_succ), runs_skip=v(self.runs_skip), runs_fail=v(self.runs_fail), runs_error=v(self.runs_error), + commit=self.commit) + UnitTestRunResultsOrDeltaResults = Union[UnitTestRunResults, UnitTestRunDeltaResults] diff --git a/python/publish_unit_test_results.py b/python/publish_unit_test_results.py index b8d26ca2..ee5949dc 100644 --- a/python/publish_unit_test_results.py +++ b/python/publish_unit_test_results.py @@ -6,7 +6,7 @@ from collections import defaultdict from datetime import datetime from glob import glob -from typing import List, Optional, Union, Tuple, Any +from typing import List, Optional, Union, Mapping, Tuple, Any import github import humanize @@ -16,7 +16,7 @@ import publish.github_action from publish import hide_comments_modes, available_annotations, default_annotations, \ pull_request_build_modes, fail_on_modes, fail_on_mode_errors, fail_on_mode_failures, \ - comment_mode_off, comment_mode_update, comment_modes, punctuation_space + comment_mode_off, comment_mode_always, comment_modes, comment_modes_deprecated, punctuation_space from publish.github_action import GithubAction from publish.junit import parse_junit_xml_files from publish.progress import progress_logger @@ -228,17 +228,18 @@ def get_bool_var(name: str, options: dict, default: bool, gha: Optional[GithubAc def check_var(var: Union[Optional[str], List[str]], name: str, label: str, - allowed_values: Optional[List[str]] = None) -> None: + allowed_values: Optional[List[str]] = None, + deprecated_values: Optional[List[str]] = None) -> None: if var is None: raise RuntimeError(f'{label} must be provided via action input or environment variable {name}') if allowed_values: if isinstance(var, str): - if var not in allowed_values: + if var not in allowed_values + (deprecated_values or []): raise RuntimeError(f"Value '{var}' is not supported for variable {name}, " f"expected: {', '.join(allowed_values)}") if isinstance(var, list): - if any([v not in allowed_values for v in var]): + if any([v not in allowed_values + (deprecated_values or []) for v in var]): raise RuntimeError(f"Some values in '{', '.join(var)}' " f"are not supported for variable {name}, " f"allowed: {', '.join(allowed_values)}") @@ -259,6 +260,24 @@ def deprecate_var(val: Optional[str], deprecated_var: str, replacement_var: str, gha.warning(message) +def available_values(values: List[str]) -> str: + values = [f'"{val}"' for val in values] + return f"{', '.join(values[:-1])} or {values[-1]}" + + +def deprecate_val(val: Optional[str], var: str, replacement_vals: Mapping[str, str], gha: Optional[GithubAction]): + if val in replacement_vals: + message = f'Value "{val}" for option {var.lower()} is deprecated!' + replacement = replacement_vals[val] + if replacement: + message = f'{message} Instead, use value "{replacement}".' + + if gha is None: + logger.debug(message) + else: + gha.warning(message) + + def is_float(text: str) -> bool: return re.match('^[+-]?(([0-9]*\\.[0-9]+)|([0-9]+(\\.[0-9]?)?))$', text) is not None @@ -318,7 +337,7 @@ def get_settings(options: dict, gha: Optional[GithubAction] = None) -> Settings: time_factor=time_factor, check_name=check_name, comment_title=get_var('COMMENT_TITLE', options) or check_name, - comment_mode=get_var('COMMENT_MODE', options) or (comment_mode_update if comment_on_pr else comment_mode_off), + comment_mode=get_var('COMMENT_MODE', options) or (comment_mode_always if comment_on_pr else comment_mode_off), job_summary=get_bool_var('JOB_SUMMARY', options, default=True, gha=gha), compare_earlier=get_bool_var('COMPARE_TO_EARLIER_COMMIT', options, default=True, gha=gha), pull_request_build=get_var('PULL_REQUEST_BUILD', options) or 'merge', @@ -335,7 +354,7 @@ def get_settings(options: dict, gha: Optional[GithubAction] = None) -> Settings: check_var(settings.token, 'GITHUB_TOKEN', 'GitHub token') check_var(settings.repo, 'GITHUB_REPOSITORY', 'GitHub repository') check_var(settings.commit, 'COMMIT, GITHUB_SHA or event file', 'Commit SHA') - check_var(settings.comment_mode, 'COMMENT_MODE', 'Commit mode', comment_modes) + check_var(settings.comment_mode, 'COMMENT_MODE', 'Comment mode', comment_modes, list(comment_modes_deprecated.keys())) check_var(settings.pull_request_build, 'PULL_REQUEST_BUILD', 'Pull Request build', pull_request_build_modes) check_var(settings.hide_comment_mode, 'HIDE_COMMENTS', 'Hide comments mode', hide_comments_modes) check_var(settings.check_run_annotation, 'CHECK_RUN_ANNOTATIONS', 'Check run annotations', available_annotations) @@ -345,7 +364,9 @@ def get_settings(options: dict, gha: Optional[GithubAction] = None) -> Settings: check_var_condition(settings.seconds_between_github_reads > 0, f'SECONDS_BETWEEN_GITHUB_READS must be a positive number: {seconds_between_github_reads}') check_var_condition(settings.seconds_between_github_writes > 0, f'SECONDS_BETWEEN_GITHUB_WRITES must be a positive number: {seconds_between_github_writes}') - deprecate_var(get_var('COMMENT_ON_PR', options) or None, 'COMMENT_ON_PR', 'Instead, use option "comment_mode" with values "off", "create new", or "update last".', gha) + deprecate_var(get_var('COMMENT_ON_PR', options) or None, 'COMMENT_ON_PR', + f'Instead, use option "comment_mode" with values {available_values(comment_modes)}.', gha) + deprecate_val(settings.comment_mode, 'COMMENT_MODE', comment_modes_deprecated, gha) return settings diff --git a/python/test/test_action_script.py b/python/test/test_action_script.py index 23a3b661..538ef5e1 100644 --- a/python/test/test_action_script.py +++ b/python/test/test_action_script.py @@ -5,11 +5,10 @@ import tempfile import unittest from typing import Optional, Union, List - import mock from publish import pull_request_build_mode_merge, fail_on_mode_failures, fail_on_mode_errors, \ - fail_on_mode_nothing, comment_mode_off, comment_mode_create, comment_mode_update, \ + fail_on_mode_nothing, comment_modes, comment_modes_deprecated, comment_mode_off, comment_mode_always, \ hide_comments_modes, pull_request_build_modes, punctuation_space from publish.github_action import GithubAction from publish.unittestresults import ParsedUnitTestResults, ParseError @@ -145,7 +144,7 @@ def get_settings(token='token', time_factor=1.0, check_name='check name', comment_title='title', - comment_mode=comment_mode_create, + comment_mode=comment_mode_always, job_summary=True, compare_earlier=True, test_changes_limit=10, @@ -305,29 +304,39 @@ def test_get_settings_comment_title(self): self.do_test_get_settings(COMMENT_TITLE=None, CHECK_NAME='name', expected=self.get_settings(comment_title='name', check_name='name')) def test_get_settings_comment_on_pr(self): - default_comment_mode = comment_mode_update + default_comment_mode = comment_mode_always bool_warning = 'Option comment_on_pr has to be boolean, so either "true" or "false": foo' - depr_warning = 'Option comment_on_pr is deprecated! Instead, use option "comment_mode" with values "off", "create new", or "update last".' + depr_warning = 'Option comment_on_pr is deprecated! Instead, use option "comment_mode" with values "off", "always", "changes", "changes in failures", "changes in errors", "failures" or "errors".' self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='false', expected=self.get_settings(comment_mode=comment_mode_off), warning=depr_warning) self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='False', expected=self.get_settings(comment_mode=comment_mode_off), warning=depr_warning) self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='true', expected=self.get_settings(comment_mode=default_comment_mode), warning=depr_warning) self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='True', expected=self.get_settings(comment_mode=default_comment_mode), warning=depr_warning) - self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='foo', expected=self.get_settings(comment_mode=comment_mode_update), warning=[bool_warning, depr_warning]) - self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=comment_mode_update)) + self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR='foo', expected=self.get_settings(comment_mode=comment_mode_always), warning=[bool_warning, depr_warning]) + self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=comment_mode_always)) def test_get_settings_comment_mode(self): - warning = 'Option comment_on_pr is deprecated! Instead, use option "comment_mode" with values "off", "create new", or "update last".' - for mode in [comment_mode_off, comment_mode_create, comment_mode_update]: + comment_on_pr_warning = 'Option comment_on_pr is deprecated! Instead, use option "comment_mode" with values "off", "always", "changes", "changes in failures", "changes in errors", "failures" or "errors".' + for mode in comment_modes: with self.subTest(mode=mode): self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=mode)) - self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR='true' if mode == comment_mode_off else 'false', expected=self.get_settings(comment_mode=mode), warning=warning) + # comment_on_pr is ignored when comment_mode is given + self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR='true', expected=self.get_settings(comment_mode=mode), warning=comment_on_pr_warning) + self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR='false', expected=self.get_settings(comment_mode=mode), warning=comment_on_pr_warning) + + for mode in comment_modes_deprecated: + deprecated_warning = f'Value "{mode}" for option comment_mode is deprecated! Instead, use value "always".' + with self.subTest(mode=mode): + self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=mode), warning=deprecated_warning) + # comment_on_pr is ignored when comment_mode is given + self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR='true', expected=self.get_settings(comment_mode=mode), warning=[comment_on_pr_warning, deprecated_warning]) + self.do_test_get_settings(COMMENT_MODE=mode, COMMENT_ON_PR='false', expected=self.get_settings(comment_mode=mode), warning=[comment_on_pr_warning, deprecated_warning]) - self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=comment_mode_update)) + self.do_test_get_settings(COMMENT_MODE=None, COMMENT_ON_PR=None, expected=self.get_settings(comment_mode=comment_mode_always)) with self.assertRaises(RuntimeError) as re: self.do_test_get_settings(COMMENT_MODE='mode') - self.assertEqual("Value 'mode' is not supported for variable COMMENT_MODE, expected: off, create new, update last", str(re.exception)) + self.assertEqual("Value 'mode' is not supported for variable COMMENT_MODE, expected: off, always, changes, changes in failures, changes in errors, failures, errors", str(re.exception)) def test_get_settings_compare_to_earlier_commit(self): warning = 'Option compare_to_earlier_commit has to be boolean, so either "true" or "false": foo' @@ -462,7 +471,7 @@ def do_test_get_settings(self, COMMIT='commit', # defaults to get_commit_sha(event, event_name) FILES='files', COMMENT_TITLE='title', # defaults to check name - COMMENT_MODE='create new', # true unless 'false' + COMMENT_MODE='always', JOB_SUMMARY='true', HIDE_COMMENTS='off', # defaults to 'all but latest' REPORT_INDIVIDUAL_RUNS='true', # false unless 'true' diff --git a/python/test/test_publish.py b/python/test/test_publish.py index f9de1027..7526c875 100644 --- a/python/test/test_publish.py +++ b/python/test/test_publish.py @@ -2,10 +2,22 @@ import locale import pathlib import unittest +from collections import defaultdict +from typing import Any import mock -from publish import * +from publish import Annotation, UnitTestCaseResults, UnitTestRunResults, UnitTestRunDeltaResults, CaseMessages, \ + 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, \ + digest_string, ungest_string, get_details_line_md, get_commit_line_md, restrict_unicode, \ + get_short_summary, get_short_summary_md, get_long_summary_md, get_long_summary_with_runs_md, \ + get_long_summary_without_runs_md, get_long_summary_with_digest_md, \ + get_test_changes_md, get_test_changes_list_md, get_test_changes_summary_md, \ + get_case_annotations, get_case_annotation, get_all_tests_list_annotation, \ + get_skipped_tests_list_annotation, get_case_messages, chunk_test_list from publish.junit import parse_junit_xml_files from publish.unittestresults import get_stats, UnitTestCase, ParseError from publish.unittestresults import get_test_results @@ -106,6 +118,17 @@ def test_test_changes_has_no_tests(self): self.assertEqual(SomeTestChanges(default, [], default, ['two']).has_no_tests(), True) self.assertEqual(SomeTestChanges(default, ['one'], default, ['two']).has_no_tests(), False) + def test_test_changes_has_changes(self): + for changes, expected in [(SomeTestChanges(None, None, None, None), False), + (SomeTestChanges([], [], [], []), False), + (SomeTestChanges(['one'], ['one'], ['two'], ['two']), False), + (SomeTestChanges(['one'], ['three'], ['two'], ['two']), True), + (SomeTestChanges(['one'], ['one'], ['two'], ['three']), True), + (SomeTestChanges(['one'], ['two'], ['two'], ['three']), True), + (SomeTestChanges(['one'], None, ['two'], None), False), + (SomeTestChanges(None, ['one'], None, ['two']), False)]: + self.assertEqual(changes.has_changes, expected, str(changes)) + def test_restrict_unicode(self): self.assertEqual(None, restrict_unicode(None)) self.assertEqual('', restrict_unicode('')) @@ -991,7 +1014,7 @@ def test_get_long_summary_with_digest_md_with_single_run(self): 'H4sIAAAAAAAC/02MywqAIBQFfyVct+kd/UyEJVzKjKuuon/vZF' 'juzsyBOYWibbFiyIo8E9aTC1ACZs+TI7MDKyAO91x13KP1UkI0' 'v1jpgGg/oSbaILpPLMyGYXoY9nvsPTPNvfzXAiexwGlLGq3JAe' - 'K6buousrLZAAAA') + 'K6buousrLZAAAA\n') def test_get_long_summary_with_digest_md_with_multiple_runs(self): # makes gzipped digest deterministic @@ -1015,7 +1038,7 @@ def test_get_long_summary_with_digest_md_with_multiple_runs(self): 'H4sIAAAAAAAC/03MwQqDMBAE0F+RnD24aiv6M0VShaVqZJOciv' '/e0brR28wbmK8ZeRq86TLKM+Mjh6OUKO8ofWC3oFaoGMI+1Zpf' 'PloLeFzw4RXwTDD2PAGaBIOIE0gBkbjsf+0Z9Y6KBP87IoXzjk' - 'qF+51188wBRdP2A3NU1srcAAAA') + 'qF+51188wBRdP2A3NU1srcAAAA\n') def test_get_long_summary_with_digest_md_with_test_errors(self): # makes gzipped digest deterministic @@ -1039,7 +1062,7 @@ def test_get_long_summary_with_digest_md_with_test_errors(self): 'H4sIAAAAAAAC/0XOwQ6CMBAE0F8hPXtgEVT8GdMUSDYCJdv2ZP' 'x3psLW28zbZLIfM/E8BvOs6FKZkDj+SoMyJLGR/Yp6RcUh5lOr' '+RWSc4DuD2/eALcCk+UZcC8winiBPCCS1rzXn1HnqC5wzBEpnH' - 'PUKOgc5QedXxaOaJq+O+lMT3jdAAAA') + 'PUKOgc5QedXxaOaJq+O+lMT3jdAAAA\n') def test_get_long_summary_with_digest_md_with_parse_errors(self): # makes gzipped digest deterministic @@ -1063,7 +1086,7 @@ def test_get_long_summary_with_digest_md_with_parse_errors(self): 'H4sIAAAAAAAC/0XOwQ6CMBAE0F8hPXtgEVT8GdMUSDYCJdv2ZP' 'x3psLW28zbZLIfM/E8BvOs6FKZkDj+SoMyJLGR/Yp6RcUh5lOr' '+RWSc4DuD2/eALcCk+UZcC8winiBPCCS1rzXn1HnqC5wzBEpnH' - 'PUKOgc5QedXxaOaJq+O+lMT3jdAAAA') + 'PUKOgc5QedXxaOaJq+O+lMT3jdAAAA\n') def test_get_long_summary_with_digest_md_with_delta(self): # makes gzipped digest deterministic @@ -1092,7 +1115,7 @@ def test_get_long_summary_with_digest_md_with_delta(self): 'H4sIAAAAAAAC/02MywqAIBQFfyVct+kd/UyEJVzKjKuuon/vZF' 'juzsyBOYWibbFiyIo8E9aTC1ACZs+TI7MDKyAO91x13KP1UkI0' 'v1jpgGg/oSbaILpPLMyGYXoY9nvsPTPNvfzXAiexwGlLGq3JAe' - 'K6buousrLZAAAA') + 'K6buousrLZAAAA\n') def test_get_long_summary_with_digest_md_with_delta_and_parse_errors(self): # makes gzipped digest deterministic @@ -1121,7 +1144,7 @@ def test_get_long_summary_with_digest_md_with_delta_and_parse_errors(self): 'H4sIAAAAAAAC/02MywqAIBQFfyVct+kd/UyEJVzKjKuuon/vZF' 'juzsyBOYWibbFiyIo8E9aTC1ACZs+TI7MDKyAO91x13KP1UkI0' 'v1jpgGg/oSbaILpPLMyGYXoY9nvsPTPNvfzXAiexwGlLGq3JAe' - 'K6buousrLZAAAA') + 'K6buousrLZAAAA\n') def test_get_long_summary_with_digest_md_with_delta_results_only(self): with self.assertRaises(ValueError) as context: diff --git a/python/test/test_publisher.py b/python/test/test_publisher.py index 33294e5a..c956c9c8 100644 --- a/python/test/test_publisher.py +++ b/python/test/test_publisher.py @@ -1,21 +1,56 @@ +import dataclasses +import json import os +import pathlib +import sys import tempfile import unittest from collections.abc import Collection from datetime import datetime, timezone +from typing import Optional, List, Mapping, Union, Any, Callable import github.CheckRun import mock from github import Github, GithubException -from publish import * +from publish import comment_mode_create, comment_mode_update, comment_mode_off, comment_mode_always, \ + comment_mode_changes, comment_mode_changes_failures, comment_mode_changes_errors, \ + comment_mode_failures, comment_mode_errors, hide_comments_mode_off, \ + hide_comments_mode_orphaned, hide_comments_mode_all_but_latest, Annotation, default_annotations, \ + 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, punctuation_space, \ + get_long_summary_with_digest_md from publish.github_action import GithubAction from publish.publisher import Publisher, Settings, PublishData -from publish.unittestresults import UnitTestCase, ParseError +from publish.unittestresults import UnitTestCase, ParseError, UnitTestRunResults, UnitTestRunDeltaResults, \ + UnitTestCaseResults + +sys.path.append(str(pathlib.Path(__file__).resolve().parent)) + +from test_unittestresults import create_unit_test_run_results + errors = [ParseError('file', 'error', 1, 2)] +@dataclasses.dataclass(frozen=True) +class CommentConditionTest: + earlier_is_none: bool + earlier_is_different: bool + earlier_is_different_in_failures: bool + earlier_is_different_in_errors: bool + earlier_has_failures: bool + earlier_has_errors: bool + # current_has_changes being None indicates it is not a UnitTestRunDeltaResults but UnitTestRunResults + current_has_changes: Optional[bool] + current_has_failure_changes: bool + current_has_error_changes: bool + current_has_failures: bool + current_has_errors: bool + + class TestPublisher(unittest.TestCase): @staticmethod @@ -43,7 +78,7 @@ def create_github_pr(repo: str, return pr @staticmethod - def create_settings(comment_mode=comment_mode_create, + def create_settings(comment_mode=comment_mode_always, job_summary=True, compare_earlier=True, hide_comment_mode=hide_comments_mode_off, @@ -282,6 +317,109 @@ def test_get_test_list_annotations_chunked_and_restricted_unicode(self): Annotation(path='.github', start_line=0, end_line=0, start_column=None, end_column=None, annotation_level='notice', message='There are 3 tests, see "Raw output" for the list of tests 3 to 3.', title='3 tests found (test 3 to 3)', raw_details='class ‑ test \\U0001d484') ], annotations) + def do_test_require_comment(self, comment_mode, 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_mode=comment_mode) + + for test, expected in tests: + with self.subTest(test): + earlier = mock.MagicMock( + is_different=mock.Mock(return_value=test.earlier_is_different), + is_different_in_failures=mock.Mock(return_value=test.earlier_is_different_in_failures), + is_different_in_errors=mock.Mock(return_value=test.earlier_is_different_in_errors), + has_failures=test.earlier_has_failures, + has_errors=test.earlier_has_errors + ) if not test.earlier_is_none else None + current = mock.MagicMock( + is_delta=test.current_has_changes is not None, + has_changes=test.current_has_changes, + has_failure_changes=test.current_has_failure_changes, + has_error_changes=test.current_has_error_changes, + has_failures=test.current_has_failures, + has_errors=test.current_has_errors) + if current.is_delta: + current.without_delta = mock.Mock(return_value=current) + required = Publisher.require_comment(publisher, current, earlier) + self.assertEqual(required, expected) + + comment_condition_tests = [CommentConditionTest(earlier_is_none, + earlier_is_different, earlier_is_different_in_failures, earlier_is_different_in_errors, + earlier_has_failures, earlier_has_errors, + current_has_changes, current_has_failure_changes, current_has_error_changes, + current_has_failures, current_has_errors) + for earlier_is_none in [False, True] + for earlier_is_different in [False, True] + for earlier_is_different_in_failures in ([False, True] if not earlier_is_different else [True]) + for earlier_is_different_in_errors in ([False, True] if not earlier_is_different else [True]) + for earlier_has_failures in [False, True] + for earlier_has_errors in [False, True] + + for current_has_changes in [None, False, True] + for current_has_failure_changes in ([False, True] if not current_has_changes else [True]) + for current_has_error_changes in ([False, True] if not current_has_changes else [True]) + for current_has_failures in [False, True] + for current_has_errors in [False, True]] + + def test_require_comment_off(self): + self.do_test_require_comment( + comment_mode_off, + lambda _: False + ) + + def test_require_comment_create(self): + self.do_test_require_comment( + comment_mode_create, + lambda _: True + ) + + def test_require_comment_update(self): + self.do_test_require_comment( + comment_mode_update, + lambda _: True + ) + + def test_require_comment_always(self): + self.do_test_require_comment( + comment_mode_always, + lambda _: True + ) + + def test_require_comment_changes(self): + self.do_test_require_comment( + comment_mode_changes, + lambda test: not test.earlier_is_none and test.earlier_is_different or + test.current_has_changes is None or test.current_has_changes + ) + + def test_require_comment_changes_failures(self): + self.do_test_require_comment( + comment_mode_changes_failures, + lambda test: not test.earlier_is_none and (test.earlier_is_different_in_failures or test.earlier_is_different_in_errors) or + test.current_has_changes is None or test.current_has_failure_changes or test.current_has_error_changes + ) + + def test_require_comment_changes_errors(self): + self.do_test_require_comment( + comment_mode_changes_errors, + lambda test: not test.earlier_is_none and test.earlier_is_different_in_errors or + test.current_has_changes is None or test.current_has_error_changes + ) + + def test_require_comment_failures(self): + self.do_test_require_comment( + comment_mode_failures, + lambda test: not test.earlier_is_none and (test.earlier_has_failures or test.earlier_has_errors) or + (test.current_has_failures or test.current_has_errors) + ) + + def test_require_comment_errors(self): + self.do_test_require_comment( + comment_mode_errors, + lambda test: not test.earlier_is_none and test.earlier_has_errors or test.current_has_errors + ) + def test_publish_without_comment(self): settings = self.create_settings(comment_mode=comment_mode_off, hide_comment_mode=hide_comments_mode_off) mock_calls = self.call_mocked_publish(settings, prs=[object()]) @@ -323,7 +461,7 @@ def test_publish_without_comment_with_hiding(self): self.assertEqual({}, kwargs) def test_publish_with_comment_without_pr(self): - settings = self.create_settings(comment_mode=comment_mode_create, hide_comment_mode=hide_comments_mode_off) + settings = self.create_settings(hide_comment_mode=hide_comments_mode_off) mock_calls = self.call_mocked_publish(settings, prs=[]) self.assertEqual(3, len(mock_calls)) @@ -346,7 +484,7 @@ def test_publish_with_comment_without_pr(self): def test_publish_with_comment_without_hiding(self): pr = object() cr = object() - settings = self.create_settings(comment_mode=comment_mode_create, hide_comment_mode=hide_comments_mode_off) + settings = self.create_settings(hide_comment_mode=hide_comments_mode_off) mock_calls = self.call_mocked_publish(settings, prs=[pr], cr=cr) self.assertEqual(4, len(mock_calls)) @@ -374,7 +512,7 @@ def test_publish_with_comment_without_hiding(self): def do_test_publish_with_comment_with_hide(self, hide_mode: str, hide_method: str): pr = object() cr = object() - settings = self.create_settings(comment_mode=comment_mode_create, hide_comment_mode=hide_mode) + settings = self.create_settings(hide_comment_mode=hide_mode) mock_calls = self.call_mocked_publish(settings, prs=[pr], cr=cr) self.assertEqual(5, len(mock_calls)) @@ -419,7 +557,7 @@ def test_publish_with_comment_hide_orphaned(self): def test_publish_without_compare(self): pr = object() cr = object() - settings = self.create_settings(comment_mode=comment_mode_create, hide_comment_mode=hide_comments_mode_all_but_latest, compare_earlier=False) + settings = self.create_settings(hide_comment_mode=hide_comments_mode_all_but_latest, compare_earlier=False) mock_calls = self.call_mocked_publish(settings, prs=[pr], cr=cr) self.assertEqual(5, len(mock_calls)) @@ -450,13 +588,13 @@ def test_publish_without_compare(self): self.assertEqual({}, kwargs) def test_publish_comment_compare_earlier(self): - pr = mock.MagicMock() + pr = mock.MagicMock(number="1234", create_issue_comment=mock.Mock(return_value=mock.MagicMock())) cr = mock.MagicMock() bcr = mock.MagicMock() bs = UnitTestRunResults(1, [], 1, 1, 3, 1, 2, 0, 0, 3, 1, 2, 0, 0, 'commit') stats = self.stats cases = UnitTestCaseResults(self.cases) - settings = self.create_settings(comment_mode=comment_mode_create, compare_earlier=True) + settings = self.create_settings(compare_earlier=True) publisher = mock.MagicMock(Publisher) publisher._settings = settings publisher.get_check_run = mock.Mock(return_value=bcr) @@ -464,11 +602,13 @@ def test_publish_comment_compare_earlier(self): publisher.get_stats_delta = mock.Mock(return_value=bs) publisher.get_base_commit_sha = mock.Mock(return_value="base commit") publisher.get_test_lists_from_check_run = mock.Mock(return_value=(None, None)) + publisher.require_comment = mock.Mock(return_value=True) + publisher.get_latest_comment = mock.Mock(return_value=None) with mock.patch('publish.publisher.get_long_summary_with_digest_md', return_value='body'): Publisher.publish_comment(publisher, 'title', stats, pr, cr, cases) mock_calls = publisher.mock_calls - self.assertEqual(4, len(mock_calls)) + self.assertEqual(6, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('get_base_commit_sha', method) @@ -490,26 +630,22 @@ def test_publish_comment_compare_earlier(self): self.assertEqual((bcr, ), args) self.assertEqual({}, kwargs) + (method, args, kwargs) = mock_calls[4] + self.assertEqual('get_latest_comment', method) + + (method, args, kwargs) = mock_calls[5] + self.assertEqual('require_comment', method) + mock_calls = pr.mock_calls - self.assertEqual(3, len(mock_calls)) + self.assertEqual(1, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('create_issue_comment', method) self.assertEqual(('## title\nbody', ), args) self.assertEqual({}, kwargs) - (method, args, kwargs) = mock_calls[1] - self.assertEqual('number.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[2] - self.assertEqual('create_issue_comment().html_url.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - def test_publish_comment_compare_earlier_with_restricted_unicode(self): - pr = mock.MagicMock() + pr = mock.MagicMock(number="1234", create_issue_comment=mock.Mock(return_value=mock.MagicMock())) cr = mock.MagicMock(html_url='html://url') bcr = mock.MagicMock() bs = UnitTestRunResults(1, [], 1, 1, 3, 1, 2, 0, 0, 3, 1, 2, 0, 0, 'commit') @@ -524,13 +660,15 @@ def test_publish_comment_compare_earlier_with_restricted_unicode(self): ((None, 'class', 'test 𝒇'), {'success': [None]}), # added test 𝒇 ]) - settings = self.create_settings(comment_mode=comment_mode_create, compare_earlier=True) + settings = self.create_settings(compare_earlier=True) publisher = mock.MagicMock(Publisher) publisher._settings = settings publisher.get_check_run = mock.Mock(return_value=bcr) publisher.get_stats_from_check_run = mock.Mock(return_value=bs) publisher.get_stats_delta = mock.Mock(return_value=bs) publisher.get_base_commit_sha = mock.Mock(return_value="base commit") + publisher.get_latest_comment = mock.Mock(return_value=None) + publisher.require_comment = mock.Mock(return_value=True) # the earlier test cases with restricted unicode as they come from the check runs API publisher.get_test_lists_from_check_run = mock.Mock(return_value=( # before, these existed: test 𝒂, test 𝒃, skipped 𝒄, skipped 𝒅 @@ -545,7 +683,7 @@ def test_publish_comment_compare_earlier_with_restricted_unicode(self): mock_calls = publisher.mock_calls - self.assertEqual(4, len(mock_calls)) + self.assertEqual(6, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('get_base_commit_sha', method) @@ -567,8 +705,14 @@ def test_publish_comment_compare_earlier_with_restricted_unicode(self): self.assertEqual((bcr, ), args) self.assertEqual({}, kwargs) + (method, args, kwargs) = mock_calls[4] + self.assertEqual('get_latest_comment', method) + + (method, args, kwargs) = mock_calls[5] + self.assertEqual('require_comment', method) + mock_calls = pr.mock_calls - self.assertEqual(3, len(mock_calls)) + self.assertEqual(1, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('create_issue_comment', method) @@ -616,17 +760,7 @@ def test_publish_comment_compare_earlier_with_restricted_unicode(self): '```\n' '
\n' '\n' - f'{expected_digest}', ), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[1] - self.assertEqual('number.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[2] - self.assertEqual('create_issue_comment().html_url.__str__', method) - self.assertEqual((), args) + f'{expected_digest}\n', ), args) self.assertEqual({}, kwargs) def test_publish_comment_compare_with_itself(self): @@ -634,12 +768,13 @@ def test_publish_comment_compare_with_itself(self): cr = mock.MagicMock() stats = self.stats cases = UnitTestCaseResults(self.cases) - settings = self.create_settings(comment_mode=comment_mode_create, compare_earlier=True) + settings = self.create_settings(compare_earlier=True) publisher = mock.MagicMock(Publisher) publisher._settings = settings publisher.get_check_run = mock.Mock(return_value=None) publisher.get_base_commit_sha = mock.Mock(return_value=stats.commit) publisher.get_test_lists_from_check_run = mock.Mock(return_value=(None, None)) + publisher.get_latest_comment = mock.Mock(return_value=None) with mock.patch('publish.publisher.get_long_summary_md', return_value='body'): Publisher.publish_comment(publisher, 'title', stats, pr, cr, cases) mock_calls = publisher.mock_calls @@ -655,21 +790,23 @@ def test_publish_comment_compare_with_itself(self): self.assertEqual(0, len(mock_calls)) def test_publish_comment_compare_with_None(self): - pr = mock.MagicMock() + pr = mock.MagicMock(number="1234", create_issue_comment=mock.Mock(return_value=mock.MagicMock())) cr = mock.MagicMock() stats = self.stats cases = UnitTestCaseResults(self.cases) - settings = self.create_settings(comment_mode=comment_mode_create, compare_earlier=True) + settings = self.create_settings(compare_earlier=True) publisher = mock.MagicMock(Publisher) publisher._settings = settings publisher.get_check_run = mock.Mock(return_value=None) publisher.get_base_commit_sha = mock.Mock(return_value=None) publisher.get_test_lists_from_check_run = mock.Mock(return_value=(None, None)) + publisher.get_latest_comment = mock.Mock(return_value=None) + publisher.require_comment = mock.Mock(return_value=True) with mock.patch('publish.publisher.get_long_summary_with_digest_md', return_value='body'): Publisher.publish_comment(publisher, 'title', stats, pr, cr, cases) mock_calls = publisher.mock_calls - self.assertEqual(3, len(mock_calls)) + self.assertEqual(5, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('get_base_commit_sha', method) @@ -686,68 +823,76 @@ def test_publish_comment_compare_with_None(self): self.assertEqual((None, ), args) self.assertEqual({}, kwargs) + (method, args, kwargs) = mock_calls[3] + self.assertEqual('get_latest_comment', method) + + (method, args, kwargs) = mock_calls[4] + self.assertEqual('require_comment', method) + mock_calls = pr.mock_calls - self.assertEqual(3, len(mock_calls)) + self.assertEqual(1, len(mock_calls)) (method, args, kwargs) = mock_calls[0] self.assertEqual('create_issue_comment', method) self.assertEqual(('## title\nbody', ), args) self.assertEqual({}, kwargs) - (method, args, kwargs) = mock_calls[1] - self.assertEqual('number.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[2] - self.assertEqual('create_issue_comment().html_url.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - def do_test_publish_comment_with_reuse_comment(self, one_exists: bool): - pr = mock.MagicMock() - cr = mock.MagicMock() - stats = self.stats - cases = UnitTestCaseResults(self.cases) - settings = self.create_settings(comment_mode=comment_mode_update, compare_earlier=False) - publisher = mock.MagicMock(Publisher) - publisher._settings = settings - publisher.get_test_lists_from_check_run = mock.Mock(return_value=(None, None)) - publisher.reuse_comment = mock.Mock(return_value=one_exists) - with mock.patch('publish.publisher.get_long_summary_with_digest_md', return_value='body'): - Publisher.publish_comment(publisher, 'title', stats, pr, cr, cases) - mock_calls = publisher.mock_calls - - self.assertEqual(2, len(mock_calls)) - - (method, args, kwargs) = mock_calls[0] - self.assertEqual('get_test_lists_from_check_run', method) - self.assertEqual((None, ), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[1] - self.assertEqual('reuse_comment', method) - self.assertEqual((pr, '## title\nbody'), args) - self.assertEqual({}, kwargs) - - mock_calls = pr.mock_calls - self.assertEqual(0 if one_exists else 3, len(mock_calls)) - - if not one_exists: - (method, args, kwargs) = mock_calls[0] - self.assertEqual('create_issue_comment', method) - self.assertEqual(('## title\nbody', ), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[1] - self.assertEqual('number.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) - - (method, args, kwargs) = mock_calls[2] - self.assertEqual('create_issue_comment().html_url.__str__', method) - self.assertEqual((), args) - self.assertEqual({}, kwargs) + for mode in [comment_mode_update, comment_mode_always]: + with self.subTest(mode=mode): + pr = mock.MagicMock(number="1234", create_issue_comment=mock.Mock(return_value=mock.MagicMock())) + cr = mock.MagicMock() + lc = mock.MagicMock(body='latest comment') if one_exists else None + stats = self.stats + cases = UnitTestCaseResults(self.cases) + settings = self.create_settings(comment_mode=mode, compare_earlier=False) + publisher = mock.MagicMock(Publisher) + publisher._settings = settings + publisher.get_test_lists_from_check_run = mock.Mock(return_value=(None, None)) + publisher.get_latest_comment = mock.Mock(return_value=lc) + publisher.reuse_comment = mock.Mock(return_value=one_exists) + publisher.require_comment = mock.Mock(return_value=True) + with mock.patch('publish.publisher.get_long_summary_with_digest_md', return_value='body'): + Publisher.publish_comment(publisher, 'title', stats, pr, cr, cases) + mock_calls = publisher.mock_calls + + self.assertEqual(5 if one_exists else 3, len(mock_calls)) + + (method, args, kwargs) = mock_calls[0] + self.assertEqual('get_test_lists_from_check_run', method) + self.assertEqual((None, ), args) + self.assertEqual({}, kwargs) + + (method, args, kwargs) = mock_calls[1] + self.assertEqual('get_latest_comment', method) + self.assertEqual((pr, ), args) + self.assertEqual({}, kwargs) + + if one_exists: + (method, args, kwargs) = mock_calls[2] + self.assertEqual('get_stats_from_summary_md', method) + self.assertEqual(('latest comment', ), args) + self.assertEqual({}, kwargs) + + (method, args, kwargs) = mock_calls[3] + self.assertEqual('require_comment', method) + + (method, args, kwargs) = mock_calls[4] + self.assertEqual('reuse_comment', method) + self.assertEqual((lc, '## title\nbody'), args) + self.assertEqual({}, kwargs) + else: + (method, args, kwargs) = mock_calls[2] + self.assertEqual('require_comment', method) + + mock_calls = pr.mock_calls + self.assertEqual(0 if one_exists else 1, len(mock_calls)) + + if not one_exists: + (method, args, kwargs) = mock_calls[0] + self.assertEqual('create_issue_comment', method) + self.assertEqual(('## title\nbody', ), args) + self.assertEqual({}, kwargs) def test_publish_comment_with_reuse_comment_none_existing(self): self.do_test_publish_comment_with_reuse_comment(one_exists=False) @@ -755,57 +900,22 @@ def test_publish_comment_with_reuse_comment_none_existing(self): def test_publish_comment_with_reuse_comment_one_existing(self): self.do_test_publish_comment_with_reuse_comment(one_exists=True) - def do_test_reuse_comment(self, - pull_request_comments: List[Any], - action_comments: List[Mapping[str, int]], - body='body', - expected_body='body\n:recycle: This comment has been updated with latest results.'): - pr = mock.MagicMock() + def do_test_reuse_comment(self, earlier_body: str, expected_body: str): comment = mock.MagicMock() - pr.get_issue_comment = mock.Mock(return_value=comment) - settings = self.create_settings(comment_mode=comment_mode_update, compare_earlier=False) publisher = mock.MagicMock(Publisher) - publisher._settings = settings - publisher.get_pull_request_comments = mock.Mock(return_value=pull_request_comments) - publisher.get_action_comments = mock.Mock(return_value=action_comments) - Publisher.reuse_comment(publisher, pr, body) - - mock_calls = publisher.mock_calls - self.assertEqual(2, len(mock_calls)) - - (method, args, kwargs) = mock_calls[0] - self.assertEqual('get_pull_request_comments', method) - self.assertEqual((pr, ), args) - self.assertEqual({'order_by_updated': True}, kwargs) - - (method, args, kwargs) = mock_calls[1] - self.assertEqual('get_action_comments', method) - self.assertEqual((pull_request_comments, ), args) - self.assertEqual({}, kwargs) - - if action_comments: - pr.get_issue_comment.assert_called_once_with(action_comments[-1].get('databaseId')) - comment.edit.assert_called_once_with(expected_body) - - def test_reuse_comment_non_existing(self): - self.do_test_reuse_comment(pull_request_comments=[1, 2, 3], action_comments=[]) - - def test_reuse_comment_one_existing(self): - self.do_test_reuse_comment(pull_request_comments=[1, 2, 3], action_comments=[{'databaseId': 1}]) + Publisher.reuse_comment(publisher, comment, earlier_body) - def test_reuse_comment_multiple_existing(self): - self.do_test_reuse_comment(pull_request_comments=[1, 2, 3], action_comments=[{'databaseId': 1}, {'databaseId': 2}, {'databaseId': 3}]) + comment.edit.assert_called_once_with(expected_body) + self.assertEqual(0, len(publisher.mock_calls)) def test_reuse_comment_existing_not_updated(self): # we do not expect the body to be extended by the recycle message - self.do_test_reuse_comment(pull_request_comments=[1, 2, 3], action_comments=[{'databaseId': 1}], - body='a new comment', + self.do_test_reuse_comment(earlier_body='a new comment', expected_body='a new comment\n:recycle: This comment has been updated with latest results.') def test_reuse_comment_existing_updated(self): # we do not expect the body to be extended by the recycle message - self.do_test_reuse_comment(pull_request_comments=[1, 2, 3], action_comments=[{'databaseId': 1}], - body='comment already updated\n:recycle: Has been updated', + self.do_test_reuse_comment(earlier_body='comment already updated\n:recycle: Has been updated', expected_body='comment already updated\n:recycle: Has been updated') def do_test_get_pulls(self, @@ -953,6 +1063,18 @@ def test_get_check_run_from_list_many(self): name = runs[0].name self.do_test_get_check_run_from_list(runs, expected) + def test_get_stats_from_summary_md(self): + results = create_unit_test_run_results() + summary = get_long_summary_with_digest_md(results, results, 'http://url') + actual = Publisher.get_stats_from_summary_md(summary) + self.assertEqual(results, actual) + + def test_get_stats_from_summary_md_recycled(self): + summary = f'body\n\n{digest_header}H4sIAGpapmIC/1WMyw7CIBQFf6Vh7QK4FMGfMeQWEmJbDI9V479LI6DuzsxJ5iDOrzaR2wSXiaTi84ClRJN92CvSivXI5yX7vqeCWIX4iod/VsGGcMavf8LGGGILxrKfPaba7j3Ghvj0ROeWg86/NQzb5nMFIhCBgnbUzQAIVik+c6W1YU5KVPoqNF04teT1BvQuAoL9AAAA\n:recycle: This comment has been updated with latest results.' + actual = Publisher.get_stats_from_summary_md(summary) + self.assertIsNotNone(actual) + self.assertEqual(6, actual.tests) + @staticmethod def mock_check_run(name: str, status: str, started_at: datetime, summary: str) -> mock.Mock: run = mock.MagicMock(status=status, started_at=started_at, output=mock.MagicMock(summary=summary)) @@ -1192,7 +1314,7 @@ def do_test_publish_check_without_base_stats(self, errors: List[ParseError], ann 'H4sIAAAAAAAC/0WOSQqEMBBFryJZu+g4tK2XkRAVCoc0lWQl3t' '3vULqr9z48alUDTb1XTaLTRPlI4YQM0EU2gdwCzIEYwjllAq2P' '1sIUrxjpD1E+YjA0QXwf0TM7hqlgOC5HMP/dt/RevnK18F3THx' - 'FS08fz1s0zBZBc2w5zHdX73QAAAA=='.format(errors='{} errors\u2004\u2003'.format(len(errors)) if len(errors) > 0 else ''), + 'FS08fz1s0zBZBc2w5zHdX73QAAAA==\n'.format(errors='{} errors\u2004\u2003'.format(len(errors)) if len(errors) > 0 else ''), 'annotations': annotations } ) @@ -1258,7 +1380,7 @@ def do_test_publish_check_with_base_stats(self, errors: List[ParseError]): 'H4sIAAAAAAAC/0WOSQqEMBBFryJZu+g4tK2XkRAVCoc0lWQl3t' '3vULqr9z48alUDTb1XTaLTRPlI4YQM0EU2gdwCzIEYwjllAq2P' '1sIUrxjpD1E+YjA0QXwf0TM7hqlgOC5HMP/dt/RevnK18F3THx' - 'FS08fz1s0zBZBc2w5zHdX73QAAAA=='.format(errors='{} errors\u2004\u2003'.format(len(errors)) if len(errors) > 0 else ''), + 'FS08fz1s0zBZBc2w5zHdX73QAAAA==\n'.format(errors='{} errors\u2004\u2003'.format(len(errors)) if len(errors) > 0 else ''), 'annotations': error_annotations + [ {'path': 'test file', 'start_line': 0, 'end_line': 0, 'annotation_level': 'warning', 'message': 'result file', 'title': '1 out of 2 runs failed: test (class)', 'raw_details': 'content'}, {'path': 'test file', 'start_line': 0, 'end_line': 0, 'annotation_level': 'failure', 'message': 'result file', 'title': '1 out of 2 runs with error: test2 (class)', 'raw_details': 'error content'}, @@ -1322,7 +1444,7 @@ def test_publish_check_without_compare(self): '[test-results]:data:application/gzip;base64,H4sIAAAAAAAC/0WOSQqEMBBFryJ' 'Zu+g4tK2XkRAVCoc0lWQl3t3vULqr9z48alUDTb1XTaLTRPlI4YQM0EU2gdwCzIEYwjllAq' '2P1sIUrxjpD1E+YjA0QXwf0TM7hqlgOC5HMP/dt/RevnK18F3THxFS08fz1s0zBZBc2w5zH' - 'dX73QAAAA==', + 'dX73QAAAA==\n', 'annotations': [ {'path': 'test file', 'start_line': 0, 'end_line': 0, 'annotation_level': 'warning', 'message': 'result file', 'title': '1 out of 2 runs failed: test (class)', 'raw_details': 'content'}, {'path': 'test file', 'start_line': 0, 'end_line': 0, 'annotation_level': 'failure', 'message': 'result file', 'title': '1 out of 2 runs with error: test2 (class)', 'raw_details': 'error content'}, @@ -1384,7 +1506,7 @@ def test_publish_check_with_multiple_annotation_pages(self): 'H4sIAAAAAAAC/0WOSQqEMBBFryJZu+g4tK2XkRAVCoc0lWQl3t' '3vULqr9z48alUDTb1XTaLTRPlI4YQM0EU2gdwCzIEYwjllAq2P' '1sIUrxjpD1E+YjA0QXwf0TM7hqlgOC5HMP/dt/RevnK18F3THx' - 'FS08fz1s0zBZBc2w5zHdX73QAAAA==', + 'FS08fz1s0zBZBc2w5zHdX73QAAAA==\n', 'annotations': ([ {'path': 'test file', 'start_line': i, 'end_line': i, 'annotation_level': 'warning', 'message': 'result file', 'title': f'test{i} (class) failed', 'raw_details': f'content{i}'} # we expect the first 50 annotations in the create call @@ -1417,7 +1539,7 @@ def test_publish_check_with_multiple_annotation_pages(self): 'H4sIAAAAAAAC/0WOSQqEMBBFryJZu+g4tK2XkRAVCoc0lWQl3t' '3vULqr9z48alUDTb1XTaLTRPlI4YQM0EU2gdwCzIEYwjllAq2P' '1sIUrxjpD1E+YjA0QXwf0TM7hqlgOC5HMP/dt/RevnK18F3THx' - 'FS08fz1s0zBZBc2w5zHdX73QAAAA==', + 'FS08fz1s0zBZBc2w5zHdX73QAAAA==\n', 'annotations': ([ {'path': 'test file', 'start_line': i, 'end_line': i, 'annotation_level': 'warning', 'message': 'result file', 'title': f'test{i} (class) failed', 'raw_details': f'content{i}'} # for each edit we expect a batch of 50 annotations starting at start @@ -1776,6 +1898,7 @@ def test_publish_comment(self): gh, gha, req, repo, commit = self.create_mocks(digest=self.base_digest, check_names=[settings.check_name]) pr = self.create_github_pr(settings.repo, base_commit_sha=base_commit) publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) # makes gzipped digest deterministic with mock.patch('gzip.time.time', return_value=0): @@ -1790,15 +1913,31 @@ def test_publish_comment(self): '\n' 'Results for commit commit.\u2003± Comparison against base commit base.\n' '\n' - f'{expected_digest}' + f'{expected_digest}\n' ) + def test_publish_comment_not_required(self): + # same as test_publish_comment but require_comment returns False + with mock.patch('publish.publisher.Publisher.require_comment', return_value=False): + settings = self.create_settings(event={'pull_request': {'base': {'sha': 'commit base'}}}, event_name='pull_request') + base_commit = 'base-commit' + + gh, gha, req, repo, commit = self.create_mocks(digest=self.base_digest, check_names=[settings.check_name]) + pr = self.create_github_pr(settings.repo, base_commit_sha=base_commit) + publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) + + publisher.publish_comment(settings.comment_title, self.stats, pr) + + pr.create_issue_comment.assert_not_called() + def test_publish_comment_without_base(self): settings = self.create_settings() gh, gha, req, repo, commit = self.create_mocks(digest=self.base_digest, check_names=[settings.check_name]) pr = self.create_github_pr(settings.repo) publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) compare = mock.MagicMock() compare.merge_base_commit.sha = None @@ -1817,7 +1956,7 @@ def test_publish_comment_without_base(self): '\n' 'Results for commit commit.\n' '\n' - f'{expected_digest}' + f'{expected_digest}\n' ) def test_publish_comment_without_compare(self): @@ -1827,6 +1966,7 @@ def test_publish_comment_without_compare(self): gh, gha, req, repo, commit = self.create_mocks(digest=self.base_digest, check_names=[settings.check_name]) pr = self.create_github_pr(settings.repo, base_commit_sha=base_commit) publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) # makes gzipped digest deterministic with mock.patch('gzip.time.time', return_value=0): @@ -1841,7 +1981,7 @@ def test_publish_comment_without_compare(self): '\n' 'Results for commit commit.\n' '\n' - f'{expected_digest}' + f'{expected_digest}\n' ) def test_publish_comment_with_check_run_with_annotations(self): @@ -1852,6 +1992,7 @@ def test_publish_comment_with_check_run_with_annotations(self): pr = self.create_github_pr(settings.repo, base_commit_sha=base_commit) cr = mock.MagicMock(html_url='http://check-run.url') publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) # makes gzipped digest deterministic with mock.patch('gzip.time.time', return_value=0): @@ -1868,7 +2009,7 @@ def test_publish_comment_with_check_run_with_annotations(self): '\n' 'Results for commit commit.\u2003± Comparison against base commit base.\n' '\n' - f'{expected_digest}' + f'{expected_digest}\n' ) def test_publish_comment_with_check_run_without_annotations(self): @@ -1879,6 +2020,7 @@ def test_publish_comment_with_check_run_without_annotations(self): pr = self.create_github_pr(settings.repo, base_commit_sha=base_commit) cr = mock.MagicMock(html_url='http://check-run.url') publisher = Publisher(settings, gh, gha) + publisher.get_latest_comment = mock.Mock(return_value=None) stats = dict(self.stats.to_dict()) stats.update(tests_fail=0, tests_error=0, runs_fail=0, runs_error=0) @@ -1897,7 +2039,7 @@ def test_publish_comment_with_check_run_without_annotations(self): '\n' 'Results for commit commit.\u2003± Comparison against base commit base.\n' '\n' - f'{expected_digest}' + f'{expected_digest}\n' ) def test_get_base_commit_sha_none_event(self): diff --git a/python/test/test_unittestresults.py b/python/test/test_unittestresults.py index 99dfabe2..8bb58653 100644 --- a/python/test/test_unittestresults.py +++ b/python/test/test_unittestresults.py @@ -1,5 +1,6 @@ -import dataclasses import unittest +import dataclasses +from typing import List from xml.etree.ElementTree import ParseError as XmlParseError from publish.unittestresults import get_test_results, get_stats, get_stats_delta, \ @@ -12,6 +13,50 @@ errors_dict = [dataclasses.asdict(e) for e in errors] +def create_unit_test_run_results(files=1, + errors: List[ParseError] = [], + suites=2, + duration=3, + tests=22, tests_succ=4, tests_skip=5, tests_fail=6, tests_error=7, + runs=38, runs_succ=8, runs_skip=9, runs_fail=10, runs_error=11, + commit='commit') -> UnitTestRunResults: + return UnitTestRunResults( + files=files, + errors=list(errors), + suites=suites, + duration=duration, + tests=tests, tests_succ=tests_succ, tests_skip=tests_skip, tests_fail=tests_fail, tests_error=tests_error, + runs=runs, runs_succ=runs_succ, runs_skip=runs_skip, runs_fail=runs_fail, runs_error=runs_error, + commit=commit + ) + + +def create_unit_test_run_delta_results(files=1, files_delta=-1, + errors=[], + suites=2, suites_delta=-2, + duration=3, duration_delta=-3, + tests=4, tests_delta=-4, + tests_succ=5, tests_succ_delta=-5, + tests_skip=6, tests_skip_delta=-6, + tests_fail=7, tests_fail_delta=-7, + tests_error=8, tests_error_delta=-8, + runs=9, runs_delta=-9, + runs_succ=10, runs_succ_delta=-10, + runs_skip=11, runs_skip_delta=-11, + runs_fail=12, runs_fail_delta=-12, + runs_error=13, runs_error_delta=-13) -> UnitTestRunDeltaResults: + return UnitTestRunDeltaResults( + files={'number': files, 'delta': files_delta}, + errors=errors, + suites={'number': suites, 'delta': suites_delta}, + duration={'duration': duration, 'delta': duration_delta}, + tests={'number': tests, 'delta': tests_delta}, tests_succ={'number': tests_succ, 'delta': tests_succ_delta}, tests_skip={'number': tests_skip, 'delta': tests_skip_delta}, tests_fail={'number': tests_fail, 'delta': tests_fail_delta}, tests_error={'number': tests_error, 'delta': tests_error_delta}, + runs={'number': runs, 'delta': runs_delta}, runs_succ={'number': runs_succ, 'delta': runs_succ_delta}, runs_skip={'number': runs_skip, 'delta': runs_skip_delta}, runs_fail={'number': runs_fail, 'delta': runs_fail_delta}, runs_error={'number': runs_error, 'delta': runs_error_delta}, + commit='commit', + reference_type='type', reference_commit='ref' + ) + + class TestUnitTestResults(unittest.TestCase): def test_parse_error_from_xml_parse_error(self): @@ -82,6 +127,7 @@ def test_unit_test_run_results_to_dict(self): ) self.assertEqual(expected, actual) + # results from dicts usually do not contain errors def test_unit_test_run_results_from_dict(self): actual = UnitTestRunResults.from_dict(dict( files=1, errors=errors_dict, suites=2, duration=3, @@ -429,3 +475,166 @@ def test_get_stats_delta(self): reference_commit='ref', reference_type='type' )) + + def test_unit_test_run_results_is_different(self): + stats = create_unit_test_run_results() + create_other = create_unit_test_run_results + for diff, other, expected in [('nothing', create_other(), False), + ('files', create_other(files=stats.files+1), True), + ('errors', create_other(errors=errors), False), + ('suites', create_other(suites=stats.suites+1), True), + ('duration', create_other(duration=stats.duration+1), False), + ('tests', create_other(tests=stats.tests+1), True), + ('test success', create_other(tests_succ=stats.tests_succ+1), True), + ('test skips', create_other(tests_skip=stats.tests_skip+1), True), + ('test failures', create_other(tests_fail=stats.tests_fail+1), True), + ('test errors', create_other(tests_error=stats.tests_error+1), True), + ('runs', create_other(runs=stats.runs+1), True), + ('runs success', create_other(runs_succ=stats.runs_succ+1), True), + ('runs skips', create_other(runs_skip=stats.runs_skip+1), True), + ('runs failures', create_other(runs_fail=stats.runs_fail+1), True), + ('runs errors', create_other(runs_error=stats.runs_error+1), True), + ('commit', create_other(commit='other'), False)]: + with self.subTest(different_in=diff): + self.assertEqual(expected, stats.is_different(other), msg=diff) + + def test_unit_test_run_results_is_different_in_failures(self): + stats = create_unit_test_run_results() + create_other = create_unit_test_run_results + for diff, other, expected in [('nothing', create_other(), False), + ('files', create_other(files=stats.files+1), False), + ('errors', create_other(errors=errors), False), + ('suites', create_other(suites=stats.suites+1), False), + ('duration', create_other(duration=stats.duration+1), False), + ('tests', create_other(tests=stats.tests+1), False), + ('test success', create_other(tests_succ=stats.tests_succ+1), False), + ('test skips', create_other(tests_skip=stats.tests_skip+1), False), + ('test failures', create_other(tests_fail=stats.tests_fail+1), True), + ('test errors', create_other(tests_error=stats.tests_error+1), False), + ('runs', create_other(runs=stats.runs+1), False), + ('runs success', create_other(runs_succ=stats.runs_succ+1), False), + ('runs skips', create_other(runs_skip=stats.runs_skip+1), False), + ('runs failures', create_other(runs_fail=stats.runs_fail+1), True), + ('runs errors', create_other(runs_error=stats.runs_error+1), False), + ('commit', create_other(commit='other'), False)]: + with self.subTest(different_in=diff): + self.assertEqual(expected, stats.is_different_in_failures(other), msg=diff) + + def test_unit_test_run_results_is_different_in_errors(self): + stats = create_unit_test_run_results() + create_other = create_unit_test_run_results + for diff, other, expected in [('nothing', create_other(), False), + ('files', create_other(files=stats.files+1), False), + ('errors', create_other(errors=errors), False), + ('suites', create_other(suites=stats.suites+1), False), + ('duration', create_other(duration=stats.duration+1), False), + ('tests', create_other(tests=stats.tests+1), False), + ('test success', create_other(tests_succ=stats.tests_succ+1), False), + ('test skips', create_other(tests_skip=stats.tests_skip+1), False), + ('test failures', create_other(tests_fail=stats.tests_fail+1), False), + ('test errors', create_other(tests_error=stats.tests_error+1), True), + ('runs', create_other(runs=stats.runs+1), False), + ('runs success', create_other(runs_succ=stats.runs_succ+1), False), + ('runs skips', create_other(runs_skip=stats.runs_skip+1), False), + ('runs failures', create_other(runs_fail=stats.runs_fail+1), False), + ('runs errors', create_other(runs_error=stats.runs_error+1), True), + ('commit', create_other(commit='other'), False)]: + with self.subTest(different_in=diff): + self.assertEqual(expected, stats.is_different_in_errors(other), msg=diff) + + def test_unit_test_run_results_has_failures(self): + def create_stats(errors=[], tests_fail=0, tests_error=0, runs_fail=0, runs_error=0) -> UnitTestRunResults: + return create_unit_test_run_results(errors=errors, tests_fail=tests_fail, tests_error=tests_error, runs_fail=runs_fail, runs_error=runs_error) + + for label, stats, expected in [('no failures', create_stats(), False), + ('errors', create_stats(errors=errors), False), + ('test failures', create_stats(tests_fail=1), True), + ('test errors', create_stats(tests_error=1), False), + ('runs failures', create_stats(runs_fail=1), True), + ('runs errors', create_stats(runs_error=1), False)]: + with self.subTest(msg=label): + self.assertEqual(stats.has_failures, expected, msg=label) + + def test_unit_test_run_results_has_errors(self): + def create_stats(errors=[], tests_fail=0, tests_error=0, runs_fail=0, runs_error=0) -> UnitTestRunResults: + return create_unit_test_run_results(errors=errors, tests_fail=tests_fail, tests_error=tests_error, runs_fail=runs_fail, runs_error=runs_error) + + for label, stats, expected in [('no errors', create_stats(), False), + ('errors', create_stats(errors=errors), True), + ('test failures', create_stats(tests_fail=1), False), + ('test errors', create_stats(tests_error=1), True), + ('runs failures', create_stats(runs_fail=1), False), + ('runs errors', create_stats(runs_error=1), True)]: + with self.subTest(msg=label): + self.assertEqual(stats.has_errors, expected, msg=label) + + def test_unit_test_run_delta_results_has_changes(self): + def create_stats_with_delta(files_delta=0, + suites_delta=0, + duration_delta=0, + tests_delta=0, + tests_succ_delta=0, + tests_skip_delta=0, + tests_fail_delta=0, + tests_error_delta=0, + runs_delta=0, + runs_succ_delta=0, + runs_skip_delta=0, + runs_fail_delta=0, + runs_error_delta=0) -> UnitTestRunDeltaResults: + return create_unit_test_run_delta_results(files_delta=files_delta, suites_delta=suites_delta, duration_delta=duration_delta, + tests_delta=tests_delta, tests_succ_delta=tests_succ_delta, tests_skip_delta=tests_skip_delta, tests_fail_delta=tests_fail_delta, tests_error_delta=tests_error_delta, + runs_delta=runs_delta, runs_succ_delta=runs_succ_delta, runs_skip_delta=runs_skip_delta, runs_fail_delta=runs_fail_delta, runs_error_delta=runs_error_delta) + + for label, stats, expected in [('no deltas', create_stats_with_delta(), False), + ('files', create_stats_with_delta(files_delta=1), True), + ('suites', create_stats_with_delta(suites_delta=1), True), + ('duration', create_stats_with_delta(duration_delta=1), False), + ('tests', create_stats_with_delta(tests_delta=1), True), + ('tests succ', create_stats_with_delta(tests_succ_delta=1), True), + ('tests skip', create_stats_with_delta(tests_skip_delta=1), True), + ('tests fail', create_stats_with_delta(tests_fail_delta=1), True), + ('tests error', create_stats_with_delta(tests_error_delta=1), True), + ('runs', create_stats_with_delta(runs_delta=1), True), + ('runs succ', create_stats_with_delta(runs_succ_delta=1), True), + ('runs skip', create_stats_with_delta(runs_skip_delta=1), True), + ('runs fail', create_stats_with_delta(runs_fail_delta=1), True), + ('runs error', create_stats_with_delta(runs_error_delta=1), True)]: + with self.subTest(msg=label): + self.assertEqual(stats.has_changes, expected, msg=label) + + def unit_test_run_delta_results_has_failures(self): + def create_delta_stats(errors=[], tests_fail=0, tests_error=0, runs_fail=0, runs_error=0) -> UnitTestRunDeltaResults: + return create_unit_test_run_delta_results(errors=errors, tests_fail=tests_fail, tests_error=tests_error, runs_fail=runs_fail, runs_error=runs_error) + + for label, stats, expected in [('no failures', create_delta_stats(), False), + ('errors', create_delta_stats(errors=errors), False), + ('test failures', create_delta_stats(tests_fail=1), True), + ('test errors', create_delta_stats(tests_error=1), False), + ('runs failures', create_delta_stats(runs_fail=1), True), + ('runs errors', create_delta_stats(runs_error=1), False)]: + with self.subTest(msg=label): + self.assertEqual(stats.has_failures, expected, msg=label) + + def test_test_run_delta_results_has_errors(self): + def create_delta_stats(errors=[], tests_fail=0, tests_error=0, runs_fail=0, runs_error=0) -> UnitTestRunDeltaResults: + return create_unit_test_run_delta_results(errors=errors, tests_fail=tests_fail, tests_error=tests_error, runs_fail=runs_fail, runs_error=runs_error) + + for label, stats, expected in [('no errors', create_delta_stats(), False), + ('errors', create_delta_stats(errors=errors), True), + ('test failures', create_delta_stats(tests_fail=1), False), + ('test errors', create_delta_stats(tests_error=1), True), + ('runs failures', create_delta_stats(runs_fail=1), False), + ('runs errors', create_delta_stats(runs_error=1), True)]: + with self.subTest(msg=label): + self.assertEqual(stats.has_errors, expected, msg=label) + + def test_test_run_delta_results_without_delta(self): + with_deltas = create_unit_test_run_delta_results(files=1, errors=errors, suites=2, duration=3, + tests=4, tests_succ=5, tests_skip=6, tests_fail=7, tests_error=8, + runs=9, runs_succ=10, runs_skip=11, runs_fail=12, runs_error=13) + without_deltas = with_deltas.without_delta() + expected = create_unit_test_run_results(files=1, errors=errors, suites=2, duration=3, + tests=4, tests_succ=5, tests_skip=6, tests_fail=7, tests_error=8, + runs=9, runs_succ=10, runs_skip=11, runs_fail=12, runs_error=13) + self.assertEqual(expected, without_deltas)