Skip to content

Commit 8a74de0

Browse files
per1234per1234
committed
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
1 parent fb464e5 commit 8a74de0

File tree

4 files changed

+191
-54
lines changed

4 files changed

+191
-54
lines changed

‎README.md‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,29 @@ This action comments on the pull request with a report on the resulting change i
1010

1111
### `sketches-reports-source-name`
1212

13-
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.
13+
**Default**: "size-deltas-reports"
1414

15-
**Default**: `"size-deltas-reports"`
15+
The action can be used in two ways:
16+
17+
#### Run from a [scheduled workflow](https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#onschedule)
18+
19+
Recommended for public repositories.
20+
21+
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).
22+
23+
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.
24+
25+
#### Run from the same workflow as the [`arduino/compile-sketches`](https://github.com/arduino/compile-sketches) action
26+
27+
Recommended for private repositories.
28+
29+
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.
30+
31+
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.
32+
33+
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.
34+
35+
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.
1636

1737
### `github-token`
1838

@@ -22,6 +42,8 @@ Name of the [workflow artifact](https://docs.github.com/en/actions/configuring-a
2242

2343
## Example usage
2444

45+
### Scheduled workflow
46+
2547
```yaml
2648
on:
2749
schedule:
@@ -49,3 +71,53 @@ jobs:
4971
name: size-deltas-reports
5072
path: size-deltas-reports
5173
```
74+
75+
### Workflow triggered by `pull_request` event
76+
77+
```yaml
78+
on: [push, pull_request]
79+
env:
80+
# It's convenient to set variables for values used multiple times in the workflow
81+
SKETCHES_REPORTS_PATH: sketches-reports
82+
SKETCHES_REPORTS_ARTIFACT_NAME: sketches-reports
83+
jobs:
84+
compile:
85+
runs-on: ubuntu-latest
86+
strategy:
87+
matrix:
88+
fqbn:
89+
- "arduino:avr:uno"
90+
- "arduino:samd:mkrzero"
91+
steps:
92+
- uses: actions/checkout@v2
93+
94+
- uses: arduino/compile-sketches@main
95+
with:
96+
fqbn: ${{ matrix.fqbn }}
97+
enable-deltas-report: true
98+
sketches-report-path: ${{ env.SKETCHES_REPORTS_PATH }}
99+
100+
# This step is needed to pass the size data to the report job
101+
- name: Upload sketches report to workflow artifact
102+
uses: actions/upload-artifact@v2
103+
with:
104+
name: ${{ env.SKETCHES_REPORTS_ARTIFACT_NAME }}
105+
path: ${{ env.SKETCHES_REPORTS_PATH }}
106+
107+
# When using a matrix to compile for multiple boards, it's necessary to use a separate job for the deltas report
108+
report:
109+
needs: compile # Wait for the compile job to finish to get the data for the report
110+
if: github.event_name == 'pull_request' # Only run the job when the workflow is triggered by a pull request
111+
runs-on: ubuntu-latest
112+
steps:
113+
# This step is needed to get the size data produced by the compile job
114+
- name: Download sketches reports artifact
115+
uses: actions/download-artifact@v2
116+
with:
117+
name: ${{ env.SKETCHES_REPORTS_ARTIFACT_NAME }}
118+
path: ${{ env.SKETCHES_REPORTS_PATH }}
119+
120+
- uses: arduino/report-size-deltas@main
121+
with:
122+
sketches-reports-source-name: ${{ env.SKETCHES_REPORTS_PATH }}
123+
```

‎reportsizedeltas/reportsizedeltas.py‎

Lines changed: 71 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
import logging
55
import os
6+
import pathlib
67
import re
78
import sys
89
import tempfile
@@ -84,59 +85,76 @@ def __init__(self, repository_name, sketches_reports_source_name, token):
8485
self.token = token
8586

8687
def report_size_deltas(self):
87-
"""Scan the repository's pull requests and comment memory usage change reports where appropriate."""
88-
# Get the repository's pull requests
89-
logger.debug("Getting PRs for " + self.repository_name)
90-
page_number = 1
91-
page_count = 1
92-
while page_number <= page_count:
93-
api_data = self.api_request(request="repos/" + self.repository_name + "/pulls",
94-
page_number=page_number)
95-
prs_data = api_data["json_data"]
96-
for pr_data in prs_data:
97-
# Note: closed PRs are not listed in the API response
98-
pr_number = pr_data["number"]
99-
pr_head_sha = pr_data["head"]["sha"]
100-
print("::debug::Processing pull request number:", pr_number)
101-
# When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will
102-
# likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So
103-
# locking the thread would cause the job to fail.
104-
if pr_data["locked"]:
105-
print("::debug::PR locked, skipping")
106-
continue
107-
108-
if self.report_exists(pr_number=pr_number,
109-
pr_head_sha=pr_head_sha):
110-
# Go on to the next PR
111-
print("::debug::Report already exists")
112-
continue
113-
114-
artifact_download_url = self.get_artifact_download_url_for_sha(pr_user_login=pr_data["user"]["login"],
115-
pr_head_ref=pr_data["head"]["ref"],
116-
pr_head_sha=pr_head_sha)
117-
if artifact_download_url is None:
118-
# Go on to the next PR
119-
print("::debug::No sketches report artifact found")
120-
continue
121-
122-
artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url)
123-
124-
sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object)
125-
126-
if sketches_reports:
127-
if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha:
128-
# The deltas report key uses the hash from the report, but the report_exists() comparison is
129-
# done using the hash provided by the API. If for some reason the two didn't match, it would
130-
# result in the deltas report being done over and over again.
131-
print("::warning::Report commit hash doesn't match PR's head commit hash, skipping")
88+
"""Comment a report of memory usage change to pull request(s)."""
89+
if os.environ["GITHUB_EVENT_NAME"] == "pull_request":
90+
# The sketches reports will be in a local folder location specified by the user
91+
sketches_reports_folder = pathlib.Path(os.environ["GITHUB_WORKSPACE"], self.sketches_reports_source_name)
92+
sketches_reports = self.get_sketches_reports(artifact_folder_object=sketches_reports_folder)
93+
94+
if sketches_reports:
95+
report = self.generate_report(sketches_reports=sketches_reports)
96+
97+
with open(file=os.environ["GITHUB_EVENT_PATH"]) as github_event_file:
98+
pr_number = json.load(github_event_file)["pull_request"]["number"]
99+
100+
self.comment_report(pr_number=pr_number, report_markdown=report)
101+
102+
else:
103+
# The script is being run from a workflow triggered by something other than a PR
104+
# Scan the repository's pull requests and comment memory usage change reports where appropriate.
105+
# Get the repository's pull requests
106+
logger.debug("Getting PRs for " + self.repository_name)
107+
page_number = 1
108+
page_count = 1
109+
while page_number <= page_count:
110+
api_data = self.api_request(request="repos/" + self.repository_name + "/pulls",
111+
page_number=page_number)
112+
prs_data = api_data["json_data"]
113+
for pr_data in prs_data:
114+
# Note: closed PRs are not listed in the API response
115+
pr_number = pr_data["number"]
116+
pr_head_sha = pr_data["head"]["sha"]
117+
print("::debug::Processing pull request number:", pr_number)
118+
# When a PR is locked, only collaborators may comment. The automatically generated GITHUB_TOKEN will
119+
# likely be used, which is owned by the github-actions bot, who doesn't have collaborator status. So
120+
# locking the thread would cause the job to fail.
121+
if pr_data["locked"]:
122+
print("::debug::PR locked, skipping")
132123
continue
133124

