Repository: capitalone/edgetest (language: python)

Issue:
Create a new plugin hook for Upgrading packages
So currently the packages are always upgraded via pip [here](https://github.com/capitalone/edgetest/blob/dev/edgetest/core.py#L122-L134).

Proposal is to make this a hook, something like: 

```python
@hookspec(firstresult=True)
def run_update(python_path: str, upgrade: List):
    """Update packages from upgrade list.

    Parameters
    ----------
    python_path : str
        The path to the python executable.
    upgrade : list
        The list of packages to upgrade
   
    Raises
    ------
    RuntimeError
        Error raised if the packages cannot be updated.
    """
```

This directly links back to a request in the conda plugin: https://github.com/capitalone/edgetest-conda/issues/30
Enabling this functionality will enable the plugin to provide a spec to upgrade via conda.




Discussion / hints:

Fix patch:
diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst
index 34070ba..cce65d9 100644
--- a/docs/source/plugins.rst
+++ b/docs/source/plugins.rst
@@ -21,6 +21,11 @@ framework that allows for additional functionality. We expose the following hook
 +---------------------------------------------------------+----------------------------------------------+
 | :py:meth:`edgetest.hookspecs.create_environment` [#f1]_ | This hook creates the virtual environment.   |
 +---------------------------------------------------------+----------------------------------------------+
+| :py:meth:`edgetest.hookspecs.run_update` [#f1]_         | | This hook runs the environment update.     |
+|                                                         | | As an example, you could implement updates |
+|                                                         | | via `pip`, `conda`, or some other package  |
+|                                                         | | manager.                                   |
++---------------------------------------------------------+----------------------------------------------+
 | :py:meth:`edgetest.hookspecs.post_run_hook`             | | This hook executes code after the testing  |
 |                                                         | | has completed. Commonly used for creating  |
 |                                                         | | notifications.                             |


Test patch:
diff --git a/edgetest/core.py b/edgetest/core.py
index 0e44d1c..f427d13 100644
--- a/edgetest/core.py
+++ b/edgetest/core.py
@@ -123,13 +123,8 @@ def setup(
         LOG.info(
             f"Upgrading the following packages in {self.envname}: {', '.join(self.upgrade)}"
         )
-        _run_command(
-            self.python_path,
-            "-m",
-            "pip",
-            "install",
-            *self.upgrade,
-            "--upgrade",
+        self.hook.run_update(
+            basedir=self._basedir, envname=self.envname, upgrade=self.upgrade
         )
         LOG.info(f"Successfully upgraded packages in {self.envname}")
 
diff --git a/edgetest/hookspecs.py b/edgetest/hookspecs.py
index 85aa91b..cb47a08 100644
--- a/edgetest/hookspecs.py
+++ b/edgetest/hookspecs.py
@@ -71,6 +71,26 @@ def create_environment(basedir: str, envname: str, conf: Dict):
     """
 
 
+@hookspec(firstresult=True)
+def run_update(basedir: str, envname: str, upgrade: List):
+    """Update packages from upgrade list.
+
+    Parameters
+    ----------
+    basedir : str
+        The base directory location for the environment.
+    envname : str
+        The name of the virtual environment.
+    upgrade : list
+        The list of packages to upgrade
+
+    Raises
+    ------
+    RuntimeError
+        Error raised if the packages cannot be updated.
+    """
+
+
 @hookspec
 def post_run_hook(testers: List, conf: Dict):
     """Post testing hook.
diff --git a/edgetest/lib.py b/edgetest/lib.py
index 9ac79a6..e2b8d1b 100644
--- a/edgetest/lib.py
+++ b/edgetest/lib.py
@@ -2,11 +2,13 @@
 
 import platform
 from pathlib import Path
-from typing import Dict
+from typing import Dict, List
 from venv import EnvBuilder
 
 import pluggy
 
+from .utils import _run_command
+
 hookimpl = pluggy.HookimplMarker("edgetest")
 
 
@@ -44,3 +46,35 @@ def create_environment(basedir: str, envname: str, conf: Dict):
         builder.create(env_dir=Path(basedir, envname))
     except Exception:
         raise RuntimeError(f"Unable to create {envname} in {basedir}")
+
+
+@hookimpl(trylast=True)
+def run_update(basedir: str, envname: str, upgrade: List):
+    """Update packages from upgrade list.
+
+    Parameters
+    ----------
+    basedir : str
+        The base directory location for the environment.
+    envname : str
+        The name of the virtual environment.
+    upgrade : list
+        The list of packages to upgrade
+
+    Raises
+    ------
+    RuntimeError
+        Error raised if the packages cannot be updated.
+    """
+    python_path = path_to_python(basedir, envname)
+    try:
+        _run_command(
+            python_path,
+            "-m",
+            "pip",
+            "install",
+            *upgrade,
+            "--upgrade",
+        )
+    except Exception:
+        raise RuntimeError(f"Unable to pip upgrade: {upgrade}")
diff --git a/tests/conftest.py b/tests/conftest.py
index 86a179a..e7666c4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2,7 +2,7 @@
 
 import platform
 from pathlib import Path
-from typing import Dict
+from typing import Dict, List
 
 import pluggy
 import pytest
@@ -29,6 +29,11 @@ def create_environment(self, basedir: str, envname: str, conf: Dict):
         """Create the virtual environment for testing."""
         pass
 
+    @hookimpl
+    def run_update(self, basedir: str, envname: str, upgrade: List):
+        """Update packages from upgrade list."""
+        pass
+
 
 @pytest.fixture
 def plugin_manager():
diff --git a/tests/test_core.py b/tests/test_core.py
index 5c3d3d7..6c9805a 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -38,18 +38,6 @@ def test_basic_setup(mock_popen, mock_path, tmpdir, plugin_manager):
             stdout=-1,
             universal_newlines=True,
         ),
-        call(
-            (
-                f"{str(py_loc)}",
-                "-m",
-                "pip",
-                "install",
-                "myupgrade",
-                "--upgrade",
-            ),
-            stdout=-1,
-            universal_newlines=True,
-        ),
     ]
 
 
@@ -101,11 +89,6 @@ def test_setup_extras(mock_popen, mock_path, tmpdir, plugin_manager):
             stdout=-1,
             universal_newlines=True,
         ),
-        call(
-            (f"{py_loc}", "-m", "pip", "install", "myupgrade", "--upgrade"),
-            stdout=-1,
-            universal_newlines=True,
-        ),
     ]
 
 
@@ -152,11 +135,6 @@ def test_setup_pip_deps(mock_popen, mock_path, tmpdir, plugin_manager):
             stdout=-1,
             universal_newlines=True,
         ),
