Skip to content

Commit

Permalink
Fix checking of confidence in the unittests (#5376)
Browse files Browse the repository at this point in the history
* Fix checking of ``confidence`` in the unittests

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
  • Loading branch information
DanielNoord and Pierre-Sassoulas committed Nov 24, 2021
1 parent 7c3533c commit c056248
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 50 deletions.
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ Release date: TBA

* Fix ``install graphiz`` message which isn't needed for puml output format.

* ``MessageTest`` of the unittest ``testutil`` now requires the ``confidence`` attribute
to match the expected value. If none is provided it is set to ``UNDEFINED``.

* ``add_message`` of the unittest ``testutil`` now actually handles the ``col_offset`` parameter
and allows it to be checked against actual output in a test.

* Fix a crash in the ``check_elif`` extensions where an undetected if in a comprehension
with an if statement within a f-string resulted in an out of range error. The checker no
longer relies on counting if statements anymore and uses known if statements locations instead.
Expand Down
6 changes: 6 additions & 0 deletions doc/whatsnew/2.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ New checkers

Follow-up in #5259

* ``MessageTest`` of the unittest ``testutil`` now requires the ``confidence`` attribute
to match the expected value. If none is provided it is set to ``UNDEFINED``.

* ``add_message`` of the unittest ``testutil`` now actually handles the ``col_offset`` parameter
and allows it to be checked against actual output in a test.

Removed checkers
================

Expand Down
30 changes: 7 additions & 23 deletions pylint/testutils/output_line.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE

import collections
from typing import Any, NamedTuple, Optional, Sequence, Tuple, Union

from astroid import nodes
Expand All @@ -12,30 +11,15 @@
from pylint.testutils.constants import UPDATE_OPTION


class MessageTest(
collections.namedtuple(
"MessageTest", ["msg_id", "line", "node", "args", "confidence"]
)
):
class MessageTest(NamedTuple):
msg_id: str
line: Optional[int] = None
node: Optional[nodes.NodeNG] = None
args: Optional[Any] = None
confidence: Optional[Confidence] = UNDEFINED
col_offset: Optional[int] = None
"""Used to test messages produced by pylint. Class name cannot start with Test as pytest doesn't allow constructors in test classes."""

def __new__(
cls,
msg_id: str,
line: Optional[int] = None,
node: Optional[nodes.NodeNG] = None,
args: Any = None,
confidence: Optional[Confidence] = None,
) -> "MessageTest":
return tuple.__new__(cls, (msg_id, line, node, args, confidence))

def __eq__(self, other: object) -> bool:
if isinstance(other, MessageTest):
if self.confidence and other.confidence:
return super().__eq__(other)
return tuple(self[:-1]) == tuple(other[:-1])
return NotImplemented # pragma: no cover


class MalformedOutputLineException(Exception):
def __init__(
Expand Down
17 changes: 14 additions & 3 deletions pylint/testutils/unittest_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from astroid import nodes

from pylint.interfaces import Confidence
from pylint.interfaces import UNDEFINED, Confidence
from pylint.testutils.global_test_linter import linter
from pylint.testutils.output_line import MessageTest
from pylint.utils import LinterStats
Expand Down Expand Up @@ -37,10 +37,21 @@ def add_message(
end_lineno: Optional[int] = None,
end_col_offset: Optional[int] = None,
) -> None:
# Do not test col_offset for now since changing Message breaks everything
"""Add a MessageTest to the _messages attribute of the linter class."""
# If confidence is None we set it to UNDEFINED as well in PyLinter
if confidence is None:
confidence = UNDEFINED
# pylint: disable=fixme
# TODO: Test col_offset
# pylint: disable=fixme
# TODO: Initialize col_offset on every node (can be None) -> astroid
# if col_offset is None and hasattr(node, "col_offset"):
# col_offset = node.col_offset
# pylint: disable=fixme
# TODO: Test end_lineno and end_col_offset :)
self._messages.append(MessageTest(msg_id, line, node, args, confidence))
self._messages.append(
MessageTest(msg_id, line, node, args, confidence, col_offset)
)

@staticmethod
def is_message_enabled(*unused_args, **unused_kwargs):
Expand Down
51 changes: 40 additions & 11 deletions tests/checkers/unittest_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import astroid

from pylint.checkers import base
from pylint.interfaces import HIGH, INFERENCE
from pylint.testutils import CheckerTestCase, MessageTest, set_config


Expand All @@ -43,7 +44,7 @@ class TestDocstring(CheckerTestCase):

def test_missing_docstring_module(self) -> None:
module = astroid.parse("something")
message = MessageTest("missing-module-docstring", node=module)
message = MessageTest("missing-module-docstring", node=module, confidence=HIGH)
with self.assertAddsMessages(message):
self.checker.visit_module(module)

Expand All @@ -54,7 +55,9 @@ def test_missing_docstring_empty_module(self) -> None:

def test_empty_docstring_module(self) -> None:
module = astroid.parse("''''''")
message = MessageTest("empty-docstring", node=module, args=("module",))
message = MessageTest(
"empty-docstring", node=module, args=("module",), confidence=HIGH
)
with self.assertAddsMessages(message):
self.checker.visit_module(module)

Expand All @@ -69,7 +72,9 @@ def __init__(self, my_param: int) -> None:
'''
"""
)
message = MessageTest("empty-docstring", node=node.body[0], args=("method",))
message = MessageTest(
"empty-docstring", node=node.body[0], args=("method",), confidence=INFERENCE
)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(node.body[0])

Expand All @@ -79,7 +84,7 @@ def test_empty_docstring_function(self) -> None:
def func(tion):
pass"""
)
message = MessageTest("missing-function-docstring", node=func)
message = MessageTest("missing-function-docstring", node=func, confidence=HIGH)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(func)

Expand All @@ -102,7 +107,7 @@ def func(tion):
pass
"""
)
message = MessageTest("missing-function-docstring", node=func)
message = MessageTest("missing-function-docstring", node=func, confidence=HIGH)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(func)

Expand All @@ -117,7 +122,7 @@ def func(tion):
pass
"""
)
message = MessageTest("missing-function-docstring", node=func)
message = MessageTest("missing-function-docstring", node=func, confidence=HIGH)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(func)

Expand All @@ -141,7 +146,9 @@ def __init__(self, my_param: int) -> None:
pass
"""
)
message = MessageTest("missing-function-docstring", node=node.body[0])
message = MessageTest(
"missing-function-docstring", node=node.body[0], confidence=INFERENCE
)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(node.body[0])

Expand All @@ -158,7 +165,11 @@ def __eq__(self, other):
return True
"""
)
message = MessageTest("missing-function-docstring", node=module.body[1].body[0])
message = MessageTest(
"missing-function-docstring",
node=module.body[1].body[0],
confidence=INFERENCE,
)
with self.assertAddsMessages(message):
self.checker.visit_functiondef(module.body[1].body[0])

Expand All @@ -168,7 +179,7 @@ def test_class_no_docstring(self) -> None:
class Klass(object):
pass"""
)
message = MessageTest("missing-class-docstring", node=klass)
message = MessageTest("missing-class-docstring", node=klass, confidence=HIGH)
with self.assertAddsMessages(message):
self.checker.visit_classdef(klass)

Expand Down Expand Up @@ -232,6 +243,7 @@ def QUX(self): #@
"invalid-name",
node=methods[1],
args=("Attribute", "bar", "'[A-Z]+' pattern"),
confidence=INFERENCE,
)
):
self.checker.visit_functiondef(methods[1])
Expand Down Expand Up @@ -310,6 +322,8 @@ class async: #@
msg_id="assign-to-new-keyword",
node=ast[0].targets[0],
args=("async", "3.7"),
confidence=HIGH,
col_offset=None,
)
):
self.checker.visit_assignname(ast[0].targets[0])
Expand All @@ -318,18 +332,28 @@ class async: #@
msg_id="assign-to-new-keyword",
node=ast[1].targets[0],
args=("await", "3.7"),
confidence=HIGH,
col_offset=None,
)
):
self.checker.visit_assignname(ast[1].targets[0])
with self.assertAddsMessages(
MessageTest(
msg_id="assign-to-new-keyword", node=ast[2], args=("async", "3.7")
msg_id="assign-to-new-keyword",
node=ast[2],
args=("async", "3.7"),
confidence=HIGH,
col_offset=None,
)
):
self.checker.visit_functiondef(ast[2])
with self.assertAddsMessages(
MessageTest(
msg_id="assign-to-new-keyword", node=ast[3], args=("async", "3.7")
msg_id="assign-to-new-keyword",
node=ast[3],
args=("async", "3.7"),
confidence=HIGH,
col_offset=None,
)
):
self.checker.visit_classdef(ast[3])
Expand Down Expand Up @@ -360,6 +384,7 @@ class CLASSC(object): #@
"classb",
"the `UP` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
)
with self.assertAddsMessages(message):
cls = None
Expand Down Expand Up @@ -389,6 +414,7 @@ class CLASSC(object): #@
"class_a",
"'(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
),
MessageTest(
"invalid-name",
Expand All @@ -398,6 +424,7 @@ class CLASSC(object): #@
"CLASSC",
"the `down` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
),
]
with self.assertAddsMessages(*messages):
Expand Down Expand Up @@ -432,6 +459,7 @@ def FUNC(): #@
"FUNC",
"the `down` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
)
with self.assertAddsMessages(message):
func = None
Expand Down Expand Up @@ -464,6 +492,7 @@ def UPPER(): #@
"UPPER",
"the `down` group in the '(?:(?P<ignore>FOO)|(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
)
with self.assertAddsMessages(message):
func = None
Expand Down
21 changes: 15 additions & 6 deletions tests/checkers/unittest_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,26 @@ def test_fixme_with_message(self) -> None:
# FIXME message
"""
with self.assertAddsMessages(
MessageTest(msg_id="fixme", line=2, args="FIXME message")
MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=17)
):
self.checker.process_tokens(_tokenize_str(code))

def test_todo_without_message(self) -> None:
code = """a = 1
# TODO
"""
with self.assertAddsMessages(MessageTest(msg_id="fixme", line=2, args="TODO")):
with self.assertAddsMessages(
MessageTest(msg_id="fixme", line=2, args="TODO", col_offset=17)
):
self.checker.process_tokens(_tokenize_str(code))

def test_xxx_without_space(self) -> None:
code = """a = 1
#XXX
"""
with self.assertAddsMessages(MessageTest(msg_id="fixme", line=2, args="XXX")):
with self.assertAddsMessages(
MessageTest(msg_id="fixme", line=2, args="XXX", col_offset=17)
):
self.checker.process_tokens(_tokenize_str(code))

def test_xxx_middle(self) -> None:
Expand All @@ -59,7 +63,9 @@ def test_without_space_fixme(self) -> None:
code = """a = 1
#FIXME
"""
with self.assertAddsMessages(MessageTest(msg_id="fixme", line=2, args="FIXME")):
with self.assertAddsMessages(
MessageTest(msg_id="fixme", line=2, args="FIXME", col_offset=17)
):
self.checker.process_tokens(_tokenize_str(code))

@set_config(notes=[])
Expand All @@ -79,7 +85,7 @@ def test_other_present_codetag(self) -> None:
# FIXME
"""
with self.assertAddsMessages(
MessageTest(msg_id="fixme", line=2, args="CODETAG")
MessageTest(msg_id="fixme", line=2, args="CODETAG", col_offset=17)
):
self.checker.process_tokens(_tokenize_str(code))

Expand All @@ -92,7 +98,10 @@ def test_issue_2321_should_trigger(self) -> None:
code = "# TODO this should not trigger a fixme"
with self.assertAddsMessages(
MessageTest(
msg_id="fixme", line=1, args="TODO this should not trigger a fixme"
msg_id="fixme",
line=1,
args="TODO this should not trigger a fixme",
col_offset=1,
)
):
self.checker.process_tokens(_tokenize_str(code))
Expand Down

0 comments on commit c056248

Please sign in to comment.