134-
report = self.generate_report(sketches_reports=sketches_reports)
125+
if self.report_exists(pr_number=pr_number,
126+
pr_head_sha=pr_head_sha):
127+
# Go on to the next PR
128+
print("::debug::Report already exists")
129+
continue
135130

136-
self.comment_report(pr_number=pr_number, report_markdown=report)
131+
artifact_download_url = self.get_artifact_download_url_for_sha(
132+
pr_user_login=pr_data["user"]["login"],
133+
pr_head_ref=pr_data["head"]["ref"],
134+
pr_head_sha=pr_head_sha)
135+
if artifact_download_url is None:
136+
# Go on to the next PR
137+
print("::debug::No sketches report artifact found")
138+
continue
137139

138-
page_number += 1
139-
page_count = api_data["page_count"]
140+
artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url)
141+
142+
sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object)
143+
144+
if sketches_reports:
145+
if sketches_reports[0][self.ReportKeys.commit_hash] != pr_head_sha:
146+
# The deltas report key uses the hash from the report, but the report_exists() comparison is
147+
# done using the hash provided by the API. If for some reason the two didn't match, it would
148+
# result in the deltas report being done over and over again.
149+
print("::warning::Report commit hash doesn't match PR's head commit hash, skipping")
150+
continue
151+
152+
report = self.generate_report(sketches_reports=sketches_reports)
153+
154+
self.comment_report(pr_number=pr_number, report_markdown=report)
155+
156+
page_number += 1
157+
page_count = api_data["page_count"]
140158

