Skip to content

Fix checking of confidence in the unittests #5376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 My goodness
feelgoodman

But is this already possible in python 3.6 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Or at least it should. The default value only from 3.6.1 but we are already depending on that in other PRs for 2.12 as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, the change was planned for 2.12.1. Would have to look up why though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think because of the default value of NamedTuple. However, the end_line PR also used those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. With the exception in place, someone with 3.6.0 won't even get that far. pip install pylint will fail. It's probably an idea to pick up most of #5068 then too, except for bumping the python_requires version and removing the exception.

https://github.com/PyCQA/pylint/blob/9e32192fe7bf77281d0dfbc107984b751983e113/setup.py#L17-L18

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 @@ -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):
Expand Down
35 changes: 26 additions & 9 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 @@ -360,6 +372,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 +402,7 @@ class CLASSC(object): #@
"class_a",
"'(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
confidence=HIGH,
),
MessageTest(
"invalid-name",
Expand All @@ -398,6 +412,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 +447,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 +480,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
19 changes: 15 additions & 4 deletions tests/checkers/unittest_typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions tests/checkers/unittest_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down