Skip to content
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

Tests Version Constraints #75

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ exclude =
__pycache__,
.venv,
tests/inputs
var
7 changes: 3 additions & 4 deletions pylint_nautobot/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Initialization file for library."""

from importlib import metadata

from packaging.specifiers import SpecifierSet
from pylint.lint import PyLinter

from .code_location_changes import NautobotCodeLocationChangesChecker
Expand All @@ -11,7 +11,7 @@
from .replaced_models import NautobotReplacedModelsImportChecker
from .string_field_blank_null import NautobotStringFieldBlankNull
from .use_fields_all import NautobotUseFieldsAllChecker
from .utils import MINIMUM_NAUTOBOT_VERSION
from .utils import is_version_compatible

__version__ = metadata.version(__name__)

Expand All @@ -29,6 +29,5 @@
def register(linter: PyLinter):
"""Pylint plugin entrypoint - register all the checks to the linter."""
for checker in CHECKERS:
checker_versions = SpecifierSet(checker.version_specifier)
if not checker_versions or MINIMUM_NAUTOBOT_VERSION in checker_versions:
if is_version_compatible(checker.version_specifier):
linter.register_checker(checker(linter))
36 changes: 9 additions & 27 deletions pylint_nautobot/incorrect_base_class.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,9 @@
"""Check for imports whose paths have changed in 2.0."""
from astroid import Assign
from astroid import ClassDef
from astroid import Const
from pylint.checkers import BaseChecker

from pylint_nautobot.utils import is_nautobot_v2_installed

from pylint.checkers import BaseChecker

def is_abstract(node):
"""Given a node, returns whether it is an abstract base model."""
for child_node in node.get_children():
if not (isinstance(child_node, ClassDef) and child_node.name == "Meta"):
continue
for meta_child in child_node.get_children():
if (
not isinstance(meta_child, Assign)
or not meta_child.targets[0].name == "abstract"
or not isinstance(meta_child.value, Const)
):
continue
# At this point we know we are dealing with an assignment to a constant for the 'abstract' field on the
# 'Meta' class. Therefore, we can assume the value of that to be whether the node is an abstract base model
# or not.
return meta_child.value.value
return False
from .utils import is_abstract_class
from .utils import is_version_compatible


class NautobotIncorrectBaseClassChecker(BaseChecker):
Expand All @@ -41,9 +21,11 @@ class NautobotIncorrectBaseClassChecker(BaseChecker):
("django.db.models.base.Model", "nautobot.core.models.BaseModel"),
(
"django.forms.forms.Form",
"nautobot.core.forms.forms.BootstrapMixin"
if is_nautobot_v2_installed()
else "nautobot.utilities.forms.forms.BootstrapMixin",
(
"nautobot.core.forms.forms.BootstrapMixin"
if is_version_compatible(">=2")
else "nautobot.utilities.forms.forms.BootstrapMixin"
),
),
]

Expand All @@ -58,7 +40,7 @@ class NautobotIncorrectBaseClassChecker(BaseChecker):

def visit_classdef(self, node):
"""Visit class definitions."""
if is_abstract(node):
if is_abstract_class(node):
return

# Skip mixin classes
Expand Down
13 changes: 10 additions & 3 deletions pylint_nautobot/use_fields_all.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Check for CharField's or TextField's on models where null=True and blank=True."""

from astroid import Assign
from astroid import AssignName
from astroid import ClassDef
Expand All @@ -15,8 +16,6 @@
"nautobot.utilities.tables.BaseTable.Meta": ">1,<2",
}

_CHECK_CLASSES = [key for key, specifier_set in _META_CLASSES.items() if is_version_compatible(specifier_set)]


class NautobotUseFieldsAllChecker(BaseChecker):
"""Visit Meta subclasses and check for use of `fields = "__all__"`, instead of `fields = ["field1", ...]`."""
Expand All @@ -34,9 +33,17 @@ class NautobotUseFieldsAllChecker(BaseChecker):
),
}

def __init__(self, *args, **kwargs):
"""Initialize the checker."""
super().__init__(*args, **kwargs)

self.meta_classes = [
key for key, specifier_set in _META_CLASSES.items() if is_version_compatible(specifier_set)
]

def visit_classdef(self, node: ClassDef):
"""Visit class definitions."""
if not any(ancestor.qname() in _CHECK_CLASSES for ancestor in node.ancestors()):
if not any(ancestor.qname() in self.meta_classes for ancestor in node.ancestors()):
return

for child_node in node.get_children():
Expand Down
129 changes: 104 additions & 25 deletions pylint_nautobot/utils.py
Original file line number Diff line number Diff line change
@@ -1,40 +1,23 @@
"""Utilities for managing data."""

