From c056248a458330b1813c144310fbc5d4bb82a3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 24 Nov 2021 15:16:14 +0100 Subject: [PATCH] Fix checking of ``confidence`` in the unittests (#5376) * Fix checking of ``confidence`` in the unittests Co-authored-by: Pierre Sassoulas --- ChangeLog | 6 ++++ doc/whatsnew/2.12.rst | 6 ++++ pylint/testutils/output_line.py | 30 ++++------------ pylint/testutils/unittest_linter.py | 17 ++++++++-- tests/checkers/unittest_base.py | 51 ++++++++++++++++++++++------ tests/checkers/unittest_misc.py | 21 ++++++++---- tests/checkers/unittest_typecheck.py | 19 ++++++++--- tests/checkers/unittest_variables.py | 10 ++++-- 8 files changed, 110 insertions(+), 50 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9a20365af9..cffad1cf34 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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. diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 473a3ba028..24f3a9ddb4 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -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 ================ diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 9e5a071d26..faba6ae260 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -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 @@ -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__( diff --git a/pylint/testutils/unittest_linter.py b/pylint/testutils/unittest_linter.py index cb222892e8..d37f17099e 100644 --- a/pylint/testutils/unittest_linter.py +++ b/pylint/testutils/unittest_linter.py @@ -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 @@ -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): diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py index 46a9913a4e..eeef6aaf76 100644 --- a/tests/checkers/unittest_base.py +++ b/tests/checkers/unittest_base.py @@ -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 @@ -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) @@ -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) @@ -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]) @@ -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) @@ -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) @@ -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) @@ -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]) @@ -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]) @@ -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) @@ -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]) @@ -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]) @@ -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]) @@ -360,6 +384,7 @@ class CLASSC(object): #@ "classb", "the `UP` group in the '(?:(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ) with self.assertAddsMessages(message): cls = None @@ -389,6 +414,7 @@ class CLASSC(object): #@ "class_a", "'(?:(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ), MessageTest( "invalid-name", @@ -398,6 +424,7 @@ class CLASSC(object): #@ "CLASSC", "the `down` group in the '(?:(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ), ] with self.assertAddsMessages(*messages): @@ -432,6 +459,7 @@ def FUNC(): #@ "FUNC", "the `down` group in the '(?:(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ) with self.assertAddsMessages(message): func = None @@ -464,6 +492,7 @@ def UPPER(): #@ "UPPER", "the `down` group in the '(?:(?PFOO)|(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ) with self.assertAddsMessages(message): func = None diff --git a/tests/checkers/unittest_misc.py b/tests/checkers/unittest_misc.py index df639cadb4..23e19a9d0f 100644 --- a/tests/checkers/unittest_misc.py +++ b/tests/checkers/unittest_misc.py @@ -30,7 +30,7 @@ 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)) @@ -38,14 +38,18 @@ 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: @@ -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=[]) @@ -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)) @@ -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)) diff --git a/tests/checkers/unittest_typecheck.py b/tests/checkers/unittest_typecheck.py index 086f60d7fb..f378fc402b 100644 --- a/tests/checkers/unittest_typecheck.py +++ b/tests/checkers/unittest_typecheck.py @@ -29,7 +29,7 @@ import pytest from pylint.checkers import typecheck -from pylint.interfaces import UNDEFINED +from pylint.interfaces import INFERENCE, UNDEFINED from pylint.testutils import CheckerTestCase, MessageTest, set_config try: @@ -63,6 +63,7 @@ def test_no_member_in_getattr(self) -> None: "no-member", node=node, args=("Module", "optparse", "THIS_does_not_EXIST", ""), + confidence=INFERENCE, ) ): self.checker.visit_attribute(node) @@ -91,7 +92,10 @@ def test_ignored_modules_invalid_pattern(self) -> None: """ ) message = MessageTest( - "no-member", node=node, args=("Module", "xml.etree", "Lala", "") + "no-member", + node=node, + args=("Module", "xml.etree", "Lala", ""), + confidence=INFERENCE, ) with self.assertAddsMessages(message): self.checker.visit_attribute(node) @@ -128,7 +132,10 @@ def test_ignored_classes_no_recursive_pattern(self) -> None: """ ) message = MessageTest( - "no-member", node=node, args=("Module", "xml.etree.ElementTree", "Test", "") + "no-member", + node=node, + args=("Module", "xml.etree.ElementTree", "Test", ""), + confidence=INFERENCE, ) with self.assertAddsMessages(message): self.checker.visit_attribute(node) @@ -167,7 +174,10 @@ def test_nomember_on_c_extension_error_msg(self) -> None: """ ) message = MessageTest( - "no-member", node=node, args=("Module", "coverage.tracer", "CTracer", "") + "no-member", + node=node, + args=("Module", "coverage.tracer", "CTracer", ""), + confidence=INFERENCE, ) with self.assertAddsMessages(message): self.checker.visit_attribute(node) @@ -185,6 +195,7 @@ def test_nomember_on_c_extension_info_msg(self) -> None: "c-extension-no-member", node=node, args=("Module", "coverage.tracer", "CTracer", ""), + confidence=INFERENCE, ) with self.assertAddsMessages(message): self.checker.visit_attribute(node) diff --git a/tests/checkers/unittest_variables.py b/tests/checkers/unittest_variables.py index 94f524f776..9b77e17050 100644 --- a/tests/checkers/unittest_variables.py +++ b/tests/checkers/unittest_variables.py @@ -30,7 +30,7 @@ from pylint.checkers import variables from pylint.constants import IS_PYPY -from pylint.interfaces import UNDEFINED +from pylint.interfaces import HIGH, UNDEFINED from pylint.testutils import CheckerTestCase, MessageTest, linter, set_config REGR_DATA_DIR = str(Path(__file__).parent / ".." / "regrtest_data") @@ -256,7 +256,9 @@ def normal_func(abc): """ ) with self.assertAddsMessages( - MessageTest("unused-argument", node=node["abc"], args="abc") + MessageTest( + "unused-argument", node=node["abc"], args="abc", confidence=HIGH + ) ): self.checker.visit_functiondef(node) self.checker.leave_functiondef(node) @@ -268,7 +270,9 @@ def cb_func(abc): """ ) with self.assertAddsMessages( - MessageTest("unused-argument", node=node["abc"], args="abc") + MessageTest( + "unused-argument", node=node["abc"], args="abc", confidence=HIGH + ) ): self.checker.visit_functiondef(node) self.checker.leave_functiondef(node)