From a1aa1bc58e759901916c9d0bfdfd433286585a37 Mon Sep 17 00:00:00 2001 From: per1234 Date: Sat, 12 Sep 2020 19:43:11 -0700 Subject: [PATCH 1/5] Fix slow unit test The test was triggering an intentional delay in a function, resulting in this test causing a signficant increase in the time required to run the unit tests. The solution was to simply fake time.sleep() so it no longer causes a delay. --- reportsizedeltas/tests/test_reportsizedeltas.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index 139335f..5f3e4aa 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -832,8 +832,9 @@ def test_handle_rate_limiting(): report_size_deltas.handle_rate_limiting() -@pytest.mark.slow(reason="Causes a delay") -def test_determine_urlopen_retry_true(): +def test_determine_urlopen_retry_true(mocker): + mocker.patch("time.sleep", autospec=True) + assert reportsizedeltas.determine_urlopen_retry( exception=urllib.error.HTTPError(None, 502, "Bad Gateway", None, None)) From fb464e54d976797e36c63ae2855f1100f21c4337 Mon Sep 17 00:00:00 2001 From: per1234 Date: Sat, 12 Sep 2020 18:39:43 -0700 Subject: [PATCH 2/5] Rename size-deltas-reports-artifact-name input to sketches-reports-source-name Preparation to use the input for specifying the source of sketches reports either in a workflow artifact or a local folder. The old input name is still supported but warning will be displayed in the build log to explain the name change. --- README.md | 2 +- action.yml | 4 +- reportsizedeltas/reportsizedeltas.py | 18 ++-- .../tests/test_reportsizedeltas.py | 92 +++++++++++++++---- 4 files changed, 88 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index d7784b5..3fe3c02 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ This action comments on the pull request with a report on the resulting change i ## Inputs -### `size-deltas-reports-artifact-name` +### `sketches-reports-source-name` Name of the [workflow artifact](https://docs.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts) that contains the memory usage data, as specified to the [`actions/upload-artifact`](https://github.com/actions/upload-artifact) action via its `name` input. diff --git a/action.yml b/action.yml index 9be0505..c27e112 100644 --- a/action.yml +++ b/action.yml @@ -1,8 +1,8 @@ name: 'Report Arduino Sketch Size Deltas' description: 'Comments on the pull request with a report on the resulting change in memory usage of Arduino sketches' inputs: - size-deltas-reports-artifact-name: - description: 'Name of the workflow artifact that contains the memory usage data, as specified to the actions/upload-artifact action via its name input' + sketches-reports-source-name: + description: 'When run from scheduled workflow, name of the workflow artifact that contains sketches reports. When run from a pull request triggered workflow, path to the folder containing sketches reports.' default: 'size-deltas-reports' github-token: description: 'GitHub access token used to comment the memory usage comparison results to the PR thread' diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 89c71e6..4573110 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -19,8 +19,13 @@ def main(): set_verbosity(enable_verbosity=False) + if "INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME" in os.environ: + print("::warning::The size-deltas-report-artifact-name input is deprecated. Use the equivalent input: " + "sketches-reports-source-name instead.") + os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"] = os.environ["INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME"] + report_size_deltas = ReportSizeDeltas(repository_name=os.environ["GITHUB_REPOSITORY"], - artifact_name=os.environ["INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME"], + sketches_reports_source_name=os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"], token=os.environ["INPUT_GITHUB-TOKEN"]) report_size_deltas.report_size_deltas() @@ -73,9 +78,9 @@ class ReportKeys: sketches = "sketches" compilation_success = "compilation_success" - def __init__(self, repository_name, artifact_name, token): + def __init__(self, repository_name, sketches_reports_source_name, token): self.repository_name = repository_name - self.artifact_name = artifact_name + self.sketches_reports_source_name = sketches_reports_source_name self.token = token def report_size_deltas(self): @@ -209,7 +214,7 @@ def get_artifact_download_url_for_run(self, run_id): for artifact_data in artifacts_data["artifacts"]: # The artifact is identified by a specific name - if artifact_data["name"] == self.artifact_name: + if artifact_data["name"] == self.sketches_reports_source_name: return artifact_data["archive_download_url"] page_number += 1 @@ -228,12 +233,13 @@ def get_artifact(self, artifact_download_url): artifact_folder_object = tempfile.TemporaryDirectory(prefix="reportsizedeltas-") try: # Download artifact - with open(file=artifact_folder_object.name + "/" + self.artifact_name + ".zip", mode="wb") as out_file: + with open(file=artifact_folder_object.name + "/" + self.sketches_reports_source_name + ".zip", + mode="wb") as out_file: with self.raw_http_request(url=artifact_download_url) as fp: out_file.write(fp.read()) # Unzip artifact - artifact_zip_file = artifact_folder_object.name + "/" + self.artifact_name + ".zip" + artifact_zip_file = artifact_folder_object.name + "/" + self.sketches_reports_source_name + ".zip" with zipfile.ZipFile(file=artifact_zip_file, mode="r") as zip_ref: zip_ref.extractall(path=artifact_folder_object.name) os.remove(artifact_zip_file) diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index 5f3e4aa..9f7a970 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -1,6 +1,7 @@ import distutils.dir_util import filecmp import json +import os import pathlib import tempfile import unittest.mock @@ -18,16 +19,17 @@ def get_reportsizedeltas_object(repository_name="FooOwner/BarRepository", - artifact_name="foo-artifact-name", + sketches_reports_source_name="foo-artifact-name", token="foo token"): """Return a reportsizedeltas.ReportSizeDeltas object to use in tests. Keyword arguments: repository_name -- repository owner and name e.g., octocat/Hello-World - artifact_name -- name of the workflow artifact that contains the memory usage data + sketches_reports_source_name -- name of the workflow artifact that contains the memory usage data token -- GitHub access token """ - return reportsizedeltas.ReportSizeDeltas(repository_name=repository_name, artifact_name=artifact_name, token=token) + return reportsizedeltas.ReportSizeDeltas(repository_name=repository_name, + sketches_reports_source_name=sketches_reports_source_name, token=token) def directories_are_same(left_directory, right_directory): @@ -89,20 +91,31 @@ def test_directories_are_same(tmp_path): assert directories_are_same(left_directory=left_directory, right_directory=right_directory) is True -def test_main(monkeypatch, mocker): - repository_name = "FooOwner/BarRepository" - artifact_name = "foo-artifact-name" - token = "foo GitHub token" - monkeypatch.setenv("GITHUB_REPOSITORY", repository_name) - monkeypatch.setenv("INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME", artifact_name) - monkeypatch.setenv("INPUT_GITHUB-TOKEN", token) +@pytest.fixture +def setup_environment_variables(monkeypatch): + """Test fixture that sets up the environment variables required by reportsizedeltas.main() and returns an object + containing the values""" + class ActionInputs: + """A container for the values of the environment variables""" + repository_name = "GoldenOwner/GoldenRepository" + sketches_reports_source_name = "golden-artifact-name" + token = "golden-github-token" + + monkeypatch.setenv("GITHUB_REPOSITORY", ActionInputs.repository_name) + monkeypatch.setenv("INPUT_SKETCHES-REPORTS-SOURCE-NAME", ActionInputs.sketches_reports_source_name) + monkeypatch.setenv("INPUT_GITHUB-TOKEN", ActionInputs.token) + + return ActionInputs() + + +def test_main(monkeypatch, mocker, setup_environment_variables): class ReportSizeDeltas: """Stub""" def report_size_deltas(self): """Stub""" - pass # pragma: no cover + pass # pragma: no cover mocker.patch("reportsizedeltas.set_verbosity", autospec=True) mocker.patch("reportsizedeltas.ReportSizeDeltas", autospec=True, return_value=ReportSizeDeltas()) @@ -110,12 +123,53 @@ def report_size_deltas(self): reportsizedeltas.main() reportsizedeltas.set_verbosity.assert_called_once_with(enable_verbosity=False) - reportsizedeltas.ReportSizeDeltas.assert_called_once_with(repository_name=repository_name, - artifact_name=artifact_name, - token=token) + reportsizedeltas.ReportSizeDeltas.assert_called_once_with( + repository_name=setup_environment_variables.repository_name, + sketches_reports_source_name=setup_environment_variables.sketches_reports_source_name, + token=setup_environment_variables.token + ) ReportSizeDeltas.report_size_deltas.assert_called_once() +@pytest.mark.parametrize("use_size_deltas_report_artifact_name", [True, False]) +def test_main_size_deltas_report_artifact_name_deprecation_warning(capsys, + mocker, + monkeypatch, setup_environment_variables, + use_size_deltas_report_artifact_name): + size_deltas_report_artifact_name = "golden-size-deltas-report-artifact-name-value" + + if use_size_deltas_report_artifact_name: + monkeypatch.setenv("INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME", size_deltas_report_artifact_name) + expected_sketches_reports_source_name = size_deltas_report_artifact_name + else: + expected_sketches_reports_source_name = setup_environment_variables.sketches_reports_source_name + + class ReportSizeDeltas: + """Stub""" + + def report_size_deltas(self): + """Stub""" + pass # pragma: no cover + + mocker.patch("reportsizedeltas.set_verbosity", autospec=True) + mocker.patch("reportsizedeltas.ReportSizeDeltas", autospec=True, return_value=ReportSizeDeltas()) + mocker.patch.object(ReportSizeDeltas, "report_size_deltas") + + reportsizedeltas.main() + + expected_output = "" + if use_size_deltas_report_artifact_name: + expected_output = ( + expected_output + + "::warning::The size-deltas-report-artifact-name input is deprecated. Use the equivalent input: " + "sketches-reports-source-name instead." + ) + + assert capsys.readouterr().out.strip() == expected_output + + assert os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"] == expected_sketches_reports_source_name + + def test_set_verbosity(): with pytest.raises(TypeError): reportsizedeltas.set_verbosity(enable_verbosity=2) @@ -303,14 +357,14 @@ def test_get_artifact_download_url_for_sha(): def test_get_artifact_download_url_for_run(): repository_name = "test_name/test_repo" - artifact_name = "test_artifact_name" + sketches_reports_source_name = "test_sketches_reports_source_name" archive_download_url = "archive_download_url" run_id = "1234" report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name, - artifact_name=artifact_name) + sketches_reports_source_name=sketches_reports_source_name) - json_data = {"artifacts": [{"name": artifact_name, "archive_download_url": archive_download_url}, + json_data = {"artifacts": [{"name": sketches_reports_source_name, "archive_download_url": archive_download_url}, {"name": "bar123", "archive_download_url": "wrong_artifact_url"}]} report_size_deltas.api_request = unittest.mock.MagicMock(return_value={"json_data": json_data, "additional_pages": False, @@ -621,7 +675,7 @@ def test_get_sketches_reports(sketches_reports_path, expected_sketches_reports): try: distutils.dir_util.copy_tree(src=str(sketches_reports_path), dst=artifact_folder_object.name) - except Exception: # pragma: no cover + except Exception: # pragma: no cover artifact_folder_object.cleanup() raise sketches_reports = report_size_deltas.get_sketches_reports(artifact_folder_object=artifact_folder_object) @@ -662,7 +716,7 @@ def test_generate_report(): try: distutils.dir_util.copy_tree(src=str(sketches_report_path), dst=artifact_folder_object.name) - except Exception: # pragma: no cover + except Exception: # pragma: no cover artifact_folder_object.cleanup() raise sketches_reports = report_size_deltas.get_sketches_reports(artifact_folder_object=artifact_folder_object) From 8a74de05157641176de0229ef77164b57e790003 Mon Sep 17 00:00:00 2001 From: per1234 Date: Sat, 12 Sep 2020 19:09:15 -0700 Subject: [PATCH 3/5] Support using sketches reports from local path The action was previously designed to only run from a scheduled workflow. The reason is that it needs a token with write permissions to comment on the PR, but due to security restrictions there is no way to have such a token when a workflow is triggered by a pull_request event from a fork. This is problematic for private repos because if the schedule is set to a short interval the action will use up the free GitHub actoins minutes allocation quickly (public repos have unlimited minutes). If the schedule is set to a long interval, then there is a long potential wait time for the report. A common work flow in private repos is for PRs to be submitted from branches, not forks, which makes it possible to trigger the action from the PR. Running from a pull request triggered workflow should actually work as the action was, but the method of finding the artifact is very inefficient and unintuitive in that context. Recently, GitHub added the ability for private repositories to allow write permissions for workflows triggered by pull requests, making it even more likely this method of using the action will be found useful: https://docs.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository#enabling-workflows-for-private-repository-forks --- README.md | 76 ++++++++++- reportsizedeltas/reportsizedeltas.py | 122 ++++++++++-------- .../githubevent.json | 8 ++ .../tests/test_reportsizedeltas.py | 39 +++++- 4 files changed, 191 insertions(+), 54 deletions(-) create mode 100644 reportsizedeltas/tests/data/test_report_size_deltas_pull_request/githubevent.json diff --git a/README.md b/README.md index 3fe3c02..b328371 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,29 @@ This action comments on the pull request with a report on the resulting change i ### `sketches-reports-source-name` -Name of the [workflow artifact](https://docs.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts) that contains the memory usage data, as specified to the [`actions/upload-artifact`](https://github.com/actions/upload-artifact) action via its `name` input. +**Default**: "size-deltas-reports" -**Default**: `"size-deltas-reports"` +The action can be used in two ways: + +#### Run from a [scheduled workflow](https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#onschedule) + +Recommended for public repositories. + +The use of a scheduled workflow is necessary in order for the action to have the [write permissions required to comment on pull requests submitted from forks](https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token). + +In this usage, the `sketches-reports-source-name` defines the name of the workflow artifact that contains the memory usage data, as specified to the [`actions/upload-artifact`](https://github.com/actions/upload-artifact) action via its `name` input. + +#### Run from the same workflow as the [`arduino/compile-sketches`](https://github.com/arduino/compile-sketches) action + +Recommended for private repositories. + +If configured to trigger on a short interval, the scheduled workflow method can use a lot of GitHub Actions minutes, quickly using up the limited allotment provided by GitHub for private repositories (public repositories get unlimited free minutes). For this reason, it may be preferable to only run the action as needed. + +In order to get reports for pull requests from forks, the ["Send write tokens to workflows from fork pull requests" setting](https://docs.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository#enabling-workflows-for-private-repository-forks) must be enabled. + +If the "Send write tokens to workflows from fork pull requests" setting is not enabled but the ["Run workflows from fork pull requests" setting](https://docs.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository#enabling-workflows-for-private-repository-forks) is enabled, the workflow should be configured to only run the action when the pull request is not from a fork (`if: github.event.pull_request.head.repo.full_name == github.repository`). This will prevent workflow job failures that would otherwise be caused when the report creation failed due to not having the necessary write permissions. + +In this usage, the `sketches-reports-source-name` defines the path to the folder containing the memory usage data, as specified to the [`actions/download-artifact`](https://github.com/actions/download-artifact) action via its `path` input. ### `github-token` @@ -22,6 +42,8 @@ Name of the [workflow artifact](https://docs.github.com/en/actions/configuring-a ## Example usage +### Scheduled workflow + ```yaml on: schedule: @@ -49,3 +71,53 @@ jobs: name: size-deltas-reports path: size-deltas-reports ``` + +### Workflow triggered by `pull_request` event + +```yaml +on: [push, pull_request] +env: + # It's convenient to set variables for values used multiple times in the workflow + SKETCHES_REPORTS_PATH: sketches-reports + SKETCHES_REPORTS_ARTIFACT_NAME: sketches-reports +jobs: + compile: + runs-on: ubuntu-latest + strategy: + matrix: + fqbn: + - "arduino:avr:uno" + - "arduino:samd:mkrzero" + steps: + - uses: actions/checkout@v2 + + - uses: arduino/compile-sketches@main + with: + fqbn: ${{ matrix.fqbn }} + enable-deltas-report: true + sketches-report-path: ${{ env.SKETCHES_REPORTS_PATH }} + + # This step is needed to pass the size data to the report job + - name: Upload sketches report to workflow artifact + uses: actions/upload-artifact@v2 + with: + name: ${{ env.SKETCHES_REPORTS_ARTIFACT_NAME }} + path: ${{ env.SKETCHES_REPORTS_PATH }} + + # When using a matrix to compile for multiple boards, it's necessary to use a separate job for the deltas report + report: + needs: compile # Wait for the compile job to finish to get the data for the report + if: github.event_name == 'pull_request' # Only run the job when the workflow is triggered by a pull request + runs-on: ubuntu-latest + steps: + # This step is needed to get the size data produced by the compile job + - name: Download sketches reports artifact + uses: actions/download-artifact@v2 + with: + name: ${{ env.SKETCHES_REPORTS_ARTIFACT_NAME }} + path: ${{ env.SKETCHES_REPORTS_PATH }} + + - uses: arduino/report-size-deltas@main + with: + sketches-reports-source-name: ${{ env.SKETCHES_REPORTS_PATH }} +``` diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 4573110..0f26de5 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -3,6 +3,7 @@ import json import logging import os +import pathlib import re import sys import tempfile @@ -84,59 +85,76 @@ def __init__(self, repository_name, sketches_reports_source_name, token): self.token = token def report_size_deltas(self): - """Scan the repository's pull requests and comment memory usage change reports where appropriate.""" - # Get the repository's pull requests - logger.debug("Getting PRs for " + self.repository_name) - page_number = 1 - page_count = 1 - while page_number <= page_count: - api_data = self.api_request(request="repos/" + self.repository_name + "/pulls", - page_number=page_number) - prs_data = api_data["json_data"] - for pr_data in prs_data: - # Note: closed PRs are not listed in the API response - pr_number = pr_data["number"] - pr_head_sha = pr_data["head"]["sha"] - print("::debug::Processing pull request number:", pr_number) - # When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will - # likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So - # locking the thread would cause the job to fail. - if pr_data["locked"]: - print("::debug::PR locked, skipping") - continue - - if self.report_exists(pr_number=pr_number, - pr_head_sha=pr_head_sha): - # Go on to the next PR - print("::debug::Report already exists") - continue - - artifact_download_url = self.get_artifact_download_url_for_sha(pr_user_login=pr_data["user"]["login"], - pr_head_ref=pr_data["head"]["ref"], - pr_head_sha=pr_head_sha) - if artifact_download_url is None: - # Go on to the next PR - print("::debug::No sketches report artifact found") - continue - - artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url) - - sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object) - - if sketches_reports: - if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha: - # The deltas report key uses the hash from the report, but the report_exists() comparison is - # done using the hash provided by the API. If for some reason the two didn't match, it would - # result in the deltas report being done over and over again. - print("::warning::Report commit hash doesn't match PR's head commit hash, skipping") + """Comment a report of memory usage change to pull request(s).""" + if os.environ["GITHUB_EVENT_NAME"] == "pull_request": + # The sketches reports will be in a local folder location specified by the user + sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source_name) + sketches_reports = self.get_sketches_reports(artifact_folder_object=sketches_reports_folder) + + if sketches_reports: + report = self.generate_report(sketches_reports=sketches_reports) + + with open(file=os.environ["GITHUB_EVENT_PATH"]) as github_event_file: + pr_number = json.load(github_event_file)["pull_request"]["number"] + + self.comment_report(pr_number=pr_number, report_markdown=report) + + else: + # The script is being run from a workflow triggered by something other than a PR + # Scan the repository's pull requests and comment memory usage change reports where appropriate. + # Get the repository's pull requests + logger.debug("Getting PRs for " + self.repository_name) + page_number = 1 + page_count = 1 + while page_number <= page_count: + api_data = self.api_request(request="repos/" + self.repository_name + "/pulls", + page_number=page_number) + prs_data = api_data["json_data"] + for pr_data in prs_data: + # Note: closed PRs are not listed in the API response + pr_number = pr_data["number"] + pr_head_sha = pr_data["head"]["sha"] + print("::debug::Processing pull request number:", pr_number) + # When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will + # likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So + # locking the thread would cause the job to fail. + if pr_data["locked"]: + print("::debug::PR locked, skipping") continue - report = self.generate_report(sketches_reports=sketches_reports) + if self.report_exists(pr_number=pr_number, + pr_head_sha=pr_head_sha): + # Go on to the next PR + print("::debug::Report already exists") + continue - self.comment_report(pr_number=pr_number, report_markdown=report) + artifact_download_url = self.get_artifact_download_url_for_sha( + pr_user_login=pr_data["user"]["login"], + pr_head_ref=pr_data["head"]["ref"], + pr_head_sha=pr_head_sha) + if artifact_download_url is None: + # Go on to the next PR + print("::debug::No sketches report artifact found") + continue - page_number += 1 - page_count = api_data["page_count"] + artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url) + + sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object) + + if sketches_reports: + if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha: + # The deltas report key uses the hash from the report, but the report_exists() comparison is + # done using the hash provided by the API. If for some reason the two didn't match, it would + # result in the deltas report being done over and over again. + print("::warning::Report commit hash doesn't match PR's head commit hash, skipping") + continue + + report = self.generate_report(sketches_reports=sketches_reports) + + self.comment_report(pr_number=pr_number, report_markdown=report) + + page_number += 1 + page_count = api_data["page_count"] def report_exists(self, pr_number, pr_head_sha): """Return whether a report has already been commented to the pull request thread for the latest workflow run @@ -257,10 +275,12 @@ def get_sketches_reports(self, artifact_folder_object): artifact_folder_object -- object containing the data about the temporary folder that stores the markdown files """ with artifact_folder_object as artifact_folder: + # artifact_folder will be a string when running in non-local report mode + artifact_folder = pathlib.Path(artifact_folder) sketches_reports = [] - for report_filename in sorted(os.listdir(path=artifact_folder)): + for report_filename in sorted(artifact_folder.iterdir()): # Combine sketches reports into an array - with open(file=artifact_folder + "/" + report_filename) as report_file: + with open(file=report_filename.joinpath(report_filename)) as report_file: report_data = json.load(report_file) if ( (self.ReportKeys.boards not in report_data) diff --git a/reportsizedeltas/tests/data/test_report_size_deltas_pull_request/githubevent.json b/reportsizedeltas/tests/data/test_report_size_deltas_pull_request/githubevent.json new file mode 100644 index 0000000..7769ff1 --- /dev/null +++ b/reportsizedeltas/tests/data/test_report_size_deltas_pull_request/githubevent.json @@ -0,0 +1,8 @@ +{ + "pull_request": { + "head": { + "sha": "pull_request-head-sha" + }, + "number": 42 + } +} diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index 9f7a970..857f231 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -177,7 +177,42 @@ def test_set_verbosity(): reportsizedeltas.set_verbosity(enable_verbosity=False) -def test_report_size_deltas(mocker): +def test_report_size_deltas_pull_request(mocker, monkeypatch): + sketches_reports_source_name = "golden-sketches-reports-source-path" + github_workspace = "golden-github-workspace" + sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source_name) + sketches_reports = unittest.mock.sentinel.sketches_reports + report = "golden report" + + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + monkeypatch.setenv("GITHUB_WORKSPACE", github_workspace) + monkeypatch.setenv("GITHUB_EVENT_PATH", + str(test_data_path.joinpath("test_report_size_deltas_pull_request", "githubevent.json"))) + + mocker.patch("reportsizedeltas.ReportSizeDeltas.get_sketches_reports", autospec=True) + mocker.patch("reportsizedeltas.ReportSizeDeltas.generate_report", autospec=True, return_value=report) + mocker.patch("reportsizedeltas.ReportSizeDeltas.comment_report", autospec=True) + + report_size_deltas = get_reportsizedeltas_object(sketches_reports_source_name=sketches_reports_source_name) + + # Test handling of no sketches reports data available + reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None + report_size_deltas.report_size_deltas() + + report_size_deltas.comment_report.assert_not_called() + + # Test report data available + mocker.resetall() + reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = sketches_reports + report_size_deltas.report_size_deltas() + + report_size_deltas.get_sketches_reports.assert_called_once_with(report_size_deltas, + artifact_folder_object=sketches_reports_folder) + report_size_deltas.generate_report.assert_called_once_with(report_size_deltas, sketches_reports=sketches_reports) + report_size_deltas.comment_report.assert_called_once_with(report_size_deltas, pr_number=42, report_markdown=report) + + +def test_report_size_deltas_not_pull_request(mocker, monkeypatch): artifact_download_url = "test_artifact_download_url" artifact_folder_object = "test_artifact_folder_object" pr_head_sha = "pr-head-sha" @@ -186,6 +221,8 @@ def test_report_size_deltas(mocker): json_data = [{"number": 1, "locked": True, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}}, {"number": 2, "locked": False, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}}] + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + report_size_deltas = get_reportsizedeltas_object() mocker.patch("reportsizedeltas.ReportSizeDeltas.api_request", From 30810a6a63b2ad39658f9fe1892a6558a9f466f1 Mon Sep 17 00:00:00 2001 From: per1234 Date: Sun, 13 Sep 2020 00:34:54 -0700 Subject: [PATCH 4/5] Split discrete functionalities of ReportSizeDeltas.report_size_deltas() into separate functions The addition of support for sourcing the sketches reports from a local path resulted in the function containing code for two completely separate usages of the script. Moving each flavor of code to a separate function improves readability, maintainability, and testability. --- reportsizedeltas/reportsizedeltas.py | 119 +++++++++--------- .../tests/test_reportsizedeltas.py | 45 +++++-- 2 files changed, 95 insertions(+), 69 deletions(-) diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 0f26de5..7c3403a 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -88,73 +88,80 @@ def report_size_deltas(self): """Comment a report of memory usage change to pull request(s).""" if os.environ["GITHUB_EVENT_NAME"] == "pull_request": # The sketches reports will be in a local folder location specified by the user - sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source_name) - sketches_reports = self.get_sketches_reports(artifact_folder_object=sketches_reports_folder) - - if sketches_reports: - report = self.generate_report(sketches_reports=sketches_reports) - - with open(file=os.environ["GITHUB_EVENT_PATH"]) as github_event_file: - pr_number = json.load(github_event_file)["pull_request"]["number"] - - self.comment_report(pr_number=pr_number, report_markdown=report) - + self.report_size_deltas_from_local_reports() else: # The script is being run from a workflow triggered by something other than a PR # Scan the repository's pull requests and comment memory usage change reports where appropriate. - # Get the repository's pull requests - logger.debug("Getting PRs for " + self.repository_name) - page_number = 1 - page_count = 1 - while page_number <= page_count: - api_data = self.api_request(request="repos/" + self.repository_name + "/pulls", - page_number=page_number) - prs_data = api_data["json_data"] - for pr_data in prs_data: - # Note: closed PRs are not listed in the API response - pr_number = pr_data["number"] - pr_head_sha = pr_data["head"]["sha"] - print("::debug::Processing pull request number:", pr_number) - # When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will - # likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So - # locking the thread would cause the job to fail. - if pr_data["locked"]: - print("::debug::PR locked, skipping") - continue + self.report_size_deltas_from_workflow_artifacts() - if self.report_exists(pr_number=pr_number, - pr_head_sha=pr_head_sha): - # Go on to the next PR - print("::debug::Report already exists") - continue + def report_size_deltas_from_local_reports(self): + """Comment a report of memory usage change to the pull request.""" + sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source_name) + sketches_reports = self.get_sketches_reports(artifact_folder_object=sketches_reports_folder) - artifact_download_url = self.get_artifact_download_url_for_sha( - pr_user_login=pr_data["user"]["login"], - pr_head_ref=pr_data["head"]["ref"], - pr_head_sha=pr_head_sha) - if artifact_download_url is None: - # Go on to the next PR - print("::debug::No sketches report artifact found") - continue + if sketches_reports: + report = self.generate_report(sketches_reports=sketches_reports) - artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url) + with open(file=os.environ["GITHUB_EVENT_PATH"]) as github_event_file: + pr_number = json.load(github_event_file)["pull_request"]["number"] - sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object) + self.comment_report(pr_number=pr_number, report_markdown=report) - if sketches_reports: - if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha: - # The deltas report key uses the hash from the report, but the report_exists() comparison is - # done using the hash provided by the API. If for some reason the two didn't match, it would - # result in the deltas report being done over and over again. - print("::warning::Report commit hash doesn't match PR's head commit hash, skipping") - continue + def report_size_deltas_from_workflow_artifacts(self): + """Scan the repository's pull requests and comment memory usage change reports where appropriate.""" + # Get the repository's pull requests + logger.debug("Getting PRs for " + self.repository_name) + page_number = 1 + page_count = 1 + while page_number <= page_count: + api_data = self.api_request(request="repos/" + self.repository_name + "/pulls", + page_number=page_number) + prs_data = api_data["json_data"] + for pr_data in prs_data: + # Note: closed PRs are not listed in the API response + pr_number = pr_data["number"] + pr_head_sha = pr_data["head"]["sha"] + print("::debug::Processing pull request number:", pr_number) + # When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will + # likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So + # locking the thread would cause the job to fail. + if pr_data["locked"]: + print("::debug::PR locked, skipping") + continue + + if self.report_exists(pr_number=pr_number, + pr_head_sha=pr_head_sha): + # Go on to the next PR + print("::debug::Report already exists") + continue + + artifact_download_url = self.get_artifact_download_url_for_sha( + pr_user_login=pr_data["user"]["login"], + pr_head_ref=pr_data["head"]["ref"], + pr_head_sha=pr_head_sha) + if artifact_download_url is None: + # Go on to the next PR + print("::debug::No sketches report artifact found") + continue + + artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url) + + sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object) + + if sketches_reports: + if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha: + # The deltas report key uses the hash from the report, but the report_exists() comparison is + # done using the hash provided by the API. If for some reason the two didn't match, it would + # result in the deltas report being done over and over again. + print("::warning::Report commit hash doesn't match PR's head commit hash, skipping") + continue - report = self.generate_report(sketches_reports=sketches_reports) + report = self.generate_report(sketches_reports=sketches_reports) - self.comment_report(pr_number=pr_number, report_markdown=report) + self.comment_report(pr_number=pr_number, report_markdown=report) - page_number += 1 - page_count = api_data["page_count"] + page_number += 1 + page_count = api_data["page_count"] def report_exists(self, pr_number, pr_head_sha): """Return whether a report has already been commented to the pull request thread for the latest workflow run diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index 857f231..dc239b8 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -177,14 +177,35 @@ def test_set_verbosity(): reportsizedeltas.set_verbosity(enable_verbosity=False) -def test_report_size_deltas_pull_request(mocker, monkeypatch): +# noinspection PyUnresolvedReferences +def test_report_size_deltas(mocker, monkeypatch): + mocker.patch("reportsizedeltas.ReportSizeDeltas.report_size_deltas_from_local_reports", autospec=True) + mocker.patch("reportsizedeltas.ReportSizeDeltas.report_size_deltas_from_workflow_artifacts", autospec=True) + + report_size_deltas = get_reportsizedeltas_object() + + # Test triggered by pull_request event + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + report_size_deltas.report_size_deltas() + # noinspection PyUnresolvedReferences + report_size_deltas.report_size_deltas_from_local_reports.assert_called_once() + report_size_deltas.report_size_deltas_from_workflow_artifacts.assert_not_called() + + # Test triggered by other than pull_request event + mocker.resetall() + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_local_reports.assert_not_called() + report_size_deltas.report_size_deltas_from_workflow_artifacts.assert_called_once() + + +def test_report_size_deltas_from_local_reports(mocker, monkeypatch): sketches_reports_source_name = "golden-sketches-reports-source-path" github_workspace = "golden-github-workspace" sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source_name) sketches_reports = unittest.mock.sentinel.sketches_reports report = "golden report" - monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") monkeypatch.setenv("GITHUB_WORKSPACE", github_workspace) monkeypatch.setenv("GITHUB_EVENT_PATH", str(test_data_path.joinpath("test_report_size_deltas_pull_request", "githubevent.json"))) @@ -197,14 +218,14 @@ def test_report_size_deltas_pull_request(mocker, monkeypatch): # Test handling of no sketches reports data available reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_local_reports() report_size_deltas.comment_report.assert_not_called() # Test report data available mocker.resetall() reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = sketches_reports - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_local_reports() report_size_deltas.get_sketches_reports.assert_called_once_with(report_size_deltas, artifact_folder_object=sketches_reports_folder) @@ -212,7 +233,7 @@ def test_report_size_deltas_pull_request(mocker, monkeypatch): report_size_deltas.comment_report.assert_called_once_with(report_size_deltas, pr_number=42, report_markdown=report) -def test_report_size_deltas_not_pull_request(mocker, monkeypatch): +def test_report_size_deltas_from_workflow_artifacts(mocker): artifact_download_url = "test_artifact_download_url" artifact_folder_object = "test_artifact_folder_object" pr_head_sha = "pr-head-sha" @@ -221,8 +242,6 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): json_data = [{"number": 1, "locked": True, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}}, {"number": 2, "locked": False, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}}] - monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") - report_size_deltas = get_reportsizedeltas_object() mocker.patch("reportsizedeltas.ReportSizeDeltas.api_request", @@ -242,7 +261,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): # Test handling of locked PR mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_size_deltas.comment_report.assert_called_once_with(report_size_deltas, pr_number=2, report_markdown=report) @@ -252,7 +271,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): reportsizedeltas.ReportSizeDeltas.report_exists.return_value = True mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_size_deltas.comment_report.assert_not_called() @@ -261,7 +280,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): reportsizedeltas.ReportSizeDeltas.get_artifact_download_url_for_sha.return_value = None mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_size_deltas.comment_report.assert_not_called() @@ -270,7 +289,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_size_deltas.comment_report.assert_not_called() @@ -281,7 +300,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_size_deltas.comment_report.assert_not_called() @@ -290,7 +309,7 @@ def test_report_size_deltas_not_pull_request(mocker, monkeypatch): reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = sketches_reports mocker.resetall() - report_size_deltas.report_size_deltas() + report_size_deltas.report_size_deltas_from_workflow_artifacts() report_exists_calls = [] get_artifact_download_url_for_sha_calls = [] From d4ae92e54c38eb1a0dc930297f4a718f3e02e2ff Mon Sep 17 00:00:00 2001 From: per1234 Date: Sun, 13 Sep 2020 02:08:36 -0700 Subject: [PATCH 5/5] Use more concise input name I decided the "-name" part of the input name was superfluous. This is part of a single PR, so no further backwards compatibility measures are necessary. I just don't feel like making the effort to fixup the previous name change commit and deal with all the conflicts while rebasing. --- README.md | 10 +++--- action.yml | 2 +- reportsizedeltas/reportsizedeltas.py | 18 +++++------ .../tests/test_reportsizedeltas.py | 32 +++++++++---------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index b328371..7d37723 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ This action comments on the pull request with a report on the resulting change i ## Inputs -### `sketches-reports-source-name` +### `sketches-reports-source` **Default**: "size-deltas-reports" @@ -20,7 +20,7 @@ Recommended for public repositories. The use of a scheduled workflow is necessary in order for the action to have the [write permissions required to comment on pull requests submitted from forks](https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token). -In this usage, the `sketches-reports-source-name` defines the name of the workflow artifact that contains the memory usage data, as specified to the [`actions/upload-artifact`](https://github.com/actions/upload-artifact) action via its `name` input. +In this usage, the `sketches-reports-source` defines the name of the workflow artifact that contains the memory usage data, as specified to the [`actions/upload-artifact`](https://github.com/actions/upload-artifact) action via its `name` input. #### Run from the same workflow as the [`arduino/compile-sketches`](https://github.com/arduino/compile-sketches) action @@ -32,7 +32,7 @@ In order to get reports for pull requests from forks, the ["Send write tokens to If the "Send write tokens to workflows from fork pull requests" setting is not enabled but the ["Run workflows from fork pull requests" setting](https://docs.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository#enabling-workflows-for-private-repository-forks) is enabled, the workflow should be configured to only run the action when the pull request is not from a fork (`if: github.event.pull_request.head.repo.full_name == github.repository`). This will prevent workflow job failures that would otherwise be caused when the report creation failed due to not having the necessary write permissions. -In this usage, the `sketches-reports-source-name` defines the path to the folder containing the memory usage data, as specified to the [`actions/download-artifact`](https://github.com/actions/download-artifact) action via its `path` input. +In this usage, the `sketches-reports-source` defines the path to the folder containing the memory usage data, as specified to the [`actions/download-artifact`](https://github.com/actions/download-artifact) action via its `path` input. ### `github-token` @@ -110,7 +110,7 @@ jobs: if: github.event_name == 'pull_request' # Only run the job when the workflow is triggered by a pull request runs-on: ubuntu-latest steps: - # This step is needed to get the size data produced by the compile job + # This step is needed to get the size data produced by the compile jobs - name: Download sketches reports artifact uses: actions/download-artifact@v2 with: @@ -119,5 +119,5 @@ jobs: - uses: arduino/report-size-deltas@main with: - sketches-reports-source-name: ${{ env.SKETCHES_REPORTS_PATH }} + sketches-reports-source: ${{ env.SKETCHES_REPORTS_PATH }} ``` diff --git a/action.yml b/action.yml index c27e112..2e6c715 100644 --- a/action.yml +++ b/action.yml @@ -1,7 +1,7 @@ name: 'Report Arduino Sketch Size Deltas' description: 'Comments on the pull request with a report on the resulting change in memory usage of Arduino sketches' inputs: - sketches-reports-source-name: + sketches-reports-source: description: 'When run from scheduled workflow, name of the workflow artifact that contains sketches reports. When run from a pull request triggered workflow, path to the folder containing sketches reports.' default: 'size-deltas-reports' github-token: diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 7c3403a..fe7406f 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -22,11 +22,11 @@ def main(): if "INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME" in os.environ: print("::warning::The size-deltas-report-artifact-name input is deprecated. Use the equivalent input: " - "sketches-reports-source-name instead.") - os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"] = os.environ["INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME"] + "sketches-reports-source instead.") + os.environ["INPUT_SKETCHES-REPORTS-SOURCE"] = os.environ["INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME"] report_size_deltas = ReportSizeDeltas(repository_name=os.environ["GITHUB_REPOSITORY"], - sketches_reports_source_name=os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"], + sketches_reports_source=os.environ["INPUT_SKETCHES-REPORTS-SOURCE"], token=os.environ["INPUT_GITHUB-TOKEN"]) report_size_deltas.report_size_deltas() @@ -79,9 +79,9 @@ class ReportKeys: sketches = "sketches" compilation_success = "compilation_success" - def __init__(self, repository_name, sketches_reports_source_name, token): + def __init__(self, repository_name, sketches_reports_source, token): self.repository_name = repository_name - self.sketches_reports_source_name = sketches_reports_source_name + self.sketches_reports_source = sketches_reports_source self.token = token def report_size_deltas(self): @@ -96,7 +96,7 @@ def report_size_deltas(self): def report_size_deltas_from_local_reports(self): """Comment a report of memory usage change to the pull request.""" - sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source_name) + sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source) sketches_reports = self.get_sketches_reports(artifact_folder_object=sketches_reports_folder) if sketches_reports: @@ -239,7 +239,7 @@ def get_artifact_download_url_for_run(self, run_id): for artifact_data in artifacts_data["artifacts"]: # The artifact is identified by a specific name - if artifact_data["name"] == self.sketches_reports_source_name: + if artifact_data["name"] == self.sketches_reports_source: return artifact_data["archive_download_url"] page_number += 1 @@ -258,13 +258,13 @@ def get_artifact(self, artifact_download_url): artifact_folder_object = tempfile.TemporaryDirectory(prefix="reportsizedeltas-") try: # Download artifact - with open(file=artifact_folder_object.name + "/" + self.sketches_reports_source_name + ".zip", + with open(file=artifact_folder_object.name + "/" + self.sketches_reports_source + ".zip", mode="wb") as out_file: with self.raw_http_request(url=artifact_download_url) as fp: out_file.write(fp.read()) # Unzip artifact - artifact_zip_file = artifact_folder_object.name + "/" + self.sketches_reports_source_name + ".zip" + artifact_zip_file = artifact_folder_object.name + "/" + self.sketches_reports_source + ".zip" with zipfile.ZipFile(file=artifact_zip_file, mode="r") as zip_ref: zip_ref.extractall(path=artifact_folder_object.name) os.remove(artifact_zip_file) diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index dc239b8..4378a61 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -19,17 +19,17 @@ def get_reportsizedeltas_object(repository_name="FooOwner/BarRepository", - sketches_reports_source_name="foo-artifact-name", + sketches_reports_source="foo-artifact-name", token="foo token"): """Return a reportsizedeltas.ReportSizeDeltas object to use in tests. Keyword arguments: repository_name -- repository owner and name e.g., octocat/Hello-World - sketches_reports_source_name -- name of the workflow artifact that contains the memory usage data + sketches_reports_source -- name of the workflow artifact that contains the memory usage data token -- GitHub access token """ return reportsizedeltas.ReportSizeDeltas(repository_name=repository_name, - sketches_reports_source_name=sketches_reports_source_name, token=token) + sketches_reports_source=sketches_reports_source, token=token) def directories_are_same(left_directory, right_directory): @@ -99,11 +99,11 @@ def setup_environment_variables(monkeypatch): class ActionInputs: """A container for the values of the environment variables""" repository_name = "GoldenOwner/GoldenRepository" - sketches_reports_source_name = "golden-artifact-name" + sketches_reports_source = "golden-source-name" token = "golden-github-token" monkeypatch.setenv("GITHUB_REPOSITORY", ActionInputs.repository_name) - monkeypatch.setenv("INPUT_SKETCHES-REPORTS-SOURCE-NAME", ActionInputs.sketches_reports_source_name) + monkeypatch.setenv("INPUT_SKETCHES-REPORTS-SOURCE", ActionInputs.sketches_reports_source) monkeypatch.setenv("INPUT_GITHUB-TOKEN", ActionInputs.token) return ActionInputs() @@ -125,7 +125,7 @@ def report_size_deltas(self): reportsizedeltas.set_verbosity.assert_called_once_with(enable_verbosity=False) reportsizedeltas.ReportSizeDeltas.assert_called_once_with( repository_name=setup_environment_variables.repository_name, - sketches_reports_source_name=setup_environment_variables.sketches_reports_source_name, + sketches_reports_source=setup_environment_variables.sketches_reports_source, token=setup_environment_variables.token ) ReportSizeDeltas.report_size_deltas.assert_called_once() @@ -140,9 +140,9 @@ def test_main_size_deltas_report_artifact_name_deprecation_warning(capsys, if use_size_deltas_report_artifact_name: monkeypatch.setenv("INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME", size_deltas_report_artifact_name) - expected_sketches_reports_source_name = size_deltas_report_artifact_name + expected_sketches_reports_source = size_deltas_report_artifact_name else: - expected_sketches_reports_source_name = setup_environment_variables.sketches_reports_source_name + expected_sketches_reports_source = setup_environment_variables.sketches_reports_source class ReportSizeDeltas: """Stub""" @@ -162,12 +162,12 @@ def report_size_deltas(self): expected_output = ( expected_output + "::warning::The size-deltas-report-artifact-name input is deprecated. Use the equivalent input: " - "sketches-reports-source-name instead." + "sketches-reports-source instead." ) assert capsys.readouterr().out.strip() == expected_output - assert os.environ["INPUT_SKETCHES-REPORTS-SOURCE-NAME"] == expected_sketches_reports_source_name + assert os.environ["INPUT_SKETCHES-REPORTS-SOURCE"] == expected_sketches_reports_source def test_set_verbosity(): @@ -200,9 +200,9 @@ def test_report_size_deltas(mocker, monkeypatch): def test_report_size_deltas_from_local_reports(mocker, monkeypatch): - sketches_reports_source_name = "golden-sketches-reports-source-path" + sketches_reports_source = "golden-sketches-reports-source-path" github_workspace = "golden-github-workspace" - sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source_name) + sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source) sketches_reports = unittest.mock.sentinel.sketches_reports report = "golden report" @@ -214,7 +214,7 @@ def test_report_size_deltas_from_local_reports(mocker, monkeypatch): mocker.patch("reportsizedeltas.ReportSizeDeltas.generate_report", autospec=True, return_value=report) mocker.patch("reportsizedeltas.ReportSizeDeltas.comment_report", autospec=True) - report_size_deltas = get_reportsizedeltas_object(sketches_reports_source_name=sketches_reports_source_name) + report_size_deltas = get_reportsizedeltas_object(sketches_reports_source=sketches_reports_source) # Test handling of no sketches reports data available reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None @@ -413,14 +413,14 @@ def test_get_artifact_download_url_for_sha(): def test_get_artifact_download_url_for_run(): repository_name = "test_name/test_repo" - sketches_reports_source_name = "test_sketches_reports_source_name" + sketches_reports_source = "test_sketches_reports_source" archive_download_url = "archive_download_url" run_id = "1234" report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name, - sketches_reports_source_name=sketches_reports_source_name) + sketches_reports_source=sketches_reports_source) - json_data = {"artifacts": [{"name": sketches_reports_source_name, "archive_download_url": archive_download_url}, + json_data = {"artifacts": [{"name": sketches_reports_source, "archive_download_url": archive_download_url}, {"name": "bar123", "archive_download_url": "wrong_artifact_url"}]} report_size_deltas.api_request = unittest.mock.MagicMock(return_value={"json_data": json_data, "additional_pages": False,