from importlib import metadata
from pathlib import Path
from typing import List
from typing import Optional
from typing import Union

import toml
from astroid import Assign
from astroid import Attribute
from astroid import ClassDef
from astroid import Const
from astroid import Name
from importlib_resources import files
from packaging.specifiers import SpecifierSet
from packaging.version import Version
from yaml import safe_load


def is_nautobot_v2_installed() -> bool:
"""Return True if Nautobot v2.x is installed."""
return MINIMUM_NAUTOBOT_VERSION.major == 2


def is_version_compatible(specifier_set: Union[str, SpecifierSet]) -> bool:
"""Return True if the Nautobot version is compatible with the given version specifier_set."""
if isinstance(specifier_set, str):
specifier_set = SpecifierSet(specifier_set)
return specifier_set.contains(MINIMUM_NAUTOBOT_VERSION)


def load_v2_code_location_changes():
"""Black magic data transform, needs schema badly."""
with open(files("pylint_nautobot") / "data" / "v2" / "v2-code-location-changes.yaml", encoding="utf-8") as rules:
changes = safe_load(rules)
changes_map = {}
for change in changes:
if change["Old Module"] not in changes_map:
changes_map[change["Old Module"]] = {}
changes_map[change["Old Module"]][change["Class/Function(s)"]] = change["New Module"]
return changes_map


def _read_poetry_lock() -> dict:
for directory in (Path.cwd(), *Path.cwd().parents):
path = directory / "poetry.lock"
Expand All @@ -56,5 +39,101 @@ def _read_locked_nautobot_version() -> Optional[str]:
return None


MAP_CODE_LOCATION_CHANGES = load_v2_code_location_changes()
MINIMUM_NAUTOBOT_VERSION = Version(_read_locked_nautobot_version() or metadata.version("nautobot"))


def is_abstract_class(node: ClassDef) -> bool:
"""Given a node, returns whether it is an abstract base model."""
for child_node in node.get_children():
if not (isinstance(child_node, ClassDef) and child_node.name == "Meta"):
continue

for meta_child in child_node.get_children():
if (
not isinstance(meta_child, Assign)
or not meta_child.targets[0].name == "abstract" # type: ignore
or not isinstance(meta_child.value, Const)
):
continue
# At this point we know we are dealing with an assignment to a constant for the 'abstract' field on the
# 'Meta' class. Therefore, we can assume the value of that to be whether the node is an abstract base model
# or not.
return meta_child.value.value

return False


def get_model_name(ancestor: str, node: ClassDef) -> str:
"""Get the model name from the class definition."""
if ancestor == "from nautobot.apps.views.NautobotUIViewSet":
raise NotImplementedError("This ancestor is not yet supported.")

meta = next((n for n in node.body if isinstance(n, ClassDef) and n.name == "Meta"), None)
if not meta:
raise NotImplementedError("This class does not have a Meta class.")

model_attr = next(
(
attr
for attr in meta.body
if isinstance(attr, Assign)
and any(
isinstance(target, (Name, Attribute))
and getattr(target, "attrname", None) == "model"
or getattr(target, "name", None) == "model"
for target in attr.targets
)
),
None,
)
if not model_attr:
raise NotImplementedError("The Meta class does not define a model attribute.")

if isinstance(model_attr.value, Name):
return model_attr.value.name
if not isinstance(model_attr.value, Attribute):
raise NotImplementedError("This utility supports only direct assignment or attribute based model names.")

model_attr_chain = []
while isinstance(model_attr.value, Attribute):
model_attr_chain.insert(0, model_attr.value.attrname)
model_attr.value = model_attr.value.expr

if isinstance(model_attr.value, Name):
model_attr_chain.insert(0, model_attr.value.name)

return model_attr_chain[-1]


def find_ancestor(node: ClassDef, ancestors: List[str]) -> str:
"""Find the class ancestor from the list of ancestors."""
ancestor_class_types = [ancestor.qname() for ancestor in node.ancestors()]
for checked_ancestor in ancestors:
if checked_ancestor in ancestor_class_types:
return checked_ancestor

return ""


def is_version_compatible(specifier_set: Union[str, SpecifierSet]) -> bool:
"""Return True if the Nautobot version is compatible with the given version specifier_set."""
if not specifier_set:
return True
if isinstance(specifier_set, str):
specifier_set = SpecifierSet(specifier_set)
return specifier_set.contains(MINIMUM_NAUTOBOT_VERSION)


