[primer] Create a class for easier comparison of pylint results

pylint/testutils/_primer/comparator.py

@@ -0,0 +1,69 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE
+# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt
+
+from __future__ import annotations
+
+import json
+from collections.abc import Generator
+from pathlib import Path
+
+from pylint.reporters.json_reporter import OldJsonExport
+from pylint.testutils._primer.primer_command import (
+    PackageData,
+    PackageMessages,
+)
+
+
+class Comparator:
+    def __init__(
+        self, base_file: str, new_file: str, batches: int | None = None
+    ) -> None:
+        self._base_file = base_file
+        self._new_file = new_file
+        self._batches = batches
+
+    def __iter__(self) -> Generator[tuple[str, PackageData, PackageData]]:
+        main_data: PackageMessages
+        if self._batches is None:
+            main_data = self._load_json(self._base_file)
+            pr_data = self._load_json(self._new_file)
+        else:
+            main_data = {}
+            pr_data = {}
+            for idx in range(self._batches):
+                main_data.update(
+                    self._load_json(
+                        self._base_file.replace("BATCHIDX", "batch" + str(idx))
+                    )
+                )
+                pr_data.update(
+                    self._load_json(
+                        self._new_file.replace("BATCHIDX", "batch" + str(idx))
+                    )
+                )
+
+        missing_messages: PackageMessages = {}
+        for package, data in main_data.items():
+            package_missing_messages: list[OldJsonExport] = []
+            for message in data["messages"]:
+                try:
+                    pr_data[package]["messages"].remove(message)
+                except ValueError:
+                    package_missing_messages.append(message)
+            missing_messages[package] = PackageData(
+                commit=pr_data[package]["commit"],
+                messages=package_missing_messages,
+            )
+
+        for package, pkg_missing in missing_messages.items():
+            new_messages = pr_data[package]
+            if not pkg_missing["messages"] and not new_messages["messages"]:
+                continue
+            yield package, pkg_missing, new_messages
+
+    @staticmethod
+    def _load_json(file_path: Path | str) -> PackageMessages:
+        with open(file_path, encoding="utf-8") as f:
+            result: PackageMessages = json.load(f)
+        return result

pylint/testutils/_primer/primer_compare_command.py

@@ -3,79 +3,28 @@
 # Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt
 from __future__ import annotations
 
-import json
-from pathlib import Path, PurePosixPath
+from pathlib import PurePosixPath
 
-from pylint.reporters.json_reporter import OldJsonExport
-from pylint.testutils._primer.primer_command import (
-    PackageData,
-    PackageMessages,
-    PrimerCommand,
-)
+from pylint.testutils._primer.comparator import Comparator
+from pylint.testutils._primer.primer_command import PackageData, PrimerCommand
 
 MAX_GITHUB_COMMENT_LENGTH = 65536
 
 
 class CompareCommand(PrimerCommand):
     def run(self) -> None:
