Fix bug in a unit test (#8314)

Fix bug in a unit test

packages/tool_util/tests/util.py

@@ -0,0 +1 @@
+../../../test/unit/tools/util.py

test/unit/tools/test_tool_deps.py

@@ -3,7 +3,6 @@ import tempfile
 from contextlib import contextmanager
 from os import (
     chmod,
-    environ,
     makedirs,
     stat,
     symlink,
@@ -22,6 +21,7 @@ from galaxy.tool_util.deps.resolvers.galaxy_packages import GalaxyPackageDepende
 from galaxy.tool_util.deps.resolvers.lmod import LmodDependency, LmodDependencyResolver
 from galaxy.tool_util.deps.resolvers.modules import ModuleDependency, ModuleDependencyResolver
 from galaxy.util.bunch import Bunch
+from .util import modify_environ
 
 # If true, test created DependencyManager objects by serializing out to json and re-constituting.
 ROUND_TRIP_TEST_DEPENDENCY_MANAGER_SERIALIZATION = True
@@ -582,7 +582,7 @@ def test_config_modulepath():
 
 def test_config_MODULEPATH():
     # Test reads and splits MODULEPATH if modulepath is not specified.
-    with __environ({"MODULEPATH": "/opt/modules/modulefiles:/usr/local/modules/modulefiles"}):
+    with modify_environ({"MODULEPATH": "/opt/modules/modulefiles:/usr/local/modules/modulefiles"}):
         with __parse_resolvers('''<dependency_resolvers>
   <modules find_by="directory" />
 </dependency_resolvers>
@@ -593,7 +593,7 @@ def test_config_MODULEPATH():
 def test_config_MODULESHOME():
     # Test fallbacks to read MODULESHOME if modulepath is not specified and
     # neither is MODULEPATH.
-    with __environ({"MODULESHOME": "/opt/modules"}, remove="MODULEPATH"):
+    with modify_environ({"MODULESHOME": "/opt/modules"}, keys_to_remove=["MODULEPATH"]):
         with __parse_resolvers('''<dependency_resolvers>
   <modules find_by="directory" />
 </dependency_resolvers>
@@ -718,28 +718,6 @@ def _first_conda_resolver_options(dm):
     return [r for r in dm.to_dict()["resolvers"] if r["resolver_type"] == "conda"][0]
 
 
-@contextmanager
-def __environ(values, remove=[]):
-    """
-    Modify the environment for a test, adding/updating values in dict `values` and
-    removing any environment variables mentioned in list `remove`.
-    """
-    new_keys = set(environ.keys()) - set(values.keys())
-    old_environ = environ.copy()
-    try:
-        environ.update(values)
-        for to_remove in remove:
-            try:
-                del environ[remove]
-            except KeyError:
-                pass
-        yield
-    finally:
-        environ.update(old_environ)
-        for key in new_keys:
-            del environ[key]
-
-
 @contextmanager
 def __parse_resolvers(file_content, extension=".xml"):
     with __dependency_manager(file_content, extension=extension) as dm:

test/unit/tools/test_util.py

@@ -0,0 +1,86 @@
+from os import environ
+
+import pytest
+
+from .util import modify_environ
+
+
+@pytest.fixture
+def load_keyval(request):
+    """
+    Create key/value pair and load it into os.environ. Delete on teardown.
+    """
+    keys = []  # preserve keys for teardown
+
+    def _load_keyval(key='a unique key', val='a value'):
+        # If this is called twice with default values within the same test function,
+        #   it will raise a KeyError. This is intentional: os.environ cannot have duplicate keys.
+        keys.append(key)
+        environ[key] = val
+        return key, val
+
+    def _teardown():
+        for k in keys:
+            del environ[k]
+
+    request.addfinalizer(_teardown)
+    return _load_keyval
+
+
+def test_modify_environ__restore(load_keyval):
+    key, val = load_keyval()
+    with modify_environ({}):
+        assert environ[key] == val  # key/val unchanged
+    assert environ[key] == val  # key/val unchanged
+
+
+def test_modify_environ__add_and_restore(load_keyval):
+    key1, val1 = load_keyval()
+    key2, val2 = 'key to add', 'value to add'
+    to_update = {key2: val2}
+
+    assert key2 not in environ  # ensure key to add does not exist
+    with modify_environ(to_update):
+        assert environ[key1] == val1  # key/val unchanged
+        assert environ[key2] == val2  # new key/val added
+    assert environ[key1] == val1  # key/val unchanged
+    assert key2 not in environ   # new key removed
+
+
+def test_modify_environ__update_and_restore(load_keyval):
+    key1, val1 = load_keyval()
+    key2, val2 = load_keyval('key to update', 'value to update')
+    val2_updated = 'updated'
+    to_update = {key2: val2_updated}
+
+    with modify_environ(to_update):
+        assert environ[key1] == val1  # key/val unchanged
+        assert environ[key2] == val2_updated  # value updated
+    assert environ[key1] == val1  # key/val unchanged
+    assert environ[key2] == val2  # value restored
+
+
+def test_modify_environ__remove_and_restore(load_keyval):
+    key1, val1 = load_keyval()
+    key2, val2 = load_keyval('key to remove', 'value to remove')
+    to_update = {}
+    to_remove = [key2]
+
+    with modify_environ(to_update, to_remove):
+        assert environ[key1] == val1  # key/val unchanged
+        assert key2 not in environ  # key removed
+    assert environ[key1] == val1  # key/val unchanged
+    assert environ[key2] == val2  # key/value restored
+
+
+def test_modify_environ__remove_nonexistant_key(load_keyval):
+    # Test that removing wrong key does not raise an error
+    key1, val1 = load_keyval()
+    key_nonexistant = 'no such key'
+    to_update = {}
+    to_remove = [key_nonexistant]
+
+    assert key_nonexistant not in environ  # ensure key to remove does not exist
+    with modify_environ(to_update, to_remove):
+        assert environ[key1] == val1  # key/val unchanged
+    assert environ[key1] == val1  # key/val unchanged

test/unit/tools/util.py

@@ -0,0 +1,22 @@
+from contextlib import contextmanager
+from os import environ
+
+
+@contextmanager
+def modify_environ(values, keys_to_remove=None):
+    """
+    Modify the environment for a test, adding/updating values in dict `values` and
+    removing any environment variables mentioned in list `keys_to_remove`.
+    """
+    old_environ = environ.copy()
+    try:
+        if values:
+            environ.update(values)
+        if keys_to_remove:
+            for key in keys_to_remove:
+                if key in environ:
+                    del environ[key]
+        yield
+    finally:
+        environ.clear()
+        environ.update(old_environ)