CI: Refactor tests and enable Ruff (#4354)

pyproject.toml

@@ -19,7 +19,6 @@ known_first_party = ["pyavd", "schema_tools"]
 line-length = 160
 extend-exclude = [
   "python-avd/pyavd/_cv/api/**/*",
-  "python-avd/tests/**/*",
   "ansible_collections/arista/avd/tests/**/*",
 ]
 target-version = "py310"
@@ -78,6 +77,11 @@ convention = "google"
 "python-avd/pyavd/_cv/client/*.py" = [
   "B904",  # Within an `except` clause, raise exceptions with `raise - TODO: Improve code
 ]
+"python-avd/tests/**/*" = [
+  "S101",  # Accept 'assert' in pytest.
+  "INP001",  # implicit namespace package. Add an `__init__.py` - Tests are not in packages
+]
+
 
 [tool.ruff.lint.pylint]
 max-args = 12

python-avd/pyavd/_schema/avdschema.py

@@ -3,7 +3,7 @@
 # that can be found in the LICENSE file.
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any, NoReturn
+from typing import TYPE_CHECKING, Any
 
 import jsonschema
 from deepmerge import always_merger
@@ -86,7 +86,7 @@ def load_schema(self, schema: dict | None = None, schema_id: str | None = None)
             msg = "An error occurred during creation of the validator"
             raise AristaAvdError(msg) from e
 
-    def extend_schema(self, schema: dict) -> NoReturn:
+    def extend_schema(self, schema: dict) -> None:
         for validation_error in self.validate_schema(schema):
             raise validation_error
         always_merger.merge(self._schema, schema)

python-avd/tests/pyavd/eos_cli_config_gen/conftest.py

@@ -1,32 +1,28 @@
 # Copyright (c) 2023-2024 Arista Networks, Inc.
 # Use of this source code is governed by the Apache License 2.0
 # that can be found in the LICENSE file.
-from glob import iglob
 from pathlib import Path
 
 import pytest
 
-from ...utils import read_file, read_vars
+from tests.utils import read_file, read_vars
 
 VARS_PATH = Path(Path(__file__).parent, "../artifacts/eos_cli_config_gen/vars")
 CONFIGS_PATH = Path(Path(__file__).parent, "../artifacts/eos_cli_config_gen/configs")
 DOCS_PATH = Path(Path(__file__).parent, "../artifacts/eos_cli_config_gen/documentation")
 
 
-def get_hostnames():
+def get_hostnames() -> list:
     assert Path(VARS_PATH).is_dir()
 
-    hostnames = []
-    for device_var_file in iglob(f"{VARS_PATH}/*"):
-        hostnames.append(Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json"))
-
-    return hostnames
+    return [Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json") for device_var_file in Path(VARS_PATH).glob("*")]
 
 
 @pytest.fixture(scope="module")
 def all_inputs() -> dict[str, dict]:
     """
-    Return dict with all inputs like
+    Return dict with all inputs.
+
     {
         "hostname1": dict
         "hostname2": dict
@@ -36,23 +32,23 @@ def all_inputs() -> dict[str, dict]:
     assert Path(VARS_PATH).is_dir()
 
     inputs = {}
-    for device_var_file in iglob(f"{VARS_PATH}/*"):
+    for device_var_file in Path(VARS_PATH).glob("*"):
         hostname = Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json")
         inputs[hostname] = read_vars(device_var_file)
 
     return inputs
 
 
 @pytest.fixture(scope="module", params=get_hostnames())
-def hostname(request) -> dict:
-    hostname = request.param
-    return hostname
+def hostname(request: pytest.FixtureRequest) -> dict:
+    return request.param
 
 
 @pytest.fixture(scope="module")
 def configs() -> dict:
     """
-    Return dict with all configs like
+    Return dict with all configs.
+
     {
         "hostname1": str
         "hostname2": str
@@ -62,7 +58,7 @@ def configs() -> dict:
     assert Path(CONFIGS_PATH).is_dir()
 
     result = {}
-    for filename in iglob(f"{CONFIGS_PATH}/*"):
+    for filename in Path(CONFIGS_PATH).glob("*"):
         hostname = Path(filename).name.removesuffix(".cfg")
         result[hostname] = read_file(filename)
 
@@ -72,7 +68,8 @@ def configs() -> dict:
 @pytest.fixture(scope="module")
 def device_docs() -> dict:
     """
-    Return dict with all docs like
+    Return dict with all docs.
+
     {
         "hostname1": str
         "hostname2": str
@@ -82,7 +79,7 @@ def device_docs() -> dict:
     assert Path(DOCS_PATH).is_dir()
 
     result = {}
-    for filename in iglob(f"{DOCS_PATH}/*"):
+    for filename in Path(DOCS_PATH).glob("*"):
         hostname = Path(filename).name.removesuffix(".md")
         result[hostname] = read_file(filename)
 

python-avd/tests/pyavd/eos_cli_config_gen/test_get_device_config.py

@@ -4,11 +4,8 @@
 from pyavd import get_device_config, validate_structured_config
 
 
-def test_get_device_config(hostname: str, all_inputs: dict, configs: dict):
-    """
-    Test get_device_config
-    """
-
+def test_get_device_config(hostname: str, all_inputs: dict, configs: dict) -> None:
+    """Test get_device_config."""
     structured_config: dict = all_inputs[hostname]
     expected_config: str = configs[hostname]
 
@@ -18,6 +15,5 @@ def test_get_device_config(hostname: str, all_inputs: dict, configs: dict):
     device_config = get_device_config(structured_config)
 
     assert isinstance(device_config, str)
-    # assert f"hostname {hostname}\n" in eos_config
 
     assert device_config == expected_config

python-avd/tests/pyavd/eos_cli_config_gen/test_get_device_doc.py

@@ -4,11 +4,8 @@
 from pyavd import get_device_doc
 
 
-def test_get_device_doc(hostname: str, all_inputs: dict, device_docs: dict):
-    """
-    Test get_device_config
-    """
-
+def test_get_device_doc(hostname: str, all_inputs: dict, device_docs: dict) -> None:
+    """Test get_device_config."""
     structured_config: dict = all_inputs[hostname]
     if not structured_config.get("generate_device_documentation", True):
         return

python-avd/tests/pyavd/eos_cli_config_gen/test_validate_structured_config.py

@@ -8,20 +8,18 @@
 SCHEMA = AvdSchemaTools(schema_id="eos_cli_config_gen").avdschema._schema
 
 
-def test_validate_structured_config_with_valid_data(hostname: str, all_inputs: dict):
-    """
-    Test validate_structured_config
-    """
+def test_validate_structured_config_with_valid_data(hostname: str, all_inputs: dict) -> None:
+    """Test validate_structured_config."""
     structured_config = all_inputs[hostname]
     validation_result = validate_structured_config(structured_config)
-    assert hostname and validation_result.validation_errors == []
-    assert hostname and validation_result.failed is False
+    assert hostname
+    assert validation_result.validation_errors == []
+    assert hostname
+    assert validation_result.failed is False
 
 
-def test_validate_structured_config_with_invalid_data(hostname: str, all_inputs: dict):
-    """
-    Test validate_structured_config
-    """
+def test_validate_structured_config_with_invalid_data(hostname: str, all_inputs: dict) -> None:
+    """Test validate_structured_config."""
     structured_config: dict = all_inputs[hostname]
 
     updated = False

python-avd/tests/pyavd/eos_designs/conftest.py

@@ -1,34 +1,29 @@
 # Copyright (c) 2023-2024 Arista Networks, Inc.
 # Use of this source code is governed by the Apache License 2.0
 # that can be found in the LICENSE file.
-from glob import iglob
 from pathlib import Path
 
 import pytest
 
 from pyavd import get_avd_facts
-
-from ...utils import read_file, read_vars
+from tests.utils import read_file, read_vars
 
 VARS_PATH = Path(Path(__file__).parent, "../artifacts/eos_designs_unit_tests/vars")
 STRUCTURED_CONFIGS_PATH = Path(Path(__file__).parent, "../artifacts/eos_designs_unit_tests/structured_configs")
 CONFIGS_PATH = Path(Path(__file__).parent, "../artifacts/eos_designs_unit_tests/configs")
 
 
-def get_hostnames():
+def get_hostnames() -> list:
     assert Path(VARS_PATH).is_dir()
 
-    hostnames = []
-    for device_var_file in iglob(f"{VARS_PATH}/*"):
-        hostnames.append(Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json"))
-
-    return hostnames
+    return [Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json") for device_var_file in Path(VARS_PATH).glob("*")]
 
 
 @pytest.fixture(scope="module")
 def all_inputs() -> dict[str, dict]:
     """
-    Return dict with all inputs like
+    Return dict with all inputs.
+
     {
         "hostname1": dict
         "hostname2": dict
@@ -38,31 +33,29 @@ def all_inputs() -> dict[str, dict]:
     assert Path(VARS_PATH).is_dir()
 
     inputs = {}
-    for device_var_file in iglob(f"{VARS_PATH}/*"):
+    for device_var_file in Path(VARS_PATH).glob("*"):
         hostname = Path(device_var_file).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json")
         inputs[hostname] = read_vars(device_var_file)
 
     return inputs
 
 
 @pytest.fixture(scope="module")
-def avd_facts(all_inputs: dict):
-    """
-    Test get_avd_facts
-    """
+def avd_facts(all_inputs: dict) -> dict:
+    """Test get_avd_facts."""
     return get_avd_facts(all_inputs)
 
 
 @pytest.fixture(scope="module", params=get_hostnames())
-def hostname(request) -> dict:
-    hostname = request.param
-    return hostname
+def hostname(request: pytest.FixtureRequest) -> dict:
+    return request.param
 
 
 @pytest.fixture(scope="module")
 def structured_configs() -> dict:
     """
-    Return dict with all structured_configs like
+    Return dict with all structured_configs.
+
     {
         "hostname1": dict
         "hostname2": dict
@@ -72,7 +65,7 @@ def structured_configs() -> dict:
     assert Path(STRUCTURED_CONFIGS_PATH).is_dir()
 
     result = {}
-    for filename in iglob(f"{STRUCTURED_CONFIGS_PATH}/*"):
+    for filename in Path(STRUCTURED_CONFIGS_PATH).glob("*"):
         hostname = Path(filename).name.removesuffix(".yaml").removesuffix(".yml").removesuffix(".json")
         result[hostname] = read_vars(filename)
 
@@ -82,7 +75,8 @@ def structured_configs() -> dict:
 @pytest.fixture(scope="module")
 def configs() -> dict:
     """
-    Return dict with all configs like
+    Return dict with all configs.
+
     {
         "hostname1": str
         "hostname2": str
@@ -92,7 +86,7 @@ def configs() -> dict:
     assert Path(CONFIGS_PATH).is_dir()
 
     result = {}
-    for filename in iglob(f"{CONFIGS_PATH}/*"):
+    for filename in Path(CONFIGS_PATH).glob("*"):
         hostname = Path(filename).name.removesuffix(".cfg")
         result[hostname] = read_file(filename)
 

python-avd/tests/pyavd/eos_designs/test_get_avd_facts.py

@@ -4,10 +4,8 @@
 from pyavd import get_avd_facts
 
 
-def test_get_avd_facts(all_inputs: dict):
-    """
-    Test get_avd_facts
-    """
+def test_get_avd_facts(all_inputs: dict) -> None:
+    """Test get_avd_facts."""
     avd_facts = get_avd_facts(all_inputs)
 
     assert isinstance(avd_facts, dict)

python-avd/tests/pyavd/eos_designs/test_get_device_config.py

@@ -4,11 +4,8 @@
 from pyavd import get_device_config, validate_structured_config
 
 
-def test_get_device_config(hostname: str, all_inputs: dict, structured_configs: dict, configs: dict):
-    """
-    Test get_device_config
-    """
-
+def test_get_device_config(hostname: str, all_inputs: dict, structured_configs: dict, configs: dict) -> None:
+    """Test get_device_config."""
     # Loading inputs first and then updating structured config on top.
     # This is how Ansible behaves, so we need this to generate the same configs.
     # The underlying cause is eos_cli_config_gen inputs being set in eos_designs molecule vars,

python-avd/tests/pyavd/eos_designs/test_get_device_structured_config.py

@@ -4,10 +4,8 @@
 from pyavd import get_device_structured_config, validate_inputs
 
 
-def test_get_device_structured_config(hostname: str, all_inputs: dict, avd_facts: dict, structured_configs: dict):
-    """
-    Test get_device_structured_config
-    """
+def test_get_device_structured_config(hostname: str, all_inputs: dict, avd_facts: dict, structured_configs: dict) -> None:
+    """Test get_device_structured_config."""
     inputs = all_inputs[hostname]
 
     # run validation on inputs to ensure it is converted

python-avd/tests/pyavd/eos_designs/test_validate_inputs.py

@@ -4,11 +4,11 @@
 from pyavd import validate_inputs
 
 
-def test_validate_inputs_with_valid_inputs(hostname: str, all_inputs: dict):
-    """
-    Test validate_inputs
-    """
+def test_validate_inputs_with_valid_inputs(hostname: str, all_inputs: dict) -> None:
+    """Test validate_inputs."""
     inputs = all_inputs[hostname]
     validation_result = validate_inputs(inputs)
-    assert hostname and validation_result.validation_errors == []
-    assert hostname and validation_result.failed is False
+    assert hostname
+    assert validation_result.validation_errors == []
+    assert hostname
+    assert validation_result.failed is False

python-avd/tests/pyavd/eos_designs/test_validate_structured_config.py

@@ -8,20 +8,18 @@
 SCHEMA = AvdSchemaTools(schema_id="eos_cli_config_gen").avdschema._schema
 
 
-def test_validate_structured_config_with_valid_data(hostname: str, structured_configs: dict):
-    """
-    Test validate_structured_config
-    """
+def test_validate_structured_config_with_valid_data(hostname: str, structured_configs: dict) -> None:
+    """Test validate_structured_config."""
     structured_config = structured_configs[hostname]
     validation_result = validate_structured_config(structured_config)
-    assert hostname and validation_result.validation_errors == []
-    assert hostname and validation_result.failed is False
+    assert hostname
+    assert validation_result.validation_errors == []
+    assert hostname
+    assert validation_result.failed is False
 
 
-def test_validate_structured_config_with_invalid_data(hostname: str, structured_configs: dict):
-    """
-    Test validate_structured_config
-    """
+def test_validate_structured_config_with_invalid_data(hostname: str, structured_configs: dict) -> None:
+    """Test validate_structured_config."""
     structured_config = structured_configs[hostname]
 
     updated = False