-        call(
-            (f"{py_loc}", "-m", "pip", "install", "myupgrade", "--upgrade"),
-            stdout=-1,
-            universal_newlines=True,
-        ),
     ]
 
 
diff --git a/tests/test_lib.py b/tests/test_lib.py
new file mode 100644
index 0000000..2a6426a
--- /dev/null
+++ b/tests/test_lib.py
@@ -0,0 +1,47 @@
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from edgetest.lib import create_environment, path_to_python, run_update
+
+
+@patch("edgetest.lib.platform", autospec=True)
+def test_path_to_python(mock_platform):
+    mock_platform.system.return_value = "Windows"
+    assert path_to_python("test", "test") == str(
+        Path("test") / "test" / "Scripts" / "python"
+    )
+
+    mock_platform.system.return_value = "Unix"
+    assert path_to_python("test", "test") == str(
+        Path("test") / "test" / "bin" / "python"
+    )
+
+    mock_platform.system.side_effect = RuntimeError()
+    with pytest.raises(RuntimeError):
+        path_to_python("test", "test")
+
+
+@patch("edgetest.lib.EnvBuilder", autospec=True)
+def test_create_environment(mock_env_builder):
+    create_environment("test", "test", {})
+    mock_env_builder.assert_called_with(with_pip=True)
+    mock_env_builder().create.assert_called_with(env_dir=Path("test", "test"))
+
+    mock_env_builder().create.side_effect = RuntimeError()
+    with pytest.raises(RuntimeError):
+        create_environment("test", "test", {})
+
+
+@patch("edgetest.lib._run_command", autospec=True)
+def test_run_update(mock_run):
+    python_path = path_to_python("test", "test")
+    run_update("test", "test", ["1", "2"])
+    mock_run.assert_called_with(
+        python_path, "-m", "pip", "install", "1", "2", "--upgrade"
+    )
+
+    mock_run.side_effect = RuntimeError()
+    with pytest.raises(RuntimeError):
+        run_update("test", "test", ["1", "2"])