Refactor API to use standard types (#4240)

Related: #4218

---------

Co-authored-by: Kate Case <this.is@katherineca.se>

.config/pydoclint-baseline.txt

@@ -1,16 +1,3 @@
-src/molecule/api.py
-	DOC101: Method `UserListMap.__getitem__`: Docstring contains fewer arguments than in function signature.
-	DOC106: Method `UserListMap.__getitem__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
-	DOC107: Method `UserListMap.__getitem__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
-	DOC103: Method `UserListMap.__getitem__`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [i: ].
-	DOC201: Method `UserListMap.__getitem__` does not have a return section in docstring
-	DOC201: Function `drivers` does not have a return section in docstring
-	DOC101: Function `verifiers`: Docstring contains fewer arguments than in function signature.
-	DOC106: Function `verifiers`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
-	DOC107: Function `verifiers`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
-	DOC103: Function `verifiers`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [config: ].
-	DOC201: Function `verifiers` does not have a return section in docstring
---------------------
 src/molecule/command/base.py
 	DOC303: Class `Base`: The __init__() docstring does not need a "Returns" section, because it cannot return anything
 	DOC302: Class `Base`: The class docstring does not need a "Returns" section, because __init__() cannot return anything
@@ -561,10 +548,6 @@ src/molecule/verifier/base.py
 	DOC603: Class `Verifier`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [__metaclass__: ]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
 	DOC106: Method `Verifier.__init__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
 	DOC107: Method `Verifier.__init__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
-	DOC101: Method `Verifier.execute`: Docstring contains fewer arguments than in function signature.
-	DOC106: Method `Verifier.execute`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
-	DOC107: Method `Verifier.execute`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
-	DOC103: Method `Verifier.execute`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [action_args: ].
 	DOC101: Method `Verifier.__eq__`: Docstring contains fewer arguments than in function signature.
 	DOC106: Method `Verifier.__eq__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
 	DOC107: Method `Verifier.__eq__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints

pyproject.toml

@@ -73,6 +73,8 @@ source_pkgs = ["molecule"]
 
 [tool.mypy]
 cache_dir = "./.cache/.mypy"
+# To ensure that calling `mypy .` produces desired results (vscode)
+exclude = ["build"]
 files = ["src", "tests"]
 strict = true
 
@@ -88,7 +90,7 @@ allow-init-docstring = true
 arg-type-hints-in-docstring = false
 baseline = ".config/pydoclint-baseline.txt"
 check-return-types = false
-exclude = '\.git|\.tox|build|out|venv'
+exclude = '\.cache|\.git|\.tox|build|out|venv'
 should-document-private-class-attributes = true
 show-filenames-in-every-violation-message = true
 skip-checking-short-docstrings = false

src/molecule/api.py

@@ -5,41 +5,22 @@
 import logging
 import traceback
 
-from collections import UserList
-from typing import Any
+from collections import OrderedDict
+from typing import TYPE_CHECKING
 
 import pluggy
 
 from ansible_compat.ports import cache
 
-from molecule.driver.base import Driver  # noqa: F401
-from molecule.verifier.base import Verifier  # noqa: F401
+from molecule.driver.base import Driver
+from molecule.verifier.base import Verifier
 
 
-LOG = logging.getLogger(__name__)
-
-
-class UserListMap(UserList):  # type: ignore[type-arg]
-    """A list where you can also access elements by their name.
-
-    Example:
-    -------
-    foo['boo']
-    foo.boo
-    """
+if TYPE_CHECKING:
+    from molecule.config import Config
 
-    def __getitem__(self, i):  # type: ignore[no-untyped-def]  # noqa: ANN001, ANN204
-        """Implement indexing."""
-        if isinstance(i, int):
-            return super().__getitem__(i)
-        return self.__dict__[i]
 
-    def get(self, key, default):  # type: ignore[no-untyped-def]  # noqa: ANN001, ANN201, D102
-        return self.__dict__.get(key, default)
-
-    def append(self, item) -> None:  # type: ignore[no-untyped-def]  # noqa: ANN001, D102
-        self.__dict__[str(item)] = item
-        super().append(item)
+LOG = logging.getLogger(__name__)
 
 
 class MoleculeRuntimeWarning(RuntimeWarning):
@@ -51,44 +32,74 @@ class IncompatibleMoleculeRuntimeWarning(MoleculeRuntimeWarning):
 
 
 @cache
-def drivers(config: Any | None = None) -> UserListMap:  # noqa: ANN401
+def drivers(config: Config | None = None) -> dict[str, Driver]:
     """Return list of active drivers.
 
     Args:
         config: plugin config
+
+    Returns:
+        A dictionary of active drivers by name.
     """
-    plugins = UserListMap()
+    plugins = OrderedDict()
     pm = pluggy.PluginManager("molecule.driver")
     try:
         pm.load_setuptools_entrypoints("molecule.driver")
     except (Exception, SystemExit):
         # These are not fatal because a broken driver should not make the entire
         # tool unusable.
         LOG.error("Failed to load driver entry point %s", traceback.format_exc())  # noqa: TRY400
-    for p in pm.get_plugins():
-        try:
-            plugins.append(p(config))
-        except (Exception, SystemExit) as e:  # noqa: PERF203
-            LOG.error("Failed to load %s driver: %s", pm.get_name(p), str(e))  # noqa: TRY400
-    plugins.sort()
+    for plugin in pm.get_plugins():
+        if issubclass(plugin, Driver):
+            try:
+                driver = plugin(config)
+                plugins[driver.name] = driver
+            except (Exception, SystemExit, TypeError) as e:
+                LOG.error(  # noqa: TRY400
+                    "Failed to load %s driver: %s",
+                    pm.get_name(plugin),
+                    str(e),
+                )
+        else:
+            msg = f"Skipped loading plugin class {plugin} because is not a subclass of Driver."
+            LOG.error(msg)
+
     return plugins
 
 
 @cache
-def verifiers(config=None) -> UserListMap:  # type: ignore[no-untyped-def]  # noqa: ANN001
-    """Return list of active verifiers."""
-    plugins = UserListMap()
+def verifiers(config: Config | None = None) -> dict[str, Verifier]:
+    """Return list of active verifiers.
+
+    Args:
+        config: plugin config
+
+    Returns:
+        A dictionary of active verifiers by name.
+    """
+    plugins = OrderedDict()
     pm = pluggy.PluginManager("molecule.verifier")
     try:
         pm.load_setuptools_entrypoints("molecule.verifier")
     except Exception:  # noqa: BLE001
         # These are not fatal because a broken verifier should not make the entire
         # tool unusable.
         LOG.error("Failed to load verifier entry point %s", traceback.format_exc())  # noqa: TRY400
-    for p in pm.get_plugins():
+    for plugin_class in pm.get_plugins():
         try:
-            plugins.append(p(config))
+            if issubclass(plugin_class, Verifier):
+                plugin = plugin_class(config)
+                plugins[plugin.name] = plugin
         except Exception as e:  # noqa: BLE001, PERF203
-            LOG.error("Failed to load %s driver: %s", pm.get_name(p), str(e))  # noqa: TRY400
-    plugins.sort()
+            LOG.error("Failed to load %s driver: %s", pm.get_name(plugin), str(e))  # noqa: TRY400
     return plugins
+
+
+__all__ = (
+    "Driver",
+    "IncompatibleMoleculeRuntimeWarning",
+    "MoleculeRuntimeWarning",
+    "Verifier",
+    "drivers",
+    "verifiers",
+)

src/molecule/command/drivers.py

@@ -44,11 +44,9 @@
 def drivers(ctx, format):  # type: ignore[no-untyped-def] # pragma: no cover  # noqa: ANN001, ANN201, A002, ARG001
     """List drivers."""
     drivers = []  # pylint: disable=redefined-outer-name
-    for driver in api.drivers():
-        description = driver
-        if format != "plain":
-            description = driver
-        else:
+    for driver in api.drivers().values():
+        description = str(driver)
+        if format == "plain":
             description = f"{driver!s:16s}[logging.level.notset] {driver.title} Version {driver.version} from {driver.module} python module.)[/logging.level.notset]"  # noqa: E501
         drivers.append([driver, description])
         console.print(description)

src/molecule/command/reset.py

@@ -46,5 +46,5 @@ def reset(ctx, scenario_name):  # type: ignore[no-untyped-def] # pragma: no cove
     command_args = {"subcommand": subcommand}
 
     base.execute_cmdline_scenarios(scenario_name, args, command_args)
-    for driver in drivers():
+    for driver in drivers().values():
         driver.reset()

src/molecule/config.py

@@ -44,6 +44,8 @@
 if TYPE_CHECKING:
     from collections.abc import MutableMapping
 
+    from molecule.verifier.base import Verifier
+
 
 LOG = logging.getLogger(__name__)
 MOLECULE_DEBUG = boolean(os.environ.get("MOLECULE_DEBUG", "False"))
@@ -262,8 +264,20 @@ def state(self):  # type: ignore[no-untyped-def]  # noqa: ANN201, D102
         return state.State(self)
 
     @cached_property
-    def verifier(self):  # type: ignore[no-untyped-def]  # noqa: ANN201, D102
-        return api.verifiers(self).get(self.config["verifier"]["name"], None)  # type: ignore[no-untyped-call]
+    def verifier(self) -> Verifier:
+        """Retrieve current verifier.
+
+        Raises:
+            RuntimeError: If is not able to find the driver.
+
+        Returns:
+            Instance of Verifier driver.
+        """
+        name = self.config["verifier"]["name"]
+        if name not in api.verifiers(self):
+            msg = f"Unable to find '{name}' verifier driver."
+            raise RuntimeError(msg)
+        return api.verifiers(self)[name]
 
     def _get_driver_name(self):  # type: ignore[no-untyped-def] # noqa: ANN202
         # the state file contains the driver from the last run

src/molecule/driver/base.py

@@ -269,17 +269,17 @@ def get_playbook(self, step):  # type: ignore[no-untyped-def]  # noqa: ANN001, A
             return p
         return None
 
-    def schema_file(self):  # type: ignore[no-untyped-def]  # noqa: ANN201, D102
+    def schema_file(self) -> None | str:  # noqa: D102
         return None
 
-    def modules_dir(self):  # type: ignore[no-untyped-def]  # noqa: ANN201
+    def modules_dir(self) -> str | None:
         """Return path to ansible modules included with driver."""
         p = os.path.join(self._path, "modules")  # noqa: PTH118
         if os.path.isdir(p):  # noqa: PTH112
             return p
         return None
 
-    def reset(self):  # type: ignore[no-untyped-def]  # noqa: ANN201
+    def reset(self) -> None:
         """Release all resources owned by molecule.
 
         This is a destructive operation that would affect all resources managed

src/molecule/driver/delegated.py

@@ -24,7 +24,7 @@
 import os
 
 from molecule import util
-from molecule.api import Driver  # type: ignore[attr-defined]
+from molecule.api import Driver
 from molecule.data import __file__ as data_module
 
 

src/molecule/provisioner/ansible.py

@@ -920,7 +920,7 @@ def _get_modules_directories(self) -> list[str]:
             util.abs_path(os.path.join(self._get_plugin_directory(), "modules")),  # noqa: PTH118
         )
 
-        for d in drivers():
+        for d in drivers().values():
             p = d.modules_dir()
             if p:
                 paths.append(p)

src/molecule/shell.py

@@ -65,7 +65,7 @@ def print_version(ctx, param, value):  # type: ignore[no-untyped-def]  # noqa: A
     msg = f"molecule [{color}]{v}[/] using python [repr.number]{sys.version_info[0]}.{sys.version_info[1]}[/] \n"  # noqa: E501
 
     msg += f"    [repr.attrib_name]ansible[/][dim]:[/][repr.number]{app.runtime.version}[/]"
-    for driver in drivers():
+    for driver in drivers().values():
         msg += f"\n    [repr.attrib_name]{driver!s}[/][dim]:[/][repr.number]{driver.version}[/][dim] from {driver.module}"  # noqa: E501
         if driver.required_collections:
             msg += " requiring collections:"

src/molecule/verifier/ansible.py

@@ -24,7 +24,7 @@
 import os
 
 from molecule import util
-from molecule.api import Verifier  # type: ignore[attr-defined]
+from molecule.api import Verifier
 
 
 log = logging.getLogger(__name__)

src/molecule/verifier/base.py

@@ -66,8 +66,12 @@ def default_env(self):  # type: ignore[no-untyped-def] # pragma: no cover  # noq
         """
 
     @abc.abstractmethod
-    def execute(self, action_args=None):  # type: ignore[no-untyped-def] # pragma: no cover  # noqa: ANN001, ANN201
-        """Execute ``cmd`` and returns None."""
+    def execute(self, action_args: None | list[str] = None) -> None:  # pragma: no cover
+        """Execute ``cmd`` and returns None.
+
+        Args:
+            action_args: list of arguments to be passed.
+        """
 
     @abc.abstractmethod
     def schema(self):  # type: ignore[no-untyped-def] # pragma: no cover  # noqa: ANN201

src/molecule/verifier/testinfra.py

@@ -25,7 +25,7 @@
 import os
 
 from molecule import util
-from molecule.api import Verifier  # type: ignore[attr-defined]
+from molecule.api import Verifier
 
 
 LOG = logging.getLogger(__name__)

tests/unit/driver/test_delegated.py

@@ -215,7 +215,6 @@ def test_ansible_connection_options(_instance):  # type: ignore[no-untyped-def]
     assert is_subset(x, _instance.ansible_connection_options("foo"))  # type: ignore[no-untyped-call]
 
 
-@pytest.mark.xfail(reason="Needs rewrite since switch to delegated")
 @pytest.mark.parametrize(
     "config_instance",
     ["_driver_managed_section_data"],  # noqa: PT007
@@ -256,7 +255,7 @@ def test_ansible_connection_options_when_managed(mocker: MockerFixture, _instanc
         ),
     }
 
-    assert ssh_expected_data == _instance.ansible_connection_options("foo")
+    assert ssh_expected_data.items() <= _instance.ansible_connection_options("foo").items()
 
     winrm_case_data = mocker.patch(
         "molecule.driver.delegated.Delegated._get_instance_config",
@@ -276,7 +275,7 @@ def test_ansible_connection_options_when_managed(mocker: MockerFixture, _instanc
         "ansible_connection": "winrm",
     }
 
-    assert winrm_expected_data == _instance.ansible_connection_options("foo")
+    assert winrm_expected_data.items() <= _instance.ansible_connection_options("foo").items()
 
 
 def test_ansible_connection_options_handles_missing_instance_config_managed(  # type: ignore[no-untyped-def]  # noqa: ANN201, D103

tests/unit/test_api.py

@@ -23,22 +23,16 @@
 from molecule import api
 
 
-def test_api_molecule_drivers_as_attributes():  # type: ignore[no-untyped-def]  # noqa: ANN201, D103
+def test_api_drivers() -> None:  # noqa: D103
     results = api.drivers()
-    assert hasattr(results, "default")
-    assert isinstance(results.default, api.Driver)  # type: ignore[attr-defined] # pylint:disable=no-member
 
-
-def test_api_drivers():  # type: ignore[no-untyped-def]  # noqa: ANN201, D103
-    results = api.drivers()
-
-    for result in results:
-        assert isinstance(result, api.Driver)  # type: ignore[attr-defined]
+    for result in results.values():
+        assert isinstance(result, api.Driver)
 
     assert "default" in results
 
 
-def test_api_verifiers():  # type: ignore[no-untyped-def]  # noqa: ANN201, D103
+def test_api_verifiers() -> None:  # noqa: D103
     x = ["testinfra", "ansible"]
 
     assert all(elem in api.verifiers() for elem in x)