Repository: NucciTheBoss/cleantest (language: python)

Issue:
KeyError: 'PYTHONPATH' - Thrown when no charm library is specified
If the end-user does not pass something to the `StartEnv` hook for `PYTHONPATH`, then the test will fail since `cleantest` cannot find a value for `PYTHONPATH`. See the error message below:

```
self = <cleantest.provider.data.env_data.EnvDataStore object at 0x7fd1e08d2bf0>, env_var = 'PYTHONPATH'

    def get(self, env_var: str) -> Any:
>       if isinstance(self.__env[env_var], list):
E       KeyError: 'PYTHONPATH'

/home/nucci/.pyenv/versions/hpct/lib/python3.10/site-packages/cleantest/provider/data/env_data.py:22: KeyError
```

This needs to be fixed quickly.


Discussion / hints:

Fix patch:
diff --git a/tox.ini b/tox.ini
index c9f9b1b..94f7700 100644
--- a/tox.ini
+++ b/tox.ini
@@ -22,7 +22,7 @@ passenv =
   MODEL_SETTINGS
 
 [testenv:fmt]
-description = Apply coding style standards to code
+description = Apply coding style standards to code.
 deps =
   black
   isort
@@ -31,7 +31,7 @@ commands =
   black {[vars]all_path}
 
 [testenv:lint]
-description = Check code against coding style standards
+description = Check code against coding style standards.
 deps =
   black
   flake8
@@ -63,10 +63,21 @@ deps =
 commands =
   mkdocs serve
 
