Skip to content

Commit 546b930

Browse files
Pierre-SassoulasPierre-Sassoulas
committed
[primer] Cover classification branches and truncation edge cases
Adds fixtures and unit tests that bring coverage on comparator.py and primer_compare_command.py to 100% — all message-classification branches (astroid-error, useless-suppression fix, locally-disabled FP, genuine new message) and both truncation code paths (multi-package iteration break and no-space fallback) now have dedicated tests.
1 parent 5ea1f49 commit 546b930

8 files changed

Lines changed: 203 additions & 1 deletion

File tree

‎doc/whatsnew/fragments/10914.internal‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
The primer now pairs residual messages by ``(symbol, path, obj)`` and reports altered
2+
diagnostics as a single *changed* entry with a compact diff, rather than as a separate
3+
removal + addition. New messages are classified into genuine new messages, fixed false
4+
positives (``useless-suppression``), new false positives (``locally-disabled``), and
5+
``astroid-error`` fatal errors.
6+
7+
Refs #10914

‎tests/testutils/_primer/cases/multi_package/expected_truncated_break.txt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
🤖 **Effect of this PR on checked open source code:** 🤖
2+
3+
4+
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
5+
6+
Changed messages:
7+
8+
<details>
9+
10+
1)...
11+
</details>
12+
13+
*This comment was truncated because GitHub allows only 400 characters in a comment.*
14+
15+
*This comment was generated for commit v2.14.2*

‎tests/testutils/_primer/cases/new_messages_classified/expected.txt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
🤖 **Effect of this PR on checked open source code:** 🤖
2+
3+
4+
**Effect on [astropy](https://github.com/astropy/astropy):**
5+
6+
1 "astroid error(s)" were found. Please open the GitHub Actions log to see what failed or crashed.
7+
8+
🎉 Fixed false positives:
9+
10+
<details>
11+
12+
1) useless-suppression:
13+
*Useless suppression of 'looping-through-iterator'*
14+
https://github.com/astropy/astropy/blob/1fb40bc1f22f176254ef583065aa155f53f3b414/astropy/coordinates/angles/angle_parsetab.py#L14
15+
</details>
16+
17+
😞 New false positives:
18+
19+
<details>
20+
21+
1) locally-disabled:
22+
*Locally disabling looping-through-iterator (W4801)*
23+
https://github.com/astropy/astropy/blob/1fb40bc1f22f176254ef583065aa155f53f3b414/astropy/coordinates/angles/angle_parsetab.py#L14
24+
</details>
25+
26+
New messages:
27+
28+
<details>
29+
30+
1) unused-variable:
31+
*Unused variable 'foo'*
32+
https://github.com/astropy/astropy/blob/1fb40bc1f22f176254ef583065aa155f53f3b414/astropy/coordinates/angles/angle_parsetab.py#L42
33+
</details>
34+
35+
*This comment was generated for commit v2.14.2*

‎tests/testutils/_primer/cases/new_messages_classified/expected_comparator.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[{ "package": "astropy", "missing": 0, "new": 4, "changed": 0 }]

‎tests/testutils/_primer/cases/new_messages_classified/main.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"astropy": {
3+
"commit": "1fb40bc1f22f176254ef583065aa155f53f3b414",
4+
"messages": []
5+
}
6+
}

‎tests/testutils/_primer/cases/new_messages_classified/pr.json‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
{
2+
"astropy": {
3+
"commit": "1fb40bc1f22f176254ef583065aa155f53f3b414",
4+
"messages": [
5+
{
6+
"type": "error",
7+
"module": "astropy.coordinates.angles.angle_parsetab",
8+
"obj": "",
9+
"line": 1,
10+
"column": 0,
11+
"endLine": null,
12+
"endColumn": null,
13+
"path": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py",
14+
"symbol": "astroid-error",
15+
"message": "Fatal error while checking 'astropy/module.py'. Please open an issue.",
16+
"messageId": "F0002",
17+
"confidence": "UNDEFINED",
18+
"absolutePath": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py"
19+
},
20+
{
21+
"type": "info",
22+
"module": "astropy.coordinates.angles.angle_parsetab",
23+
"obj": "",
24+
"line": 14,
25+
"column": 0,
26+
"endLine": null,
27+
"endColumn": null,
28+
"path": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py",
29+
"symbol": "locally-disabled",
30+
"message": "Locally disabling looping-through-iterator (W4801)",
31+
"messageId": "I0011",
32+
"confidence": "UNDEFINED",
33+
"absolutePath": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py"
34+
},
35+
{
36+
"type": "warning",
37+
"module": "astropy.coordinates.angles.angle_parsetab",
38+
"obj": "",
39+
"line": 14,
40+
"column": 0,
41+
"endLine": null,
42+
"endColumn": null,
43+
"path": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py",
44+
"symbol": "useless-suppression",
45+
"message": "Useless suppression of 'looping-through-iterator'",
46+
"messageId": "I0021",
47+
"confidence": "UNDEFINED",
48+
"absolutePath": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py"
49+
},
50+
{
51+
"type": "warning",
52+
"module": "astropy.coordinates.angles.angle_parsetab",
53+
"obj": "my_function",
54+
"line": 42,
55+
"column": 4,
56+
"endLine": 42,
57+
"endColumn": 16,
58+
"path": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py",
59+
"symbol": "unused-variable",
60+
"message": "Unused variable 'foo'",
61+
"messageId": "W0612",
62+
"confidence": "UNDEFINED",
63+
"absolutePath": "tests/.pylint_primer_tests/astropy/astropy/astropy/coordinates/angles/angle_parsetab.py"
64+
}
65+
]
66+
}
67+
}