def load_v2_code_location_changes():
"""Black magic data transform, needs schema badly."""
with open(files("pylint_nautobot") / "data" / "v2" / "v2-code-location-changes.yaml", encoding="utf-8") as rules: # type: ignore
changes = safe_load(rules)
changes_map = {}
for change in changes:
if change["Old Module"] not in changes_map:
changes_map[change["Old Module"]] = {}
changes_map[change["Old Module"]][change["Class/Function(s)"]] = change["New Module"]
return changes_map


MAP_CODE_LOCATION_CHANGES = load_v2_code_location_changes()
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ exclude = '''
| buck-out
| build
| dist
| var
)/
| settings.py # This is where you define files that should not be stylized by black
# the root of the project
Expand Down
5 changes: 4 additions & 1 deletion tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,19 @@ def run_command(context, command, **kwargs):
help={
"force_rm": "Always remove intermediate containers",
"cache": "Whether to use Docker's cache when building the image (defaults to enabled)",
"pull": "Always attempt to pull a newer version of the base image",
}
)
def build(context, force_rm=False, cache=True):
def build(context, force_rm=False, cache=True, pull=False):
"""Build Nautobot docker image."""
command = "build"

if not cache:
command += " --no-cache"
if force_rm:
command += " --force-rm"
if pull:
command += " --pull"

print(f"Building Nautobot with Python {context.pylint_nautobot.python_ver}...")
docker_compose(context, command)
Expand Down
17 changes: 8 additions & 9 deletions tests/test_deprecated_status_model.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
"""Tests for deprecated status model."""
from pathlib import Path

from pylint.testutils import CheckerTestCase
from pytest import mark

from pylint_nautobot.deprecated_status_model import NautobotDeprecatedStatusModelChecker

from .utils import assert_error_file
from .utils import assert_good_file
from .utils import parametrize_error_files
from .utils import parametrize_good_files

_INPUTS_PATH = Path(__file__).parent / "inputs/deprecated-status-model/"
_EXPECTED_ERRORS = {
"status_model": {
"msg_id": "nb-status-field-instead-of-status-model",
Expand All @@ -25,11 +24,11 @@ class TestDeprecatedStatusModelChecker(CheckerTestCase):

CHECKER_CLASS = NautobotDeprecatedStatusModelChecker

@mark.parametrize("path", _INPUTS_PATH.glob("error_*.py"))
def test_deprecated_status_model(self, path):
assert_error_file(self, path, _EXPECTED_ERRORS)
@parametrize_error_files(__file__, _EXPECTED_ERRORS)
def test_error(self, filename, expected_error):
assert_error_file(self, filename, expected_error)

# TBD: Missing test for good_status_model.py
@mark.parametrize("path", _INPUTS_PATH.glob("good_*.py"))
def test_no_issues(self, path):
assert_good_file(self, path)
@parametrize_good_files(__file__)
def test_good(self, filename):
assert_good_file(self, filename)
27 changes: 16 additions & 11 deletions tests/test_incorrect_base_class.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
"""Tests for incorrect base class checker."""
from pathlib import Path

from pylint.testutils import CheckerTestCase
from pytest import mark

from pylint_nautobot.incorrect_base_class import NautobotIncorrectBaseClassChecker

from .utils import assert_error_file
from .utils import assert_good_file
from .utils import parametrize_error_files
from .utils import parametrize_good_files

_INPUTS_PATH = Path(__file__).parent / "inputs/incorrect-base-class"
_EXPECTED_ERROR_ARGS = {

def _find_error_node(module_node):
return module_node.body[1]


_EXPECTED_ERRORS = {
"model": {
"versions": ">=2",
"msg_id": "nb-incorrect-base-class",
"line": 4,
"col_offset": 0,
"node": lambda module_node: module_node.body[1],
"node": _find_error_node,
"args": ("django.db.models.base.Model", "nautobot.core.models.BaseModel"),
},
}
Expand All @@ -26,10 +31,10 @@ class TestIncorrectBaseClassChecker(CheckerTestCase):

CHECKER_CLASS = NautobotIncorrectBaseClassChecker

@mark.parametrize("path", _INPUTS_PATH.glob("error_*.py"))
def test_incorrect_base_class(self, path):
assert_error_file(self, path, _EXPECTED_ERROR_ARGS)
@parametrize_error_files(__file__, _EXPECTED_ERRORS)
def test_error(self, filename, expected_error):
assert_error_file(self, filename, expected_error)

@mark.parametrize("path", _INPUTS_PATH.glob("good_*.py"))
def test_no_issues(self, path):
assert_good_file(self, path)
@parametrize_good_files(__file__)
def test_good(self, filename):
assert_good_file(self, filename)
Loading