mirror of
https://github.com/home-assistant/core.git
synced 2026-09-03 03:51:51 +01:00
Add pylint checker for unused test fixture arguments (#170537)
This commit is contained in:
@@ -98,6 +98,7 @@ Every check has a code following the
|
||||
| `W7406` | [`home-assistant-unique-id-ip-based`](#w7406-home-assistant-unique-id-ip-based) | Unique ID should not be based on IP/hostname |
|
||||
| `W7407` | [`home-assistant-config-flow-polling-field`](#w7407-home-assistant-config-flow-polling-field) | Config flow should not include polling interval fields |
|
||||
| `W7408` | [`home-assistant-config-flow-name-field`](#w7408-home-assistant-config-flow-name-field) | Config flow should not include name fields |
|
||||
| `R7402` | [`home-assistant-unused-test-fixture-argument`](#r7402-home-assistant-unused-test-fixture-argument) | Unused test function argument should use `@pytest.mark.usefixtures` |
|
||||
|
||||
|
||||
## `home_assistant_logger` checker
|
||||
@@ -324,3 +325,18 @@ Config flow should not include a name field. Users should not set names
|
||||
in config flows; they come automatically from the device or are set by
|
||||
the integration.
|
||||
|
||||
|
||||
## `home_assistant_unused_test_fixture_args` checker
|
||||
|
||||
**Disabled by default** while existing violations are being cleaned up.
|
||||
|
||||
|
||||
### `R7402`: `home-assistant-unused-test-fixture-argument`
|
||||
|
||||
Test functions that receive a fixture argument but never reference it in
|
||||
the function body should use `@pytest.mark.usefixtures("name")` instead.
|
||||
This keeps the function signature clean and makes it clear the fixture is
|
||||
only needed for its side effects.
|
||||
|
||||
This rule only applies to `test_*` functions, not to fixture functions.
|
||||
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
"""Checker for unused fixture arguments in test functions.
|
||||
|
||||
Test functions that receive a fixture argument but never reference it in the
|
||||
function body should use ``@pytest.mark.usefixtures("name")`` instead. This
|
||||
keeps the function signature clean and makes it clear the fixture is only
|
||||
needed for its side effects.
|
||||
|
||||
This rule only applies to ``test_*`` functions, not to fixture functions.
|
||||
"""
|
||||
|
||||
from astroid import nodes
|
||||
from pylint.checkers import BaseChecker
|
||||
from pylint.lint import PyLinter
|
||||
|
||||
from pylint_home_assistant.helpers.module_info import is_test_module
|
||||
|
||||
|
||||
class UnusedTestFixtureArgsChecker(BaseChecker):
|
||||
"""Checker for unused fixture arguments in test functions."""
|
||||
|
||||
name = "home_assistant_unused_test_fixture_args"
|
||||
priority = -1
|
||||
msgs = {
|
||||
"R7402": (
|
||||
"Argument '%s' is not used in %s, use "
|
||||
'`@pytest.mark.usefixtures("%s")` instead',
|
||||
"home-assistant-unused-test-fixture-argument",
|
||||
"Used when a test function has a fixture argument that is never "
|
||||
"referenced in the function body. Use @pytest.mark.usefixtures "
|
||||
"to declare the dependency instead.",
|
||||
),
|
||||
}
|
||||
options = ()
|
||||
|
||||
_in_test_module: bool
|
||||
|
||||
def visit_module(self, node: nodes.Module) -> None:
|
||||
"""Track whether we are in a test module."""
|
||||
self._in_test_module = is_test_module(node.name)
|
||||
|
||||
def visit_functiondef(self, node: nodes.FunctionDef) -> None:
|
||||
"""Check test functions for unused fixture arguments."""
|
||||
if not self._in_test_module:
|
||||
return
|
||||
|
||||
if not node.name.startswith("test_"):
|
||||
return
|
||||
|
||||
# Only check top-level test functions (not nested)
|
||||
if not isinstance(node.parent, nodes.Module):
|
||||
return
|
||||
|
||||
# Collect all argument names (skip 'self' for methods)
|
||||
arg_names = {arg.name for arg in node.args.args if arg.name != "self"}
|
||||
|
||||
if not arg_names:
|
||||
return
|
||||
|
||||
# Collect all Name references in the function body
|
||||
used_names: set[str] = set()
|
||||
for child in node.nodes_of_class(nodes.Name):
|
||||
used_names.add(child.name)
|
||||
|
||||
for arg_name in sorted(arg_names - used_names):
|
||||
arg_node = next(arg for arg in node.args.args if arg.name == arg_name)
|
||||
self.add_message(
|
||||
"home-assistant-unused-test-fixture-argument",
|
||||
node=arg_node,
|
||||
args=(arg_name, node.name, arg_name),
|
||||
)
|
||||
|
||||
visit_asyncfunctiondef = visit_functiondef
|
||||
|
||||
|
||||
def register(linter: PyLinter) -> None:
|
||||
"""Register the checker."""
|
||||
linter.register_checker(UnusedTestFixtureArgsChecker(linter))
|
||||
@@ -176,6 +176,9 @@ disable = [
|
||||
"consider-using-assignment-expr",
|
||||
"possibly-used-before-assignment",
|
||||
|
||||
# Disabled while existing violations are being cleaned up
|
||||
"home-assistant-unused-test-fixture-argument",
|
||||
|
||||
# Handled by ruff
|
||||
# Ref: <https://github.com/astral-sh/ruff/issues/970>
|
||||
"await-outside-async", # PLE1142
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
"""Tests for the unused test fixture arguments checker."""
|
||||
|
||||
import astroid
|
||||
from pylint.testutils import UnittestLinter
|
||||
from pylint.utils.ast_walker import ASTWalker
|
||||
from pylint_home_assistant.checkers.unused_test_fixture_args import (
|
||||
UnusedTestFixtureArgsChecker,
|
||||
)
|
||||
import pytest
|
||||
|
||||
from . import assert_no_messages
|
||||
|
||||
|
||||
@pytest.fixture(name="unused_args_checker")
|
||||
def unused_args_checker_fixture(
|
||||
linter: UnittestLinter,
|
||||
) -> UnusedTestFixtureArgsChecker:
|
||||
"""Fixture to provide an unused test fixture args checker."""
|
||||
return UnusedTestFixtureArgsChecker(linter)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"code",
|
||||
[
|
||||
pytest.param(
|
||||
"""
|
||||
def test_something(hass: HomeAssistant) -> None:
|
||||
assert hass.state
|
||||
""",
|
||||
id="all_args_used",
|
||||
),
|
||||
pytest.param(
|
||||
"""
|
||||
def test_something(hass: HomeAssistant, config_entry: MockConfigEntry) -> None:
|
||||
hass.states.async_set("sensor.test", "on")
|
||||
config_entry.add_to_hass(hass)
|
||||
""",
|
||||
id="multiple_args_all_used",
|
||||
),
|
||||
pytest.param(
|
||||
"""
|
||||
@pytest.fixture
|
||||
def my_fixture(enable_bluetooth: None) -> None:
|
||||
pass
|
||||
""",
|
||||
id="fixture_function_ignored",
|
||||
),
|
||||
pytest.param(
|
||||
"""
|
||||
def helper_function(unused_arg: str) -> None:
|
||||
pass
|
||||
""",
|
||||
id="non_test_function_ignored",
|
||||
),
|
||||
pytest.param(
|
||||
"""
|
||||
def test_empty() -> None:
|
||||
pass
|
||||
""",
|
||||
id="no_args",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_no_warning(
|
||||
linter: UnittestLinter,
|
||||
unused_args_checker: UnusedTestFixtureArgsChecker,
|
||||
code: str,
|
||||
) -> None:
|
||||
"""Test cases that should not trigger a warning."""
|
||||
root_node = astroid.parse(code, "tests.components.test_integration.test_init")
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(unused_args_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
def test_unused_single_arg(
|
||||
linter: UnittestLinter,
|
||||
unused_args_checker: UnusedTestFixtureArgsChecker,
|
||||
) -> None:
|
||||
"""Test that unused fixture arg is flagged."""
|
||||
root_node = astroid.parse(
|
||||
"""
|
||||
def test_something(hass: HomeAssistant, enable_bluetooth: None) -> None:
|
||||
assert hass.state
|
||||
""",
|
||||
"tests.components.test_integration.test_init",
|
||||
)
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(unused_args_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 1
|
||||
assert messages[0].msg_id == "home-assistant-unused-test-fixture-argument"
|
||||
assert messages[0].args == (
|
||||
"enable_bluetooth",
|
||||
"test_something",
|
||||
"enable_bluetooth",
|
||||
)
|
||||
|
||||
|
||||
def test_unused_multiple_args(
|
||||
linter: UnittestLinter,
|
||||
unused_args_checker: UnusedTestFixtureArgsChecker,
|
||||
) -> None:
|
||||
"""Test that multiple unused fixture args are all flagged."""
|
||||
root_node = astroid.parse(
|
||||
"""
|
||||
def test_something(hass: HomeAssistant, enable_bluetooth: None, socket_enabled: None) -> None:
|
||||
assert hass.state
|
||||
""",
|
||||
"tests.components.test_integration.test_init",
|
||||
)
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(unused_args_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 2
|
||||
assert messages[0].args[0] == "enable_bluetooth"
|
||||
assert messages[1].args[0] == "socket_enabled"
|
||||
|
||||
|
||||
def test_not_test_module(
|
||||
linter: UnittestLinter,
|
||||
unused_args_checker: UnusedTestFixtureArgsChecker,
|
||||
) -> None:
|
||||
"""Test that non-test modules are ignored."""
|
||||
root_node = astroid.parse(
|
||||
"""
|
||||
def test_something(unused: str) -> None:
|
||||
pass
|
||||
""",
|
||||
"homeassistant.components.test_integration",
|
||||
)
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(unused_args_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
def test_async_test_function(
|
||||
linter: UnittestLinter,
|
||||
unused_args_checker: UnusedTestFixtureArgsChecker,
|
||||
) -> None:
|
||||
"""Test that async test functions are also checked."""
|
||||
root_node = astroid.parse(
|
||||
"""
|
||||
async def test_something(hass: HomeAssistant, enable_bluetooth: None) -> None:
|
||||
assert hass.state
|
||||
""",
|
||||
"tests.components.test_integration.test_init",
|
||||
)
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(unused_args_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 1
|
||||
assert messages[0].args[0] == "enable_bluetooth"
|
||||
Reference in New Issue
Block a user