‎tests/testutils/_primer/test_comparator.py‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,40 @@
44

55
import json
66
from pathlib import Path
7+
from typing import cast
78

89
import pytest
910

10-
from pylint.testutils._primer.comparator import Comparator
11+
from pylint.reporters.json_reporter import JSONMessage
12+
from pylint.testutils._primer.comparator import (
13+
Comparator,
14+
_caret_hint,
15+
format_span,
16+
)
1117

1218
CASES_PATH = Path(__file__).parent / "cases"
1319

1420

21+
def _msg(**overrides: object) -> JSONMessage:
22+
base: dict[str, object] = {
23+
"type": "warning",
24+
"module": "m",
25+
"obj": "",
26+
"line": 1,
27+
"column": 0,
28+
"endLine": 1,
29+
"endColumn": 5,
30+
"path": "m.py",
31+
"symbol": "s",
32+
"message": "msg",
33+
"messageId": "W0000",
34+
"confidence": "UNDEFINED",
35+
"absolutePath": "m.py",
36+
}
37+
base.update(overrides)
38+
return cast(JSONMessage, base)
39+
40+
1541
@pytest.mark.parametrize(
1642
"directory",
1743
[pytest.param(p, id=p.name) for p in CASES_PATH.iterdir() if p.is_dir()],
@@ -29,6 +55,18 @@ def test_comparator(directory: Path) -> None:
2955
assert len(changed) == exp["changed"]
3056

3157

58+
def test_format_span_without_end_position() -> None:
59+
"""Messages missing endLine/endColumn fall back to a start-only span."""
60+
assert (
61+
format_span(_msg(line=42, column=7, endLine=None, endColumn=None)) == "`42:7`"
62+
)
63+
64+
65+
def test_caret_hint_returns_empty_when_mostly_changed() -> None:
66+
"""When more than 60% of the text differs, carets are noise and are skipped."""
67+
assert _caret_hint("hello world", "goodbye universe") == ""
68+
69+
3270
def test_comparator_batched() -> None:
3371
fixture = Path(__file__).parent / "batched_cases"
3472
comparator = Comparator.from_json(

‎tests/testutils/_primer/test_primer.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from __future__ import annotations
88

9+
import argparse
910
import sys
1011
from pathlib import Path
1112
from unittest.mock import patch
@@ -15,6 +16,7 @@
1516

1617
from pylint.constants import IS_PYPY
1718
from pylint.testutils._primer.primer import Primer
19+
from pylint.testutils._primer.primer_compare_command import CompareCommand
1820

1921
HERE = Path(__file__).parent
2022
TEST_DIR_ROOT = HERE.parent.parent
@@ -86,6 +88,23 @@ def test_truncated_compare(self) -> None:
8688
)
8789
assert len(content) < max_comment_length
8890

91+
def test_truncated_compare_stops_iterating_packages(self) -> None:
92+
"""Once the comment exceeds MAX, further packages should be skipped."""
93+
max_comment_length = 400
94+
directory = CASES_PATH / "multi_package"
95+
with patch(
96+
"pylint.testutils._primer.primer_compare_command.MAX_GITHUB_COMMENT_LENGTH",
97+
max_comment_length,
98+
):
99+
content = self.__assert_expected(
100+
directory,
101+
expected_file=directory / "expected_truncated_break.txt",
102+
)
103+
# Only the first package made it in; the second was skipped by the break.
104+
assert "astroid" in content
105+
assert "astropy" not in content
106+
assert len(content) < max_comment_length
107+
89108
def test_truncated_compare_in_details(self) -> None:
90109
"""Test for the truncation of comments that are too long inside details."""
91110
max_comment_length = 400
@@ -99,6 +118,20 @@ def test_truncated_compare_in_details(self) -> None:
99118
)
100119
assert len(content) < max_comment_length
101120

121+
def test_truncate_falls_back_when_no_space(self) -> None:
122+
"""When the pre-limit prefix has no space, cut at ``max_len - 10`` directly."""
123+
max_comment_length = 200
124+
config = argparse.Namespace(commit="v2.14.2")
125+
command = CompareCommand(PRIMER_DIRECTORY, {}, config)
126+
spaceless = "x" * 500
127+
with patch(
128+
"pylint.testutils._primer.primer_compare_command.MAX_GITHUB_COMMENT_LENGTH",
129+
max_comment_length,
130+
):
131+
truncated = command._truncate_comment(spaceless)
132+
assert "..." in truncated
133+
assert len(truncated) < max_comment_length
134+
102135
@staticmethod
103136
def __assert_expected(
104137
directory: Path,

0 commit comments

Comments
 (0)