Skip to content

Commit fadf6ea

Browse files
DanielNoordDanielNoord
authored
Compare difference between primer runs and post comment (#6723)
1 parent 41f5ece commit fadf6ea

File tree

3 files changed

+229
-2
lines changed

3 files changed

+229
-2
lines changed

‎.github/workflows/primer_comment.yaml

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Most of this is inspired by the mypy primer
2+
# See: https://github.com/hauntsaninja/mypy_primer
3+
# This is the primer job that creates the comment on the PR
4+
# It needs to trigger on workflow_run instead of pull_request
5+
# as we need repository wide access to create a comment
6+
7+
name: Primer / Comment
8+
9+
on:
10+
workflow_run:
11+
workflows: [Primer / Run]
12+
types:
13+
- completed
14+
15+
env:
16+
CACHE_VERSION: 1
17+
18+
permissions:
19+
contents: read
20+
pull-requests: write
21+
22+
jobs:
23+
primer-comment:
24+
name: Run
25+
runs-on: ubuntu-latest
26+
steps:
27+
- uses: actions/setup-node@v1
28+
with:
29+
version: 12
30+
- run: npm install @octokit/rest
31+
- name: Check out code from GitHub
32+
uses: actions/[email protected]
33+
- name: Set up Python 3.8
34+
id: python
35+
uses: actions/[email protected]
36+
with:
37+
python-version: 3.8
38+
39+
# Restore cached Python environment
40+
- name: Generate partial Python venv restore key
41+
id: generate-python-key
42+
run: >-
43+
echo "::set-output name=key::venv-${{ env.CACHE_VERSION }}-${{
44+
hashFiles('setup.cfg', 'requirements_test.txt', 'requirements_test_min.txt')
45+
}}"
46+
- name: Restore Python virtual environment
47+
id: cache-venv
48+
uses: actions/[email protected]
49+
with:
50+
path: venv
51+
key: >-
52+
${{ runner.os }}-${{ steps.python.outputs.python-version }}-${{
53+
steps.generate-python-key.outputs.key }}
54+
restore-keys: |
55+
${{ runner.os }}-${{ steps.python.outputs.python-version }}-venv-${{ env.CACHE_VERSION }}-
56+
- name: Create Python virtual environment
57+
if: steps.cache-venv.outputs.cache-hit != 'true'
58+
run: |
59+
python -m venv venv
60+
. venv/bin/activate
61+
python -m pip install -U pip setuptools wheel
62+
pip install -U -r requirements_test.txt
63+
64+
- name: Download outputs
65+
uses: actions/github-script@v6
66+
with:
67+
script: |
68+
// Download workflow pylint output
69+
const fs = require('fs');
70+
const artifacts_workflow = await github.rest.actions.listWorkflowRunArtifacts({
71+
owner: context.repo.owner,
72+
repo: context.repo.repo,
73+
run_id: ${{ github.event.workflow_run.id }},
74+
});
75+
76+
// Get 'main' output
77+
const [matchArtifactWorkflowMain] = artifacts_workflow.data.artifacts.filter((artifact) =>
78+
artifact.name == "primer_output_main");
79+
const downloadOne = await github.rest.actions.downloadArtifact({
80+
owner: context.repo.owner,
81+
repo: context.repo.repo,
82+
artifact_id: matchArtifactWorkflowMain.id,
83+
archive_format: "zip",
84+
});
85+
fs.writeFileSync("primer_main_output.zip", Buffer.from(downloadOne.data));
86+
87+
// Get PR output
88+
const [matchArtifactWorkflowPR] = artifacts_workflow.data.artifacts.filter((artifact) =>
89+
artifact.name == "primer_output_pr");
90+
const downloadTwo = await github.rest.actions.downloadArtifact({
91+
owner: context.repo.owner,
92+
repo: context.repo.repo,
93+
artifact_id: matchArtifactWorkflowPR.id,
94+
archive_format: "zip",
95+
});
96+
fs.writeFileSync("primer_pr_output.zip", Buffer.from(downloadTwo.data));
97+
98+
// Get PR number
99+
const [matchArtifactWorkflowNumber] = artifacts_workflow.data.artifacts.filter((artifact) =>
100+
artifact.name == "pr_number");
101+
const downloadThree = await github.rest.actions.downloadArtifact({
102+
owner: context.repo.owner,
103+
repo: context.repo.repo,
104+
artifact_id: matchArtifactWorkflowNumber.id,
105+
archive_format: "zip",
106+
});
107+
fs.writeFileSync("primer_pr_number.zip", Buffer.from(downloadThree.data));
108+
- run: unzip primer_main_output.zip
109+
- run: unzip primer_pr_output.zip
110+
- run: unzip primer_pr_number.zip
111+
- name: Compare outputs
112+
run: |
113+
. venv/bin/activate
114+
python tests/primer/primer_tool.py compare --base-file=output_main.txt --new-file=output_pr.txt
115+
- name: Post comment
116+
id: post-comment
117+
uses: actions/github-script@v6
118+
with:
119+
github-token: ${{ secrets.GITHUB_TOKEN }}
120+
script: |
121+
const fs = require('fs')
122+
const comment = fs.readFileSync('tests/.pylint_primer_tests/comment.txt', { encoding: 'utf8' })
123+
console.log("Comment to post:")
124+
console.log(comment)
125+
const prNumber = parseInt(fs.readFileSync("pr_number.txt", { encoding: "utf8" }))
126+
await github.rest.issues.createComment({
127+
issue_number: prNumber,
128+
owner: context.repo.owner,
129+
repo: context.repo.repo,
130+
body: comment
131+
})
132+
return prNumber
133+
- name: Hide old comments
134+
# Taken from mypy primer
135+
# v0.3.0
136+
uses: kanga333/comment-hider@bbdf5b562fbec24e6f60572d8f712017428b92e0
137+
with:
138+
github_token: ${{ secrets.GITHUB_TOKEN }}
139+
leave_visible: 1
140+
issue_number: ${{ steps.post-comment.outputs.result }}