+[testenv:publish]
+description = Publish cleantest to PyPI.
+deps = 
+  twine
+  setuptools
+  wheel
+commands =
+  rm -rf {toxinidir}/dist
+  python setup.py sdist bdist_wheel
+  twine upload {toxinidir}/dist/*
+
 [testenv:local]
 
 [testenv:lxd-local]
-description = Run functional tests for a locally-hosted LXD cluster
+description = Run functional tests for a locally-hosted LXD cluster.
 deps =
   -r {toxinidir}/requirements.txt
   pytest
@@ -75,10 +86,10 @@ commands =
   pytest -v --tb native --log-cli-level=ERROR {[vars]tst_path}/lxd/local
 
 [testenv:lxd-local-parallel]
-description = Run functional tests in parallel for a locally-hosted LXD cluster
+description = Run functional tests in parallel for a locally-hosted LXD cluster.
 
 [testenv:lxd-remote]
-description = Run functional tests for a remotely-hosted LXD cluster
+description = Run functional tests for a remotely-hosted LXD cluster.
 
 [testenv:lxd-remote-parallel]
-description = Run functional tests in parallel for a remotely-hosted LXD cluster
+description = Run functional tests in parallel for a remotely-hosted LXD cluster.


Test patch:
diff --git a/src/cleantest/__init__.py b/src/cleantest/__init__.py
index 901656d..9a75b22 100644
--- a/src/cleantest/__init__.py
+++ b/src/cleantest/__init__.py
@@ -2,4 +2,4 @@
 # Copyright 2022 Canonical Ltd.
 # See LICENSE file for licensing details.
 
-from .config import Simple, StartEnvHook, StartTestletHook, StopEnvHook, StopTestletHook
+from .control import Configure
diff --git a/src/cleantest/config/__init__.py b/src/cleantest/config/__init__.py
deleted file mode 100644
index c9925e0..0000000
--- a/src/cleantest/config/__init__.py
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/usr/bin/env python3
-# Copyright 2022 Canonical Ltd.
-# See LICENSE file for licensing details.
-
-from .configurator import Simple
-from .hooks import StartEnvHook, StartTestletHook, StopEnvHook, StopTestletHook
diff --git a/src/cleantest/config/hooks.py b/src/cleantest/config/hooks.py
deleted file mode 100644
index 46219ca..0000000
--- a/src/cleantest/config/hooks.py
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/usr/bin/env python3
-# Copyright 2022 Canonical Ltd.
-# See LICENSE file for licensing details.
-
-"""Hooks for configuring test providers and environments."""
-
-from __future__ import annotations
-
-from typing import List
-
-from pydantic import BaseModel
-
-
-class StartEnvHook(BaseModel):
-    name: str = "default"
-    packages: List[str] | None = None
-    requirements: str | List[str] | None = None
-    constraints: str | List[str] | None = None
-    python_path: List[str] | None = None
-
-
-class StopEnvHook(BaseModel):
-    """Not implemented yet as I do not know if hooks are the move."""
-
-    def __init__(self) -> None:
-        raise NotImplementedError("Hook not implemented yet.")
-
-
-class StartTestletHook(BaseModel):
-    """Not implemented yet as I do not know if hooks are the move."""
-
-    def __init__(self) -> None:
-        raise NotImplementedError("Hook not implemented yet.")
-
-
-class StopTestletHook(BaseModel):
-    """Not implemented yet as I do not know if hooks are the move."""
-
-    def __init__(self) -> None:
-        raise NotImplementedError("Hook not implemented yet.")
diff --git a/src/cleantest/control/__init__.py b/src/cleantest/control/__init__.py
new file mode 100644
index 0000000..816949f
--- /dev/null
+++ b/src/cleantest/control/__init__.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+from .configurator import Configure
diff --git a/src/cleantest/config/configurator.py b/src/cleantest/control/configurator.py
similarity index 55%
rename from src/cleantest/config/configurator.py
rename to src/cleantest/control/configurator.py
index c353d2c..f1d110a 100644
--- a/src/cleantest/config/configurator.py
+++ b/src/cleantest/control/configurator.py
@@ -2,44 +2,57 @@
 # Copyright 2022 Canonical Ltd.
 # See LICENSE file for licensing details.
 
-""""""
+"""Manage the state and flow of cleantest."""
 
 from __future__ import annotations
 
-from typing import Dict
+from collections import deque
+from typing import Deque
 
 from pydantic import BaseModel
 
-from cleantest.config.hooks import StartEnvHook, StartTestletHook, StopEnvHook, StopTestletHook
+from cleantest.hooks import StartEnvHook, StartTestletHook, StopEnvHook, StopTestletHook
+
+
+class DuplicateHookNameError(Exception):
+    ...
 
 
 class HookRegistry(BaseModel):
-    start_env: Dict[str, StartEnvHook] = {}
-    stop_env: Dict[str, StopEnvHook] = {}
-    start_testlet: Dict[str, StartTestletHook] = {}
-    stop_testlet: Dict[str, StopTestletHook] = {}
+    start_env: Deque[StartEnvHook] = deque()
+    stop_env: Deque[StopEnvHook] = deque()
+    start_testlet: Deque[StartTestletHook] = deque()
+    stop_testlet: Deque[StopTestletHook] = deque()
 
 
-class Simple(object):
+class Configure(object):
 
     __hook_registry = HookRegistry()
     __metadata = set()
 
     def __new__(cls) -> None:
         if not hasattr(cls, "instance"):
-            cls.instance = super(Simple, cls).__new__(cls)
+            cls.instance = super(Configure, cls).__new__(cls)
         return cls.instance
 
     def register_hook(
         self, hook: StartEnvHook | StopEnvHook | StartTestletHook | StopTestletHook
     ) -> None:
+        lint = [
+            h for h in self.__metadata if hook.name == h[0] and hook.__class__.__name__ == h[1]
+        ]
+        if len(lint) > 0:
+            raise DuplicateHookNameError(
+                f"Hook type {hook.__class__.__name__} with name {hook.name} already exists."
+            )
+
         dispatch = {
             StartEnvHook.__name__: self.__hook_registry.start_env,
             StopEnvHook.__name__: self.__hook_registry.stop_env,
             StartTestletHook.__name__: self.__hook_registry.start_testlet,
             StopTestletHook.__name__: self.__hook_registry.stop_testlet,
         }
-        dispatch[hook.__class__.__name__].update({hook.name: hook})
+        dispatch[hook.__class__.__name__].appendleft(hook)
         self.__metadata.add((hook.name, hook.__class__.__name__))
 
     def deregister_hook(self, hook_name: str) -> None:
@@ -51,20 +64,20 @@ def deregister_hook(self, hook_name: str) -> None:
         }
         for hook in self.__metadata:
             if hook_name == hook[0]:
-                dispatch[hook[1]].pop(hook_name)
+                dispatch[hook[1]].remove(hook_name)
 
-    def get_start_env_hooks(self) -> Dict[str, StartEnvHook]:
+    def get_start_env_hooks(self) -> Deque[StartEnvHook]:
         return self.__hook_registry.start_env
 
-    def get_stop_env_hooks(self) -> Dict[str, StopEnvHook]:
+    def get_stop_env_hooks(self) -> Deque[StopEnvHook]:
         return self.__hook_registry.stop_env
 
-    def get_start_testlet_hooks(self) -> Dict[str, StartTestletHook]:
+    def get_start_testlet_hooks(self) -> Deque[StartTestletHook]:
         return self.__hook_registry.start_testlet
 
-    def get_stop_testlet_hooks(self) -> Dict[str, StopTestletHook]:
+    def get_stop_testlet_hooks(self) -> Deque[StopTestletHook]:
         return self.__hook_registry.stop_testlet
 
     @property
-    def _hooks(self) -> HookRegistry:
+    def _hook_registry(self) -> HookRegistry:
         return self.__hook_registry
diff --git a/src/cleantest/hooks/__init__.py b/src/cleantest/hooks/__init__.py
new file mode 100644
index 0000000..f25b449
--- /dev/null
+++ b/src/cleantest/hooks/__init__.py
@@ -0,0 +1,8 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+from .startenv import StartEnvHook
+from .starttestlet import StartTestletHook
+from .stopenv import StopEnvHook
+from .stoptestlet import StopTestletHook
diff --git a/src/cleantest/hooks/startenv.py b/src/cleantest/hooks/startenv.py
new file mode 100644
index 0000000..5beb9f3
--- /dev/null
+++ b/src/cleantest/hooks/startenv.py
@@ -0,0 +1,19 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Hook run when test environment first starts."""
+
+from __future__ import annotations
+
+from typing import List
+
+from pydantic import BaseModel
+
+
+class StartEnvHook(BaseModel):
+    name: str = "default"
+    packages: List[str] | None = None
+    requirements: str | List[str] | None = None
+    constraints: str | List[str] | None = None
+    python_path: List[str] | None = None
diff --git a/src/cleantest/hooks/starttestlet.py b/src/cleantest/hooks/starttestlet.py
new file mode 100644
index 0000000..b269b3c
--- /dev/null
+++ b/src/cleantest/hooks/starttestlet.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Hook run before testlet starts but after StartEnv hook."""
+
+from pydantic import BaseModel
+
+
+class StartTestletHook(BaseModel):
+    """Not implemented yet as I do not know if hooks are the move."""
+
+    def __init__(self) -> None:
+        raise NotImplementedError("Hook not implemented yet.")
diff --git a/src/cleantest/hooks/stopenv.py b/src/cleantest/hooks/stopenv.py
new file mode 100644
index 0000000..dc0e8cd
--- /dev/null
+++ b/src/cleantest/hooks/stopenv.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Hook run before test environment stops."""
+
+from pydantic import BaseModel
+
+
+class StopEnvHook(BaseModel):
+    """Not implemented yet as I do not know if hooks are the move."""
+
+    def __init__(self) -> None:
+        raise NotImplementedError("Hook not implemented yet.")
diff --git a/src/cleantest/hooks/stoptestlet.py b/src/cleantest/hooks/stoptestlet.py
new file mode 100644
index 0000000..984de77
--- /dev/null
+++ b/src/cleantest/hooks/stoptestlet.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Hook run after testlet finishes but before StopEnv hook."""
+
+from pydantic import BaseModel
+
+
+class StopTestletHook(BaseModel):
+    """Not implemented yet as I do not know if hooks are the move."""
+
+    def __init__(self) -> None:
+        raise NotImplementedError("Hook not implemented yet.")
diff --git a/src/cleantest/pkg/__init__.py b/src/cleantest/pkg/__init__.py
new file mode 100644
index 0000000..2fef9b8
--- /dev/null
+++ b/src/cleantest/pkg/__init__.py
@@ -0,0 +1,6 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+from .charmlib import CharmLibManager
+from .pip import PipManager
diff --git a/src/cleantest/pkg/_manager.py b/src/cleantest/pkg/_manager.py
new file mode 100644
index 0000000..c7aa516
--- /dev/null
+++ b/src/cleantest/pkg/_manager.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Manage package classes sent to test environment provider."""
+
+import inspect
+
+
+class Pkg:
+    @staticmethod
+    def construct() -> None:
+        # TODO: Construct file that can be sent to remote process
+        ...
diff --git a/src/cleantest/pkg/charmlib.py b/src/cleantest/pkg/charmlib.py
new file mode 100644
index 0000000..7d74674
--- /dev/null
+++ b/src/cleantest/pkg/charmlib.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Manager for installing charm libraries inside remote processes."""
+
+from __future__ import annotations
+
+import hashlib
+import pathlib
+import pickle
+import uuid
+from shutil import which
+from typing import List, Tuple
+
+
+class CharmLibManagerError(Exception):
+    ...
+
+
+# NOTE: After class has been constructed, serialize and wormhole over to remote agent.
+class CharmLibManager:
+    def __init__(self, charmlibs: str | List[str]) -> None:
+        raise NotImplementedError("CharmLibManager not available until cleantest-0.2.0")
+        self.__charmlib_store = set()
+        if type(charmlibs) == str:
+            self.__charmlib_store.add(charmlibs)
+        elif type(charmlibs) == list:
+            for lib in charmlibs:
+                self.__charmlib_store.add(lib)
+        else:
+            raise CharmLibManagerError(
+                f"{type(charmlibs)} is invalid. charmlibs must either be str or List[str]."
+            )
+
+    def _setup(self) -> None:
+        # TODO: Check if charmcraft is installed. If not, install it.
+        # Requires snap and snapd to be available on the host system.
+        ...
+
+    def _run(self, provider: str) -> None:
+        # TODO: Invoke __handle_charm_lib_install.
+        ...
+
+    def _dump(self) -> Tuple[str, str]:
+        """Return a path to a pickled object and hash for verification."""
+        filepath = f"/tmp/{uuid.uuid4()}.dat"
+        data = pickle.dumps(self)
+        hash = hashlib.sha224(data).hexdigest()
+        fout = pathlib.Path(filepath)
+        fout.write_bytes(pickle.dumps(self))
+        return filepath, hash
+
+    def _verify(self, hash: str) -> bool:
+        # TODO: Validate hash of sent charmlib manager
+        ...
+
+    def __handle_charm_lib_install(self) -> None:
+        # TODO: Handle placing charm libraries on site-packages.
+        ...
diff --git a/src/cleantest/pkg/pip.py b/src/cleantest/pkg/pip.py
new file mode 100644
index 0000000..7bb8c34
--- /dev/null
+++ b/src/cleantest/pkg/pip.py
@@ -0,0 +1,10 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Manager for installing pip packages inside remote processes."""
+
+
+class PipManager:
+    def __init__(self) -> None:
+        raise NotImplementedError("PipManager not availbe until cleantest-0.2.0")
diff --git a/src/cleantest/provider/data/env_data.py b/src/cleantest/provider/data/env_data.py
index 162aeb3..e7353f6 100644
--- a/src/cleantest/provider/data/env_data.py
+++ b/src/cleantest/provider/data/env_data.py
@@ -18,9 +18,20 @@ def append(self, env_var: str, value: str) -> None:
 
         self.__env[env_var].append(value)
 
-    def get(self, env_var: str) -> Any:
-        if isinstance(self.__env[env_var], list):
-            return os.pathsep.join(self.__env[env_var])
+    def get(self, env_var: str) -> Any | None:
+        """Retrieve environment variable from store.
+
+        Args:
+            env_var (str): Environment variable to retrieve.
+
+        Returns:
+            Any | None: Environment variable value. Returns None if variable is not in store.
+        """
+        try:
+            if type(self.__env[env_var]) == list:
+                return os.pathsep.join(self.__env[env_var])
+        except KeyError:
+            return None
 
     @property
     def _raw_env(self) -> Dict[str, Any]:
diff --git a/src/cleantest/provider/lxd.py b/src/cleantest/provider/lxd.py
index ce5493b..b7aee93 100644
--- a/src/cleantest/provider/lxd.py
+++ b/src/cleantest/provider/lxd.py
@@ -14,7 +14,7 @@
 from pydantic import BaseModel
 from pylxd import Client
 
-from cleantest.config.configurator import Simple
+from cleantest.control.configurator import Configure
 from cleantest.provider._base import Provider, Result
 from cleantest.provider.data import EnvDataStore, LXDDataStore
 
@@ -73,7 +73,7 @@ def __init__(
                 project=client_config.project,
             )
 
-        self._simpleconfig = Simple()
+        self._cleanconfig = Configure()
 
     def __call__(self, func: Callable) -> Callable:
         def wrapper(*args, **kwargs) -> None:
@@ -114,8 +114,9 @@ def _build(self, instances: List[Instance]) -> None:
                     tmp.start(wait=True)
 
     def _handle_start_env_hooks(self, instances: List[Instance]) -> None:
-        startenvhooks = self._simpleconfig.get_start_env_hooks()
-        for hook in startenvhooks.values():
+        startenvhooks = self._cleanconfig.get_start_env_hooks()
+        while len(startenvhooks) > 0:
+            hook = startenvhooks.pop()
             for i in instances:
                 instance = self._client.instances.get(i.name)
                 instance.execute(["apt-get", "install", "-y", "-qq", "python3-pip"])
@@ -180,9 +181,13 @@ def _execute(self, test: str, instances: List[Instance]) -> Any:
             instance = self._client.instances.get(i.name)
             instance.files.put("/root/test", test)
             instance.execute(["chmod", "+x", "/root/test"])
-            result = instance.execute(
-                ["/root/test"], environment={"PYTHONPATH": self._env.get("PYTHONPATH")}
-            )
+            if self._env.get("PYTHONPATH") is None:
+                result = instance.execute(
+                    ["/root/test"], environment={"PYTHONPATH": self._env.get("PYTHONPATH")}
+                )
+            else:
+                result = instance.execute(["/root/test"])
+
             return result
 
     def _process(self, result: Any) -> Result:
diff --git a/tests/lxd/local/test_lxd_local.py b/tests/lxd/local/test_lxd_local.py
index 81f4d77..5f0c2bb 100644
--- a/tests/lxd/local/test_lxd_local.py
+++ b/tests/lxd/local/test_lxd_local.py
@@ -6,12 +6,13 @@
 
 import os
 
-from cleantest import Simple, StartEnvHook
+from cleantest import Configure
+from cleantest.hooks import StartEnvHook
 from cleantest.provider import lxd
 
 # Define the hooks and register them.
 root = os.path.dirname(os.path.realpath(__file__))
-simpleconfig = Simple()
+simpleconfig = Configure()
 startenvhook = StartEnvHook(
     name="my_start_hook",
     requirements=os.path.join(root, "requirements.txt"),