From f6939ccea9ac7beaf50d006c4965f3df37ab7bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 22:31:10 +0100 Subject: [PATCH 1/6] Fix checking of ``confidence`` in the unittests --- pylint/testutils/output_line.py | 30 ++++++------------------ pylint/testutils/unittest_linter.py | 17 +++++++++++--- tests/checkers/unittest_base.py | 35 +++++++++++++++++++++------- tests/checkers/unittest_misc.py | 21 ++++++++++++----- tests/checkers/unittest_typecheck.py | 19 +++++++++++---- tests/checkers/unittest_variables.py | 10 +++++--- 6 files changed, 84 insertions(+), 48 deletions(-) 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 2d9e42e657..96b887529b 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 @@ -35,8 +35,19 @@ def add_message( confidence: Optional[Confidence] = None, col_offset: Optional[int] = None, ) -> None: - # Do not test col_offset for now since changing Message breaks everything - self._messages.append(MessageTest(msg_id, line, node, args, confidence)) + """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 + 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..a2c6117e70 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]) @@ -360,6 +372,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 +402,7 @@ class CLASSC(object): #@ "class_a", "'(?:(?P[A-Z]+)|(?P[a-z]+))$' pattern", ), + confidence=HIGH, ), MessageTest( "invalid-name", @@ -398,6 +412,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 +447,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 +480,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) From a93ddd8ef97c69d99870c69f0bf503646ebaedcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:29:10 +0100 Subject: [PATCH 2/6] Add changelog --- ChangeLog | 6 ++++++ doc/whatsnew/2.12.rst | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/ChangeLog b/ChangeLog index 10adc33792..e7b9b14eaa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,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 ``simplify-boolean-expression`` when condition can be inferred as False. Closes #5200 diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 9e8bfbb30a..73408f6500 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 ================ From f4ff1cabd8689e8c57487011382720a250d1e029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:29:42 +0100 Subject: [PATCH 3/6] Fix test --- tests/checkers/unittest_base.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py index a2c6117e70..62d7151356 100644 --- a/tests/checkers/unittest_base.py +++ b/tests/checkers/unittest_base.py @@ -35,7 +35,7 @@ import astroid from pylint.checkers import base -from pylint.interfaces import HIGH, INFERENCE +from pylint.interfaces import HIGH, INFERENCE, UNDEFINED from pylint.testutils import CheckerTestCase, MessageTest, set_config @@ -322,6 +322,7 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[0].targets[0], args=("async", "3.7"), + confidence=UNDEFINED, ) ): self.checker.visit_assignname(ast[0].targets[0]) @@ -330,18 +331,25 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[1].targets[0], args=("await", "3.7"), + confidence=UNDEFINED, ) ): 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=UNDEFINED, ) ): 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=UNDEFINED, ) ): self.checker.visit_classdef(ast[3]) From 1d3ccd71515e3b025ab31e68532d0042e05151c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:51:06 +0100 Subject: [PATCH 4/6] Retry test --- tests/checkers/unittest_base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py index 62d7151356..70661d29a3 100644 --- a/tests/checkers/unittest_base.py +++ b/tests/checkers/unittest_base.py @@ -323,6 +323,7 @@ class async: #@ node=ast[0].targets[0], args=("async", "3.7"), confidence=UNDEFINED, + col_offset=None, ) ): self.checker.visit_assignname(ast[0].targets[0]) From 5d6a8f9298f6a13ec23d16a756a7ed39f6e761f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 00:14:43 +0100 Subject: [PATCH 5/6] Really not having my day with these tests --- tests/checkers/unittest_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py index 70661d29a3..bcf3e92e4e 100644 --- a/tests/checkers/unittest_base.py +++ b/tests/checkers/unittest_base.py @@ -322,7 +322,7 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[0].targets[0], args=("async", "3.7"), - confidence=UNDEFINED, + confidence=HIGH, col_offset=None, ) ): From 536efc7b8f62723ae70dc25835fa8edb99bc366b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 00:30:30 +0100 Subject: [PATCH 6/6] Third try is a charm? --- tests/checkers/unittest_base.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py index bcf3e92e4e..eeef6aaf76 100644 --- a/tests/checkers/unittest_base.py +++ b/tests/checkers/unittest_base.py @@ -35,7 +35,7 @@ import astroid from pylint.checkers import base -from pylint.interfaces import HIGH, INFERENCE, UNDEFINED +from pylint.interfaces import HIGH, INFERENCE from pylint.testutils import CheckerTestCase, MessageTest, set_config @@ -332,7 +332,8 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[1].targets[0], args=("await", "3.7"), - confidence=UNDEFINED, + confidence=HIGH, + col_offset=None, ) ): self.checker.visit_assignname(ast[1].targets[0]) @@ -341,7 +342,8 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[2], args=("async", "3.7"), - confidence=UNDEFINED, + confidence=HIGH, + col_offset=None, ) ): self.checker.visit_functiondef(ast[2]) @@ -350,7 +352,8 @@ class async: #@ msg_id="assign-to-new-keyword", node=ast[3], args=("async", "3.7"), - confidence=UNDEFINED, + confidence=HIGH, + col_offset=None, ) ): self.checker.visit_classdef(ast[3])