diff --git a/README.md b/README.md index d7784b5..7d37723 100644 --- a/README.md +++ b/README.md @@ -8,11 +8,31 @@ 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 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` 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` 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 jobs + - 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: ${{ env.SKETCHES_REPORTS_PATH }} +``` diff --git a/action.yml b/action.yml index 9be0505..2e6c715 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: + 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..fe7406f 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 @@ -19,8 +20,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 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"], - artifact_name=os.environ["INPUT_SIZE-DELTAS-REPORTS-ARTIFACT-NAME"], + sketches_reports_source=os.environ["INPUT_SKETCHES-REPORTS-SOURCE"], token=os.environ["INPUT_GITHUB-TOKEN"]) report_size_deltas.report_size_deltas() @@ -73,12 +79,35 @@ class ReportKeys: sketches = "sketches" compilation_success = "compilation_success" - def __init__(self, repository_name, artifact_name, token): + def __init__(self, repository_name, sketches_reports_source, token): self.repository_name = repository_name - self.artifact_name = artifact_name + self.sketches_reports_source = sketches_reports_source self.token = token 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 + 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. + self.report_size_deltas_from_workflow_artifacts() + + 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) + 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) + + 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) @@ -106,9 +135,10 @@ def report_size_deltas(self): 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) + 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") @@ -209,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.artifact_name: + if artifact_data["name"] == self.sketches_reports_source: return artifact_data["archive_download_url"] page_number += 1 @@ -228,12 +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.artifact_name + ".zip", mode="wb") as out_file: + 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.artifact_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) @@ -251,10 +282,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 139335f..4378a61 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="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 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=sketches_reports_source, 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 = "golden-source-name" + token = "golden-github-token" + + monkeypatch.setenv("GITHUB_REPOSITORY", ActionInputs.repository_name) + monkeypatch.setenv("INPUT_SKETCHES-REPORTS-SOURCE", ActionInputs.sketches_reports_source) + 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=setup_environment_variables.sketches_reports_source, + 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 = size_deltas_report_artifact_name + else: + expected_sketches_reports_source = setup_environment_variables.sketches_reports_source + + 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 instead." + ) + + assert capsys.readouterr().out.strip() == expected_output + + assert os.environ["INPUT_SKETCHES-REPORTS-SOURCE"] == expected_sketches_reports_source + + def test_set_verbosity(): with pytest.raises(TypeError): reportsizedeltas.set_verbosity(enable_verbosity=2) @@ -123,7 +177,63 @@ def test_set_verbosity(): reportsizedeltas.set_verbosity(enable_verbosity=False) -def test_report_size_deltas(mocker): +# 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 = "golden-sketches-reports-source-path" + github_workspace = "golden-github-workspace" + sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source) + sketches_reports = unittest.mock.sentinel.sketches_reports + report = "golden report" + + 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=sketches_reports_source) + + # Test handling of no sketches reports data available + reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None + 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_from_local_reports() + + 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_from_workflow_artifacts(mocker): artifact_download_url = "test_artifact_download_url" artifact_folder_object = "test_artifact_folder_object" pr_head_sha = "pr-head-sha" @@ -151,7 +261,7 @@ def test_report_size_deltas(mocker): # 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) @@ -161,7 +271,7 @@ def test_report_size_deltas(mocker): 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() @@ -170,7 +280,7 @@ def test_report_size_deltas(mocker): 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() @@ -179,7 +289,7 @@ def test_report_size_deltas(mocker): 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() @@ -190,7 +300,7 @@ def test_report_size_deltas(mocker): 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() @@ -199,7 +309,7 @@ def test_report_size_deltas(mocker): 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 = [] @@ -303,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" - artifact_name = "test_artifact_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, - artifact_name=artifact_name) + sketches_reports_source=sketches_reports_source) - json_data = {"artifacts": [{"name": artifact_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, @@ -621,7 +731,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 +772,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) @@ -832,8 +942,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))