Skip to content

Commit 01be3cc

Browse files
Pierre-SassoulasPierre-Sassoulas
committed
[refactor] Burst the primer command in three files
1 parent 942f0e4 commit 01be3cc

File tree

5 files changed

+285
-218
lines changed

5 files changed

+285
-218
lines changed

‎pylint/testutils/_primer/primer.py

Lines changed: 10 additions & 218 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,13 @@
66

77
import argparse
88
import json
9-
import sys
10-
import warnings
11-
from io import StringIO
12-
from itertools import chain
139
from pathlib import Path
14-
from typing import Dict, List, Union
1510

16-
import git
17-
18-
from pylint.lint import Run
19-
from pylint.reporters import JSONReporter
2011
from pylint.testutils._primer import PackageToLint
21-
22-
MAX_GITHUB_COMMENT_LENGTH = 65536
23-
24-
PackageMessages = Dict[str, List[Dict[str, Union[str, int]]]]
25-
26-
GITHUB_CRASH_TEMPLATE_LOCATION = "/home/runner/.cache"
27-
CRASH_TEMPLATE_INTRO = "There is a pre-filled template"
12+
from pylint.testutils._primer.primer_command import PrimerCommand
13+
from pylint.testutils._primer.primer_compare_command import CompareCommand
14+
from pylint.testutils._primer.primer_prepare_command import PrepareCommand
15+
from pylint.testutils._primer.primer_run_command import RunCommand
2816

2917

3018
class Primer:
@@ -90,212 +78,16 @@ def __init__(self, primer_directory: Path, json_path: Path) -> None:
9078
self.packages = self._get_packages_to_lint_from_json(json_path)
9179
"""All packages to prime."""
9280

93-
def run(self) -> None:
9481
if self.config.command == "prepare":
95-
self._handle_prepare_command()
82+
command_class: type[PrimerCommand] = PrepareCommand
9683
if self.config.command == "run":
97-
self._handle_run_command()
84+
command_class = RunCommand
9885
if self.config.command == "compare":
99-
self._handle_compare_command()
100-
101-
def _handle_prepare_command(self) -> None:
102-
commit_string = ""
103-
if self.config.clone:
104-
for package, data in self.packages.items():
105-
local_commit = data.lazy_clone()
106-
print(f"Cloned '{package}' at commit '{local_commit}'.")
107-
commit_string += local_commit + "_"
108-
elif self.config.check:
109-
for package, data in self.packages.items():
110-
local_commit = git.Repo(data.clone_directory).head.object.hexsha
111-
print(f"Found '{package}' at commit '{local_commit}'.")
112-
commit_string += local_commit + "_"
113-
elif self.config.make_commit_string:
114-
for package, data in self.packages.items():
115-
remote_sha1_commit = (
116-
git.cmd.Git().ls_remote(data.url, data.branch).split("\t")[0]
117-
)
118-
print(f"'{package}' remote is at commit '{remote_sha1_commit}'.")
119-
commit_string += remote_sha1_commit + "_"
120-
elif self.config.read_commit_string:
121-
with open(
122-
self.primer_directory / "commit_string.txt", encoding="utf-8"
123-
) as f:
124-
print(f.read())
125-
126-
if commit_string:
127-
with open(
128-
self.primer_directory / "commit_string.txt", "w", encoding="utf-8"
129-
) as f:
130-
f.write(commit_string)
131-
132-
def _handle_run_command(self) -> None:
133-
packages: PackageMessages = {}
134-
135-
for package, data in self.packages.items():
136-
output = self._lint_package(data)
137-
packages[package] = output
138-
print(f"Successfully primed {package}.")
139-
140-
astroid_errors = []
141-
other_fatal_msgs = []
142-
for msg in chain.from_iterable(packages.values()):
143-
if msg["type"] == "fatal":
144-
# Remove the crash template location if we're running on GitHub.
145-
# We were falsely getting "new" errors when the timestamp changed.
146-
assert isinstance(msg["message"], str)
147-
if GITHUB_CRASH_TEMPLATE_LOCATION in msg["message"]:
148-
msg["message"] = msg["message"].rsplit(CRASH_TEMPLATE_INTRO)[0]
149-
if msg["symbol"] == "astroid-error":
150-
astroid_errors.append(msg)
151-
else:
152-
other_fatal_msgs.append(msg)
153-
154-
with open(
155-
self.primer_directory
156-
/ f"output_{'.'.join(str(i) for i in sys.version_info[:3])}_{self.config.type}.txt",
157-
"w",
158-
encoding="utf-8",
159-
) as f:
160-
json.dump(packages, f)
161-
162-
# Fail loudly (and fail CI pipelines) if any fatal errors are found,
163-
# unless they are astroid-errors, in which case just warn.
164-
# This is to avoid introducing a dependency on bleeding-edge astroid
165-
# for pylint CI pipelines generally, even though we want to use astroid main
166-
# for the purpose of diffing emitted messages and generating PR comments.
167-
if astroid_errors:
168-
warnings.warn(f"Fatal errors traced to astroid: {astroid_errors}")
169-
assert not other_fatal_msgs, other_fatal_msgs
170-
171-
def _handle_compare_command(self) -> None:
172-
with open(self.config.base_file, encoding="utf-8") as f:
173-
main_dict: PackageMessages = json.load(f)
174-
with open(self.config.new_file, encoding="utf-8") as f:
175-
new_dict: PackageMessages = json.load(f)
86+
command_class = CompareCommand
87+
self.command = command_class(self.primer_directory, self.packages, self.config)
17688