-        if self.config.batches is None:
-            main_data = self._load_json(self.config.base_file)
-            pr_data = self._load_json(self.config.new_file)
-        else:
-            main_data = {}
-            pr_data = {}
-            for idx in range(self.config.batches):
-                main_data.update(
-                    self._load_json(
-                        self.config.base_file.replace("BATCHIDX", "batch" + str(idx))
-                    )
-                )
-                pr_data.update(
-                    self._load_json(
-                        self.config.new_file.replace("BATCHIDX", "batch" + str(idx))
-                    )
-                )
-
-        missing_messages_data, new_messages_data = self._cross_reference(
-            main_data, pr_data
+        comparator = Comparator(
+            self.config.base_file, self.config.new_file, self.config.batches
         )
-        comment = self._create_comment(missing_messages_data, new_messages_data)
+        comment = self._create_comment(comparator)
         with open(self.primer_directory / "comment.txt", "w", encoding="utf-8") as f:
             f.write(comment)
 
-    @staticmethod
-    def _cross_reference(
-        main_data: PackageMessages, pr_data: PackageMessages
-    ) -> tuple[PackageMessages, PackageMessages]:
-        missing_messages_data: PackageMessages = {}
-        for package, data in main_data.items():
-            package_missing_messages: list[OldJsonExport] = []
-            for message in data["messages"]:
-                try:
-                    pr_data[package]["messages"].remove(message)
-                except ValueError:
-                    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:
-        with open(file_path, encoding="utf-8") as f:
-            result: PackageMessages = json.load(f)
-        return result
-
-    def _create_comment(
-        self, all_missing_messages: PackageMessages, all_new_messages: PackageMessages
-    ) -> str:
+    def _create_comment(self, comparator: Comparator) -> str:
         comment = ""
-        for package, missing_messages in all_missing_messages.items():
+        for package, missing_messages, new_messages in comparator:
             if len(comment) >= MAX_GITHUB_COMMENT_LENGTH:
                 break
-            new_messages = all_new_messages[package]
-            if not missing_messages["messages"] and not new_messages["messages"]:
-                continue
             comment += self._create_comment_for_package(
                 package, new_messages, missing_messages
             )

tests/config/test_find_default_config_files.py

@@ -60,9 +60,6 @@ def fake_home() -> Iterator[None]:
         shutil.rmtree(folder, ignore_errors=True)
 
 
-# pylint: enable=duplicate-code
-
-
 @contextlib.contextmanager
 def tempdir() -> Iterator[str]:
     """Create a temp directory and change the current location to it.
@@ -83,6 +80,9 @@ def tempdir() -> Iterator[str]:
         shutil.rmtree(abs_tmp)
 
 
+# pylint: enable=duplicate-code
+
+
 @pytest.mark.usefixtures("pop_pylintrc")
 def test_pylintrc() -> None:
     """Test that the environment variable is checked for existence."""

tests/testutils/_primer/batched_cases/expected_comparator.json

@@ -0,0 +1 @@
+[{ "package": "astroid", "missing": 2, "new": 0 }]

tests/testutils/_primer/cases/both_empty/expected_comparator.json

@@ -0,0 +1 @@
+[]

tests/testutils/_primer/cases/message_changed/expected_comparator.json

@@ -0,0 +1 @@
+[{ "package": "astroid", "missing": 1, "new": 1 }]

tests/testutils/_primer/cases/new_message/expected_comparator.json

@@ -0,0 +1 @@
+[{ "package": "astroid", "missing": 0, "new": 1 }]

tests/testutils/_primer/cases/no_change/expected_comparator.json

@@ -0,0 +1 @@
+[]

tests/testutils/_primer/cases/removed_message/expected_comparator.json

@@ -0,0 +1 @@
+[{ "package": "astroid", "missing": 1, "new": 0 }]

tests/testutils/_primer/test_comparator.py

@@ -0,0 +1,44 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE
+# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt
+
+import json
+from pathlib import Path
+
+import pytest
+
+from pylint.testutils._primer.comparator import Comparator
+
+CASES_PATH = Path(__file__).parent / "cases"
+
+
+@pytest.mark.parametrize(
+    "directory",
+    [pytest.param(p, id=p.name) for p in CASES_PATH.iterdir() if p.is_dir()],
+)
+def test_comparator(directory: Path) -> None:
+    """Test Comparator with each fixture directory."""
+    comparator = Comparator(str(directory / "main.json"), str(directory / "pr.json"))
+    expected = json.loads((directory / "expected_comparator.json").read_text("utf-8"))
+    results = list(comparator)
+    assert len(results) == len(expected)
+    for (package, missing, new), exp in zip(results, expected):
+        assert package == exp["package"]
+        assert len(missing["messages"]) == exp["missing"]
+        assert len(new["messages"]) == exp["new"]
+
+
+def test_comparator_batched() -> None:
+    fixture = Path(__file__).parent / "batched_cases"
+    comparator = Comparator(
+        str(fixture / "main_BATCHIDX.json"),
+        str(fixture / "pr_BATCHIDX.json"),
+        batches=2,
+    )
+    expected = json.loads((fixture / "expected_comparator.json").read_text("utf-8"))
+    results = list(comparator)
+    assert len(results) == len(expected)
+    for (package, missing, new), exp in zip(results, expected):
+        assert package == exp["package"]
+        assert len(missing["messages"]) == exp["missing"]
+        assert len(new["messages"]) == exp["new"]

tests/testutils/_primer/test_primer.py

@@ -20,7 +20,7 @@
 TEST_DIR_ROOT = HERE.parent.parent
 PRIMER_DIRECTORY = TEST_DIR_ROOT / ".pylint_primer_tests/"
 PACKAGES_TO_PRIME_PATH = TEST_DIR_ROOT / "primer/packages_to_prime.json"
-FIXTURES_PATH = HERE / "fixtures"
+CASES_PATH = HERE / "cases"
 
 # If you change this, also change DEFAULT_PYTHON in
 # ``.github/workflows/primer_comment.yaml``
@@ -52,20 +52,20 @@ class TestPrimer:
     @pytest.mark.parametrize(
         "directory",
         [
-            pytest.param(p, id=str(p.relative_to(FIXTURES_PATH)))
-            for p in FIXTURES_PATH.iterdir()
-            if p.is_dir() and p.name != "batched"  # tested separately
+            pytest.param(p, id=str(p.relative_to(CASES_PATH)))
+            for p in CASES_PATH.iterdir()
+            if p.is_dir()
         ],
     )
     def test_compare(self, directory: Path) -> None:
         """Test for the standard case.
 
-        Directory in 'fixtures/' with 'main.json', 'pr.json' and 'expected.txt'.
+        Directory in 'cases/' with 'main.json', 'pr.json' and 'expected.txt'.
         """
         self.__assert_expected(directory)
 
     def test_compare_batched(self) -> None:
-        fixture = FIXTURES_PATH / "batched"
+        fixture = HERE / "batched_cases"
         self.__assert_expected(
             fixture,
             fixture / "main_BATCHIDX.json",
@@ -76,7 +76,7 @@ def test_compare_batched(self) -> None:
     def test_truncated_compare(self) -> None:
         """Test for the truncation of comments that are too long."""
         max_comment_length = 525
-        directory = FIXTURES_PATH / "message_changed"
+        directory = CASES_PATH / "message_changed"
         with patch(
             "pylint.testutils._primer.primer_compare_command.MAX_GITHUB_COMMENT_LENGTH",
             max_comment_length,