Add pylint checks for fixture scope (#126723)
* Prevent session scope fixtures in component tests * Link message to the decorator - not the function * Add checks for package also * Add check for session scope autouse * Rename variable * Adjust message * Ignore fancy autouse * Simplify
This commit is contained in:
parent
fb91377139
commit
083b586d19
2 changed files with 294 additions and 4 deletions
|
@ -18,14 +18,100 @@ class HassDecoratorChecker(BaseChecker):
|
||||||
"hass-async-callback-decorator",
|
"hass-async-callback-decorator",
|
||||||
"Used when a coroutine function has an invalid @callback decorator",
|
"Used when a coroutine function has an invalid @callback decorator",
|
||||||
),
|
),
|
||||||
|
"W7472": (
|
||||||
|
"Fixture %s is invalid here, please %s",
|
||||||
|
"hass-pytest-fixture-decorator",
|
||||||
|
"Used when a pytest fixture is invalid",
|
||||||
|
),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
def _get_pytest_fixture_node(self, node: nodes.FunctionDef) -> nodes.Call | None:
|
||||||
|
for decorator in node.decorators.nodes:
|
||||||
|
if (
|
||||||
|
isinstance(decorator, nodes.Call)
|
||||||
|
and decorator.func.as_string() == "pytest.fixture"
|
||||||
|
):
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
return None
|
||||||
|
|
||||||
|
def _get_pytest_fixture_node_keyword(
|
||||||
|
self, decorator: nodes.Call, search_arg: str
|
||||||
|
) -> nodes.Keyword | None:
|
||||||
|
for keyword in decorator.keywords:
|
||||||
|
if keyword.arg == search_arg:
|
||||||
|
return keyword
|
||||||
|
|
||||||
|
return None
|
||||||
|
|
||||||
|
def _check_pytest_fixture(
|
||||||
|
self, node: nodes.FunctionDef, decoratornames: set[str]
|
||||||
|
) -> None:
|
||||||
|
if (
|
||||||
|
"_pytest.fixtures.FixtureFunctionMarker" not in decoratornames
|
||||||
|
or not (root_name := node.root().name).startswith("tests.")
|
||||||
|
or (decorator := self._get_pytest_fixture_node(node)) is None
|
||||||
|
or not (
|
||||||
|
scope_keyword := self._get_pytest_fixture_node_keyword(
|
||||||
|
decorator, "scope"
|
||||||
|
)
|
||||||
|
)
|
||||||
|
or not isinstance(scope_keyword.value, nodes.Const)
|
||||||
|
or not (scope := scope_keyword.value.value)
|
||||||
|
):
|
||||||
|
return
|
||||||
|
|
||||||
|
parts = root_name.split(".")
|
||||||
|
test_component: str | None = None
|
||||||
|
if root_name.startswith("tests.components.") and parts[2] != "conftest":
|
||||||
|
test_component = parts[2]
|
||||||
|
|
||||||
|
if scope == "session":
|
||||||
|
if test_component:
|
||||||
|
self.add_message(
|
||||||
|
"hass-pytest-fixture-decorator",
|
||||||
|
node=decorator,
|
||||||
|
args=("scope `session`", "use `package` or lower"),
|
||||||
|
)
|
||||||
|
return
|
||||||
|
if not (
|
||||||
|
autouse_keyword := self._get_pytest_fixture_node_keyword(
|
||||||
|
decorator, "autouse"
|
||||||
|
)
|
||||||
|
) or (
|
||||||
|
isinstance(autouse_keyword.value, nodes.Const)
|
||||||
|
and not autouse_keyword.value.value
|
||||||
|
):
|
||||||
|
self.add_message(
|
||||||
|
"hass-pytest-fixture-decorator",
|
||||||
|
node=decorator,
|
||||||
|
args=(
|
||||||
|
"scope/autouse combination",
|
||||||
|
"set `autouse=True` or reduce scope",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
test_module = parts[3] if len(parts) > 3 else ""
|
||||||
|
|
||||||
|
if test_component and scope == "package" and test_module != "conftest":
|
||||||
|
self.add_message(
|
||||||
|
"hass-pytest-fixture-decorator",
|
||||||
|
node=decorator,
|
||||||
|
args=("scope `package`", "use `module` or lower"),
|
||||||
|
)
|
||||||
|
|
||||||
def visit_asyncfunctiondef(self, node: nodes.AsyncFunctionDef) -> None:
|
def visit_asyncfunctiondef(self, node: nodes.AsyncFunctionDef) -> None:
|
||||||
"""Apply checks on an AsyncFunctionDef node."""
|
"""Apply checks on an AsyncFunctionDef node."""
|
||||||
if (
|
if decoratornames := node.decoratornames():
|
||||||
decoratornames := node.decoratornames()
|
if "homeassistant.core.callback" in decoratornames:
|
||||||
) and "homeassistant.core.callback" in decoratornames:
|
self.add_message("hass-async-callback-decorator", node=node)
|
||||||
self.add_message("hass-async-callback-decorator", node=node)
|
self._check_pytest_fixture(node, decoratornames)
|
||||||
|
|
||||||
|
def visit_functiondef(self, node: nodes.FunctionDef) -> None:
|
||||||
|
"""Apply checks on an AsyncFunctionDef node."""
|
||||||
|
if decoratornames := node.decoratornames():
|
||||||
|
self._check_pytest_fixture(node, decoratornames)
|
||||||
|
|
||||||
|
|
||||||
def register(linter: PyLinter) -> None:
|
def register(linter: PyLinter) -> None:
|
||||||
|
|
|
@ -8,6 +8,7 @@ from pylint.interfaces import UNDEFINED
|
||||||
from pylint.testutils import MessageTest
|
from pylint.testutils import MessageTest
|
||||||
from pylint.testutils.unittest_linter import UnittestLinter
|
from pylint.testutils.unittest_linter import UnittestLinter
|
||||||
from pylint.utils.ast_walker import ASTWalker
|
from pylint.utils.ast_walker import ASTWalker
|
||||||
|
import pytest
|
||||||
|
|
||||||
from . import assert_adds_messages, assert_no_messages
|
from . import assert_adds_messages, assert_no_messages
|
||||||
|
|
||||||
|
@ -62,3 +63,206 @@ def test_bad_callback(linter: UnittestLinter, decorator_checker: BaseChecker) ->
|
||||||
),
|
),
|
||||||
):
|
):
|
||||||
walker.walk(root_node)
|
walker.walk(root_node)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("keywords", "path"),
|
||||||
|
[
|
||||||
|
('scope="function"', "tests.test_bootstrap"),
|
||||||
|
('scope="class"', "tests.test_bootstrap"),
|
||||||
|
('scope="module"', "tests.test_bootstrap"),
|
||||||
|
('scope="package"', "tests.test_bootstrap"),
|
||||||
|
('scope="session", autouse=True', "tests.test_bootstrap"),
|
||||||
|
('scope="function"', "tests.components.conftest"),
|
||||||
|
('scope="class"', "tests.components.conftest"),
|
||||||
|
('scope="module"', "tests.components.conftest"),
|
||||||
|
('scope="package"', "tests.components.conftest"),
|
||||||
|
('scope="session", autouse=True', "tests.components.conftest"),
|
||||||
|
(
|
||||||
|
'scope="session", autouse=find_spec("zeroconf") is not None',
|
||||||
|
"tests.components.conftest",
|
||||||
|
),
|
||||||
|
('scope="function"', "tests.components.pylint_tests.conftest"),
|
||||||
|
('scope="class"', "tests.components.pylint_tests.conftest"),
|
||||||
|
('scope="module"', "tests.components.pylint_tests.conftest"),
|
||||||
|
('scope="package"', "tests.components.pylint_tests.conftest"),
|
||||||
|
('scope="function"', "tests.components.pylint_test"),
|
||||||
|
('scope="class"', "tests.components.pylint_test"),
|
||||||
|
('scope="module"', "tests.components.pylint_test"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_good_fixture(
|
||||||
|
linter: UnittestLinter, decorator_checker: BaseChecker, keywords: str, path: str
|
||||||
|
) -> None:
|
||||||
|
"""Test good `@pytest.fixture` decorator."""
|
||||||
|
code = f"""
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def setup(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.fixture({keywords})
|
||||||
|
def setup_session(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
"""
|
||||||
|
|
||||||
|
root_node = astroid.parse(code, path)
|
||||||
|
walker = ASTWalker(linter)
|
||||||
|
walker.add_checker(decorator_checker)
|
||||||
|
|
||||||
|
with assert_no_messages(linter):
|
||||||
|
walker.walk(root_node)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"path",
|
||||||
|
[
|
||||||
|
"tests.components.pylint_test",
|
||||||
|
"tests.components.pylint_test.conftest",
|
||||||
|
"tests.components.pylint_test.module",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_bad_fixture_session_scope(
|
||||||
|
linter: UnittestLinter, decorator_checker: BaseChecker, path: str
|
||||||
|
) -> None:
|
||||||
|
"""Test bad `@pytest.fixture` decorator."""
|
||||||
|
code = """
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def setup(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.fixture(scope="session")
|
||||||
|
def setup_session(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
"""
|
||||||
|
|
||||||
|
root_node = astroid.parse(code, path)
|
||||||
|
walker = ASTWalker(linter)
|
||||||
|
walker.add_checker(decorator_checker)
|
||||||
|
|
||||||
|
with assert_adds_messages(
|
||||||
|
linter,
|
||||||
|
MessageTest(
|
||||||
|
msg_id="hass-pytest-fixture-decorator",
|
||||||
|
line=10,
|
||||||
|
node=root_node.body[2].decorators.nodes[0],
|
||||||
|
args=("scope `session`", "use `package` or lower"),
|
||||||
|
confidence=UNDEFINED,
|
||||||
|
col_offset=1,
|
||||||
|
end_line=10,
|
||||||
|
end_col_offset=32,
|
||||||
|
),
|
||||||
|
):
|
||||||
|
walker.walk(root_node)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"path",
|
||||||
|
[
|
||||||
|
"tests.components.pylint_test",
|
||||||
|
"tests.components.pylint_test.module",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_bad_fixture_package_scope(
|
||||||
|
linter: UnittestLinter, decorator_checker: BaseChecker, path: str
|
||||||
|
) -> None:
|
||||||
|
"""Test bad `@pytest.fixture` decorator."""
|
||||||
|
code = """
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def setup(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.fixture(scope="package")
|
||||||
|
def setup_session(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
"""
|
||||||
|
|
||||||
|
root_node = astroid.parse(code, path)
|
||||||
|
walker = ASTWalker(linter)
|
||||||
|
walker.add_checker(decorator_checker)
|
||||||
|
|
||||||
|
with assert_adds_messages(
|
||||||
|
linter,
|
||||||
|
MessageTest(
|
||||||
|
msg_id="hass-pytest-fixture-decorator",
|
||||||
|
line=10,
|
||||||
|
node=root_node.body[2].decorators.nodes[0],
|
||||||
|
args=("scope `package`", "use `module` or lower"),
|
||||||
|
confidence=UNDEFINED,
|
||||||
|
col_offset=1,
|
||||||
|
end_line=10,
|
||||||
|
end_col_offset=32,
|
||||||
|
),
|
||||||
|
):
|
||||||
|
walker.walk(root_node)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"keywords",
|
||||||
|
[
|
||||||
|
'scope="session"',
|
||||||
|
'scope="session", autouse=False',
|
||||||
|
],
|
||||||
|
)
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"path",
|
||||||
|
[
|
||||||
|
"tests.test_bootstrap",
|
||||||
|
"tests.components.conftest",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_bad_fixture_autouse(
|
||||||
|
linter: UnittestLinter, decorator_checker: BaseChecker, keywords: str, path: str
|
||||||
|
) -> None:
|
||||||
|
"""Test bad `@pytest.fixture` decorator."""
|
||||||
|
code = f"""
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def setup(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.fixture({keywords})
|
||||||
|
def setup_session(
|
||||||
|
arg1, arg2
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
"""
|
||||||
|
|
||||||
|
root_node = astroid.parse(code, path)
|
||||||
|
walker = ASTWalker(linter)
|
||||||
|
walker.add_checker(decorator_checker)
|
||||||
|
|
||||||
|
with assert_adds_messages(
|
||||||
|
linter,
|
||||||
|
MessageTest(
|
||||||
|
msg_id="hass-pytest-fixture-decorator",
|
||||||
|
line=10,
|
||||||
|
node=root_node.body[2].decorators.nodes[0],
|
||||||
|
args=("scope/autouse combination", "set `autouse=True` or reduce scope"),
|
||||||
|
confidence=UNDEFINED,
|
||||||
|
col_offset=1,
|
||||||
|
end_line=10,
|
||||||
|
end_col_offset=17 + len(keywords),
|
||||||
|
),
|
||||||
|
):
|
||||||
|
walker.walk(root_node)
|
||||||
|
|
Loading…
Add table
Reference in a new issue