141159
def report_exists(self, pr_number, pr_head_sha):
142160
"""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):
257275
artifact_folder_object -- object containing the data about the temporary folder that stores the markdown files
258276
"""
259277
with artifact_folder_object as artifact_folder:
278+
# artifact_folder will be a string when running in non-local report mode
279+
artifact_folder = pathlib.Path(artifact_folder)
260280
sketches_reports = []
261-
for report_filename in sorted(os.listdir(path=artifact_folder)):
281+
for report_filename in sorted(artifact_folder.iterdir()):
262282
# Combine sketches reports into an array
263-
with open(file=artifact_folder + "/" + report_filename) as report_file:
283+
with open(file=report_filename.joinpath(report_filename)) as report_file:
264284
report_data = json.load(report_file)
265285
if (
266286
(self.ReportKeys.boards not in report_data)

‎reportsizedeltas/tests/data/test_report_size_deltas_pull_request/githubevent.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"pull_request": {
3+
"head": {
4+
"sha": "pull_request-head-sha"
5+
},
6+
"number": 42
7+
}
8+
}

‎reportsizedeltas/tests/test_reportsizedeltas.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,42 @@ def test_set_verbosity():
177177
reportsizedeltas.set_verbosity(enable_verbosity=False)
178178

179179

180-
def test_report_size_deltas(mocker):
180+
def test_report_size_deltas_pull_request(mocker, monkeypatch):
181+
sketches_reports_source_name = "golden-sketches-reports-source-path"
182+
github_workspace = "golden-github-workspace"
183+
sketches_reports_folder = pathlib.Path(github_workspace, sketches_reports_source_name)
184+
sketches_reports = unittest.mock.sentinel.sketches_reports
185+
report = "golden report"
186+
187+
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
188+
monkeypatch.setenv("GITHUB_WORKSPACE", github_workspace)
189+
monkeypatch.setenv("GITHUB_EVENT_PATH",
190+
str(test_data_path.joinpath("test_report_size_deltas_pull_request", "githubevent.json")))
191+
192+
mocker.patch("reportsizedeltas.ReportSizeDeltas.get_sketches_reports", autospec=True)
193+
mocker.patch("reportsizedeltas.ReportSizeDeltas.generate_report", autospec=True, return_value=report)
194+
mocker.patch("reportsizedeltas.ReportSizeDeltas.comment_report", autospec=True)
195+
196+
report_size_deltas = get_reportsizedeltas_object(sketches_reports_source_name=sketches_reports_source_name)
197+
198+
# Test handling of no sketches reports data available
199+
reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None
200+
report_size_deltas.report_size_deltas()
201+
202+
report_size_deltas.comment_report.assert_not_called()
203+
204+
# Test report data available
205+
mocker.resetall()
206+
reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = sketches_reports
207+
report_size_deltas.report_size_deltas()
208+
209+
report_size_deltas.get_sketches_reports.assert_called_once_with(report_size_deltas,
210+
artifact_folder_object=sketches_reports_folder)
211+
report_size_deltas.generate_report.assert_called_once_with(report_size_deltas, sketches_reports=sketches_reports)
212+
report_size_deltas.comment_report.assert_called_once_with(report_size_deltas, pr_number=42, report_markdown=report)
213+
214+
215+
def test_report_size_deltas_not_pull_request(mocker, monkeypatch):
181216
artifact_download_url = "test_artifact_download_url"
182217
artifact_folder_object = "test_artifact_folder_object"
183218
pr_head_sha = "pr-head-sha"
@@ -186,6 +221,8 @@ def test_report_size_deltas(mocker):
186221
json_data = [{"number": 1, "locked": True, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}},
187222
{"number": 2, "locked": False, "head": {"sha": pr_head_sha, "ref": "asdf"}, "user": {"login": "1234"}}]
188223

224+
monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule")
225+
189226
report_size_deltas = get_reportsizedeltas_object()
190227

191228
mocker.patch("reportsizedeltas.ReportSizeDeltas.api_request",

0 commit comments

Comments
 (0)