177-
final_main_dict: PackageMessages = {}
178-
for package, messages in main_dict.items():
179-
final_main_dict[package] = []
180-
for message in messages:
181-
try:
182-
new_dict[package].remove(message)
183-
except ValueError:
184-
final_main_dict[package].append(message)
185-
186-
self._create_comment(final_main_dict, new_dict)
187-
188-
def _create_comment(
189-
self, all_missing_messages: PackageMessages, all_new_messages: PackageMessages
190-
) -> None:
191-
comment = ""
192-
for package, missing_messages in all_missing_messages.items():
193-
if len(comment) >= MAX_GITHUB_COMMENT_LENGTH:
194-
break
195-
196-
new_messages = all_new_messages[package]
197-
package_data = self.packages[package]
198-
199-
if not missing_messages and not new_messages:
200-
continue
201-
202-
comment += f"\n\n**Effect on [{package}]({self.packages[package].url}):**\n"
203-
204-
# Create comment for new messages
205-
count = 1
206-
astroid_errors = 0
207-
new_non_astroid_messages = ""
208-
if new_messages:
209-
print("Now emitted:")
210-
for message in new_messages:
211-
filepath = str(message["path"]).replace(
212-
str(package_data.clone_directory), ""
213-
)
214-
# Existing astroid errors may still show up as "new" because the timestamp
215-
# in the message is slightly different.
216-
if message["symbol"] == "astroid-error":
217-
astroid_errors += 1
218-
else:
219-
new_non_astroid_messages += (
220-
f"{count}) {message['symbol']}:\n*{message['message']}*\n"
221-
f"{package_data.url}/blob/{package_data.branch}{filepath}#L{message['line']}\n"
222-
)
223-
print(message)
224-
count += 1
225-
226-
if astroid_errors:
227-
comment += (
228-
f"{astroid_errors} error(s) were found stemming from the `astroid` library. "
229-
"This is unlikely to have been caused by your changes. "
230-
"A GitHub Actions warning links directly to the crash report template. "
231-
"Please open an issue against `astroid` if one does not exist already. \n\n"
232-
)
233-
if new_non_astroid_messages:
234-
comment += (
235-
"The following messages are now emitted:\n\n<details>\n\n"
236-
+ new_non_astroid_messages
237-
+ "\n</details>\n\n"
238-
)
239-
240-
# Create comment for missing messages
241-
count = 1
242-
if missing_messages:
243-
comment += (
244-
"The following messages are no longer emitted:\n\n<details>\n\n"
245-
)
246-
print("No longer emitted:")
247-
for message in missing_messages:
248-
comment += f"{count}) {message['symbol']}:\n*{message['message']}*\n"
249-
filepath = str(message["path"]).replace(
250-
str(package_data.clone_directory), ""
251-
)
252-
assert not package_data.url.endswith(
253-
".git"
254-
), "You don't need the .git at the end of the github url."
255-
comment += f"{package_data.url}/blob/{package_data.branch}{filepath}#L{message['line']}\n"
256-
count += 1
257-
print(message)
258-
if missing_messages:
259-
comment += "\n</details>\n\n"
260-
261-
if comment == "":
262-
comment = (
263-
"🤖 According to the primer, this change has **no effect** on the"
264-
" checked open source code. 🤖🎉\n\n"
265-
)
266-
else:
267-
comment = (
268-
f"🤖 **Effect of this PR on checked open source code:** 🤖\n\n{comment}"
269-
)
270-
hash_information = (
271-
f"*This comment was generated for commit {self.config.commit}*"
272-
)
273-
if len(comment) + len(hash_information) >= MAX_GITHUB_COMMENT_LENGTH:
274-
truncation_information = (
275-
f"*This comment was truncated because GitHub allows only"
276-
f" {MAX_GITHUB_COMMENT_LENGTH} characters in a comment.*"
277-
)
278-
max_len = (
279-
MAX_GITHUB_COMMENT_LENGTH
280-
- len(hash_information)
281-
- len(truncation_information)
282-
)
283-
comment = f"{comment[:max_len - 10]}...\n\n{truncation_information}\n\n"
284-
comment += hash_information
285-
with open(self.primer_directory / "comment.txt", "w", encoding="utf-8") as f:
286-
f.write(comment)
287-
288-
def _lint_package(self, data: PackageToLint) -> list[dict[str, str | int]]:
289-
# We want to test all the code we can
290-
enables = ["--enable-all-extensions", "--enable=all"]
291-
# Duplicate code takes too long and is relatively safe
292-
# TODO: Find a way to allow cyclic-import and compare output correctly
293-
disables = ["--disable=duplicate-code,cyclic-import"]
294-
arguments = data.pylint_args + enables + disables
295-
output = StringIO()
296-
reporter = JSONReporter(output)
297-
Run(arguments, reporter=reporter, exit=False)
298-
return json.loads(output.getvalue())
89+
def run(self) -> None:
90+
self.command.run()
29991

30092
@staticmethod
30193
def _get_packages_to_lint_from_json(json_path: Path) -> dict[str, PackageToLint]:

‎pylint/testutils/_primer/primer_command.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
2+
# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
3+
# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt
4+
5+
from __future__ import annotations
6+
7+
import abc
8+
import argparse
9+
from pathlib import Path
10+
from typing import Dict, List, Union
11+
12+
from pylint.testutils._primer import PackageToLint
13+
14+
PackageMessages = Dict[str, List[Dict[str, Union[str, int]]]]
15+
16+
17+
class PrimerCommand:
18+
19+
"""Generic primer action with required arguments."""
20+
21+
def __init__(
22+
self,
23+
primer_directory: Path,
24+
packages: dict[str, PackageToLint],
25+
config: argparse.Namespace,
26+
) -> None:
27+
self.primer_directory = primer_directory
28+
self.packages = packages
29+
self.config = config
30+
31+
@abc.abstractmethod
32+
def run(self) -> None:
33+
pass

0 commit comments

Comments
 (0)