‎tests/primer/packages_to_lint_batch_two.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
"directories": ["src_py"],
1616
"url": "https://github.com/pygame/pygame.git"
1717
},
18-
1918
"sentry": {
2019
"branch": "master",
2120
"directories": ["src/sentry"],

‎tests/primer/primer_tool.py

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,19 @@ def __init__(self, json_path: Path) -> None:
6161
"--type", choices=["main", "pr"], required=True, help="Type of primer run."
6262
)
6363

64+
# All arguments for the compare parser
65+
compare_parser = self._subparsers.add_parser("compare")
66+
compare_parser.add_argument(
67+
"--base-file",
68+
required=True,
69+
help="Location of output file of the base run.",
70+
)
71+
compare_parser.add_argument(
72+
"--new-file",
73+
required=True,
74+
help="Location of output file of the new run.",
75+
)
76+
6477
# Storing arguments
6578
self.config = self._argument_parser.parse_args()
6679

@@ -72,6 +85,8 @@ def run(self) -> None:
7285
self._handle_prepare_command()
7386
if self.config.command == "run":
7487
self._handle_run_command()
88+
if self.config.command == "compare":
89+
self._handle_compare_command()
7590

7691
def _handle_prepare_command(self) -> None:
7792
commit_string = ""
@@ -115,11 +130,84 @@ def _handle_run_command(self) -> None:
115130
) as f:
116131
json.dump(packages, f)
117132

133+
def _handle_compare_command(self) -> None:
134+
with open(self.config.base_file, encoding="utf-8") as f:
135+
main_dict: PackageMessages = json.load(f)
136+
with open(self.config.new_file, encoding="utf-8") as f:
137+
new_dict: PackageMessages = json.load(f)
138+
139+
final_main_dict: PackageMessages = {}
140+
for package, messages in main_dict.items():
141+
final_main_dict[package] = []
142+
for message in messages:
143+
try:
144+
new_dict[package].remove(message)
145+
except ValueError:
146+
final_main_dict[package].append(message)
147+
148+
self._create_comment(final_main_dict, new_dict)
149+
150+
def _create_comment(
151+
self, all_missing_messages: PackageMessages, all_new_messages: PackageMessages
152+
) -> None:
153+
comment = ""
154+
for package, missing_messages in all_missing_messages.items():
155+
new_messages = all_new_messages[package]
156+
package_data = self.packages[package]
157+
158+
if not missing_messages and not new_messages:
159+
continue
160+
161+
comment += f"\n\n**Effect on [{package}]({self.packages[package].url}):**\n"
162+
163+
if missing_messages:
164+
comment += (
165+
"The following messages are no longer emitted:\n\n<details>\n\n"
166+
)
167+
print("No longer emitted:")
168+
count = 1
169+
for message in missing_messages:
170+
comment += f"{count}) {message['symbol']}:\n*{message['message']}*\n"
171+
filepath = str(message["path"]).replace(
172+
str(package_data.clone_directory), ""
173+
)
174+
comment += f"{package_data.url}/blob/{package_data.branch}{filepath}#L{message['line']}\n"
175+
count += 1
176+
print(message)
177+
if missing_messages:
178+
comment += "\n</details>\n"
179+
180+
count = 1
181+
if new_messages:
182+
comment += "The following messages are now emitted:\n\n<details>\n\n"
183+
print("Now emitted:")
184+
for message in new_messages:
185+
comment += f"{count}) {message['symbol']}:\n*{message['message']}*\n"
186+
filepath = str(message["path"]).replace(
187+
str(package_data.clone_directory), ""
188+
)
189+
comment += f"{package_data.url}/blob/{package_data.branch}{filepath}#L{message['line']}\n"
190+
count += 1
191+
print(message)
192+
if new_messages:
193+
comment += "\n</details>\n"
194+
195+
if comment == "":
196+
comment = "🤖 According to the primer, this change has **no effect** on the checked open source code. 🤖🎉"
197+
else:
198+
comment = (
199+
"🤖 **Effect of this PR on checked open source code:** 🤖\n\n" + comment
200+
)
201+
202+
with open(PRIMER_DIRECTORY / "comment.txt", "w", encoding="utf-8") as f:
203+
f.write(comment)
204+
118205
def _lint_package(self, data: PackageToLint) -> list[dict[str, str | int]]:
119206
# We want to test all the code we can
120207
enables = ["--enable-all-extensions", "--enable=all"]
121208
# Duplicate code takes too long and is relatively safe
122-
disables = ["--disable=duplicate-code"]
209+
# TODO: Find a way to allow cyclic-import and compare output correctly
210+
disables = ["--disable=duplicate-code,cyclic-import"]
123211
arguments = data.pylint_args + enables + disables
124212
if data.pylintrc_relpath:
125213
arguments += [f"--rcfile={data.pylintrc_relpath}"]

0 commit comments

Comments
 (0)