diff --git a/pylint/testutils/_primer/primer_command.py b/pylint/testutils/_primer/primer_command.py index 8e9b95b8c3..bbc930913d 100644 --- a/pylint/testutils/_primer/primer_command.py +++ b/pylint/testutils/_primer/primer_command.py @@ -6,13 +6,25 @@ import abc import argparse +import sys from pathlib import Path -from typing import Dict, List +from typing import Dict -from pylint.message import Message +from pylint.reporters.json_reporter import OldJsonExport from pylint.testutils._primer import PackageToLint -PackageMessages = Dict[str, List[Message]] +if sys.version_info >= (3, 8): + from typing import TypedDict +else: + from typing_extensions import TypedDict + + +class PackageData(TypedDict): + commit: str + messages: list[OldJsonExport] + + +PackageMessages = Dict[str, PackageData] class PrimerCommand: diff --git a/pylint/testutils/_primer/primer_compare_command.py b/pylint/testutils/_primer/primer_compare_command.py index e161dce19b..baf28d8b7f 100644 --- a/pylint/testutils/_primer/primer_compare_command.py +++ b/pylint/testutils/_primer/primer_compare_command.py @@ -4,37 +4,45 @@ from __future__ import annotations import json -from pathlib import Path +from pathlib import Path, PurePosixPath -from pylint.testutils._primer.primer_command import PackageMessages, PrimerCommand +from pylint.reporters.json_reporter import OldJsonExport +from pylint.testutils._primer.primer_command import ( + PackageData, + PackageMessages, + PrimerCommand, +) MAX_GITHUB_COMMENT_LENGTH = 65536 class CompareCommand(PrimerCommand): def run(self) -> None: - main_messages = self._load_json(self.config.base_file) - pr_messages = self._load_json(self.config.new_file) - missing_messages, new_messages = self._cross_reference( - main_messages, pr_messages + main_data = self._load_json(self.config.base_file) + pr_data = self._load_json(self.config.new_file) + missing_messages_data, new_messages_data = self._cross_reference( + main_data, pr_data ) - comment = self._create_comment(missing_messages, new_messages) + comment = self._create_comment(missing_messages_data, new_messages_data) with open(self.primer_directory / "comment.txt", "w", encoding="utf-8") as f: f.write(comment) @staticmethod def _cross_reference( - main_dict: PackageMessages, pr_messages: PackageMessages + main_data: PackageMessages, pr_data: PackageMessages ) -> tuple[PackageMessages, PackageMessages]: - missing_messages: PackageMessages = {} - for package, messages in main_dict.items(): - missing_messages[package] = [] - for message in messages: + missing_messages_data: PackageMessages = {} + for package, data in main_data.items(): + package_missing_messages: list[OldJsonExport] = [] + for message in data["messages"]: try: - pr_messages[package].remove(message) + pr_data[package]["messages"].remove(message) except ValueError: - missing_messages[package].append(message) - return missing_messages, pr_messages + package_missing_messages.append(message) + missing_messages_data[package] = PackageData( + commit=pr_data[package]["commit"], messages=package_missing_messages + ) + return missing_messages_data, pr_data @staticmethod def _load_json(file_path: Path | str) -> PackageMessages: @@ -50,7 +58,7 @@ def _create_comment( if len(comment) >= MAX_GITHUB_COMMENT_LENGTH: break new_messages = all_new_messages[package] - if not missing_messages and not new_messages: + if not missing_messages["messages"] and not new_messages["messages"]: continue comment += self._create_comment_for_package( package, new_messages, missing_messages @@ -67,18 +75,20 @@ def _create_comment( return self._truncate_comment(comment) def _create_comment_for_package( - self, package: str, new_messages, missing_messages + self, package: str, new_messages: PackageData, missing_messages: PackageData ) -> str: comment = f"\n\n**Effect on [{package}]({self.packages[package].url}):**\n" # Create comment for new messages count = 1 astroid_errors = 0 new_non_astroid_messages = "" - if new_messages: + if new_messages["messages"]: print("Now emitted:") - for message in new_messages: - filepath = str(message["path"]).replace( - str(self.packages[package].clone_directory), "" + for message in new_messages["messages"]: + filepath = str( + PurePosixPath(message["path"]).relative_to( + self.packages[package].clone_directory + ) ) # Existing astroid errors may still show up as "new" because the timestamp # in the message is slightly different. @@ -87,7 +97,7 @@ def _create_comment_for_package( else: new_non_astroid_messages += ( f"{count}) {message['symbol']}:\n*{message['message']}*\n" - f"{self.packages[package].url}/blob/{self.packages[package].branch}{filepath}#L{message['line']}\n" + f"{self.packages[package].url}/blob/{new_messages['commit']}/{filepath}#L{message['line']}\n" ) print(message) count += 1 @@ -106,18 +116,20 @@ def _create_comment_for_package( # Create comment for missing messages count = 1 - if missing_messages: + if missing_messages["messages"]: comment += "The following messages are no longer emitted:\n\n
\n\n" print("No longer emitted:") - for message in missing_messages: + for message in missing_messages["messages"]: comment += f"{count}) {message['symbol']}:\n*{message['message']}*\n" - filepath = str(message["path"]).replace( - str(self.packages[package].clone_directory), "" + filepath = str( + PurePosixPath(message["path"]).relative_to( + self.packages[package].clone_directory + ) ) assert not self.packages[package].url.endswith( ".git" ), "You don't need the .git at the end of the github url." - comment += f"{self.packages[package].url}/blob/{self.packages[package].branch}{filepath}#L{message['line']}\n" + comment += f"{self.packages[package].url}/blob/{new_messages['commit']}/{filepath}#L{message['line']}\n" count += 1 print(message) if missing_messages: diff --git a/pylint/testutils/_primer/primer_run_command.py b/pylint/testutils/_primer/primer_run_command.py index bce8d8a04c..d2fce7793f 100644 --- a/pylint/testutils/_primer/primer_run_command.py +++ b/pylint/testutils/_primer/primer_run_command.py @@ -9,12 +9,18 @@ import warnings from io import StringIO +from git.repo import Repo + from pylint.lint import Run from pylint.message import Message from pylint.reporters import JSONReporter from pylint.reporters.json_reporter import OldJsonExport from pylint.testutils._primer.package_to_lint import PackageToLint -from pylint.testutils._primer.primer_command import PrimerCommand +from pylint.testutils._primer.primer_command import ( + PackageData, + PackageMessages, + PrimerCommand, +) GITHUB_CRASH_TEMPLATE_LOCATION = "/home/runner/.cache" CRASH_TEMPLATE_INTRO = "There is a pre-filled template" @@ -22,12 +28,13 @@ class RunCommand(PrimerCommand): def run(self) -> None: - packages: dict[str, list[OldJsonExport]] = {} + packages: PackageMessages = {} fatal_msgs: list[Message] = [] for package, data in self.packages.items(): messages, p_fatal_msgs = self._lint_package(package, data) fatal_msgs += p_fatal_msgs - packages[package] = messages + local_commit = Repo(data.clone_directory).head.object.hexsha + packages[package] = PackageData(commit=local_commit, messages=messages) path = ( self.primer_directory / f"output_{'.'.join(str(i) for i in sys.version_info[:3])}_{self.config.type}.txt" diff --git a/tests/testutils/_primer/fixtures/message_changed/expected.txt b/tests/testutils/_primer/fixtures/message_changed/expected.txt index 8a43727646..9640bed9e3 100644 --- a/tests/testutils/_primer/fixtures/message_changed/expected.txt +++ b/tests/testutils/_primer/fixtures/message_changed/expected.txt @@ -9,7 +9,7 @@ The following messages are now emitted: 1) locally-disabled: *Locally disabling redefined-builtin [we added some text in the message] (W0622)* -https://github.com/PyCQA/astroid/blob/main/astroid/__init__.py#L91 +https://github.com/PyCQA/astroid/blob/123456789abcdef/astroid/__init__.py#L91
@@ -19,7 +19,7 @@ The following messages are no longer emitted: 1) locally-disabled: *Locally disabling redefined-builtin (W0622)* -https://github.com/PyCQA/astroid/blob/main/astroid/__init__.py#L91 +https://github.com/PyCQA/astroid/blob/123456789abcdef/astroid/__init__.py#L91 diff --git a/tests/testutils/_primer/fixtures/message_changed/expected_truncated.txt b/tests/testutils/_primer/fixtures/message_changed/expected_truncated.txt index e8db96a3ed..3912ea15bc 100644 --- a/tests/testutils/_primer/fixtures/message_changed/expected_truncated.txt +++ b/tests/testutils/_primer/fixtures/message_changed/expected_truncated.txt @@ -9,12 +9,12 @@ The following messages are now emitted: 1) locally-disabled: *Locally disabling redefined-builtin [we added some text in the message] (W0622)* -https://github.com/PyCQA/astroid/blob/main/astroid/__init__.py#L91 +https://github.com/PyCQA/astroid/blob/123456789abcdef/astroid/__init__.py#L91 -The fol... +The following message... -*This comment was truncated because GitHub allows only 500 characters in a comment.* +*This comment was truncated because GitHub allows only 525 characters in a comment.* *This comment was generated for commit v2.14.2* diff --git a/tests/testutils/_primer/fixtures/message_changed/main.json b/tests/testutils/_primer/fixtures/message_changed/main.json index 987771f7aa..96f0aaaef7 100644 --- a/tests/testutils/_primer/fixtures/message_changed/main.json +++ b/tests/testutils/_primer/fixtures/message_changed/main.json @@ -1,30 +1,33 @@ { - "astroid": [ - { - "type": "info", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "locally-disabled", - "message": "Locally disabling redefined-builtin (W0622)", - "message-id": "I0011" - }, - { - "type": "warning", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "unknown-option-value", - "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", - "message-id": "W0012" - } - ] + "astroid": { + "commit": "123456789abcdef", + "messages": [ + { + "type": "info", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "locally-disabled", + "message": "Locally disabling redefined-builtin (W0622)", + "message-id": "I0011" + }, + { + "type": "warning", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "unknown-option-value", + "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", + "message-id": "W0012" + } + ] + } } diff --git a/tests/testutils/_primer/fixtures/message_changed/pr.json b/tests/testutils/_primer/fixtures/message_changed/pr.json index 460e6e99a2..9e2eda37b8 100644 --- a/tests/testutils/_primer/fixtures/message_changed/pr.json +++ b/tests/testutils/_primer/fixtures/message_changed/pr.json @@ -1,30 +1,33 @@ { - "astroid": [ - { - "type": "info", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "locally-disabled", - "message": "Locally disabling redefined-builtin [we added some text in the message] (W0622)", - "message-id": "I0011" - }, - { - "type": "warning", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "unknown-option-value", - "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", - "message-id": "W0012" - } - ] + "astroid": { + "commit": "123456789abcdef", + "messages": [ + { + "type": "info", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "locally-disabled", + "message": "Locally disabling redefined-builtin [we added some text in the message] (W0622)", + "message-id": "I0011" + }, + { + "type": "warning", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "unknown-option-value", + "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", + "message-id": "W0012" + } + ] + } } diff --git a/tests/testutils/_primer/fixtures/no_change/main.json b/tests/testutils/_primer/fixtures/no_change/main.json index 987771f7aa..cd361e6188 100644 --- a/tests/testutils/_primer/fixtures/no_change/main.json +++ b/tests/testutils/_primer/fixtures/no_change/main.json @@ -1,30 +1,33 @@ { - "astroid": [ - { - "type": "info", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "locally-disabled", - "message": "Locally disabling redefined-builtin (W0622)", - "message-id": "I0011" - }, - { - "type": "warning", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "unknown-option-value", - "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", - "message-id": "W0012" - } - ] + "astroid": { + "commit": "1234567890abcdef", + "messages": [ + { + "type": "info", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "locally-disabled", + "message": "Locally disabling redefined-builtin (W0622)", + "message-id": "I0011" + }, + { + "type": "warning", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "unknown-option-value", + "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", + "message-id": "W0012" + } + ] + } } diff --git a/tests/testutils/_primer/fixtures/no_change/pr.json b/tests/testutils/_primer/fixtures/no_change/pr.json index 987771f7aa..96f0aaaef7 100644 --- a/tests/testutils/_primer/fixtures/no_change/pr.json +++ b/tests/testutils/_primer/fixtures/no_change/pr.json @@ -1,30 +1,33 @@ { - "astroid": [ - { - "type": "info", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "locally-disabled", - "message": "Locally disabling redefined-builtin (W0622)", - "message-id": "I0011" - }, - { - "type": "warning", - "module": "astroid", - "obj": "", - "line": 91, - "column": 0, - "endLine": null, - "endColumn": null, - "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", - "symbol": "unknown-option-value", - "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", - "message-id": "W0012" - } - ] + "astroid": { + "commit": "123456789abcdef", + "messages": [ + { + "type": "info", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "locally-disabled", + "message": "Locally disabling redefined-builtin (W0622)", + "message-id": "I0011" + }, + { + "type": "warning", + "module": "astroid", + "obj": "", + "line": 91, + "column": 0, + "endLine": null, + "endColumn": null, + "path": "tests/.pylint_primer_tests/PyCQA/astroid/astroid/__init__.py", + "symbol": "unknown-option-value", + "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'", + "message-id": "W0012" + } + ] + } } diff --git a/tests/testutils/_primer/test_primer.py b/tests/testutils/_primer/test_primer.py index 28416de69b..d57e98d213 100644 --- a/tests/testutils/_primer/test_primer.py +++ b/tests/testutils/_primer/test_primer.py @@ -36,12 +36,6 @@ def test_primer_launch_bad_args(args: list[str], capsys: CaptureFixture) -> None assert "usage: Pylint Primer" in err -@pytest.mark.skipif( - sys.platform in {"win32", "darwin"}, - reason=( - "Primers are internal and will never be run on costly github action (mac or windows)" - ), -) @pytest.mark.skipif( sys.version_info[:2] != PRIMER_CURRENT_INTERPRETER or IS_PYPY, reason=( @@ -66,7 +60,7 @@ def test_compare(self, directory: Path) -> None: def test_truncated_compare(self) -> None: """Test for the truncation of comments that are too long.""" - max_comment_length = 500 + max_comment_length = 525 directory = FIXTURES_PATH / "message_changed" with patch( "pylint.testutils._primer.primer_compare_command.MAX_GITHUB_COMMENT_LENGTH",