Fix environment comparison code and add tests

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

pipenv/environments.py

@@ -14,6 +14,25 @@ from .vendor.vistir.misc import _isatty, fs_str
 # HACK: avoid resolver.py uses the wrong byte code files.
 # I hope I can remove this one day.
 os.environ["PYTHONDONTWRITEBYTECODE"] = fs_str("1")
+_false_values = ("0", "false", "no", "off")
+_true_values = ("1", "true", "yes", "on")
+
+
+def env_to_bool(val):
+    """
+    Convert **val** to boolean, returning True if truthy or False if falsey
+
+    :param Any val: The value to convert
+    :return: False if Falsey, True if truthy
+    :rtype: bool
+    """
+    if isinstance(val, bool):
+        return val
+    if val.lower() in _false_values:
+        return False
+    if val.lower() in _true_values:
+        return True
+    raise ValueError("Value is not a valid boolean-like: {0}".format(val))
 
 
 def _is_env_truthy(name):
@@ -21,7 +40,7 @@ def _is_env_truthy(name):
     """
     if name not in os.environ:
         return False
-    return os.environ.get(name).lower() not in ("0", "false", "no", "off")
+    return os.environ.get(name).lower() not in _false_values
 
 
 def get_from_env(arg, prefix="PIPENV", check_for_negation=True):
@@ -44,13 +63,17 @@ def get_from_env(arg, prefix="PIPENV", check_for_negation=True):
         positive_lookup = "{0}_{1}".format(prefix, arg)
         negative_lookup = "{0}_{1}".format(prefix, negative_lookup)
     if positive_lookup in os.environ:
-        if _is_env_truthy(positive_lookup):
-            return bool(os.environ[positive_lookup])
-        return os.environ[positive_lookup]
-    if negative_lookup in os.environ:
-        if _is_env_truthy(negative_lookup):
-            return not bool(os.environ[negative_lookup])
-        return os.environ[negative_lookup]
+        value = os.environ[positive_lookup]
+        try:
+            return env_to_bool(value)
+        except ValueError:
+            return value
+    if check_for_negation and negative_lookup in os.environ:
+        value = os.environ[negative_lookup]
+        try:
+            return not env_to_bool(value)
+        except ValueError:
+            return value
     return None
 
 

tests/unit/test_environments.py

@@ -0,0 +1,68 @@
+import itertools
+import pytest
+import os
+from pipenv import environments
+from pipenv.utils import temp_environ
+
+
+@pytest.mark.environments
+@pytest.mark.parametrize(
+    "arg, prefix, use_negation",
+    list(itertools.product(("ENABLE_SOMETHING",), ("FAKEPREFIX", None), (True, False))),
+)
+def test_get_from_env(arg, prefix, use_negation):
+    negated_arg = "NO_{0}".format(arg)
+    positive_var = arg
+    negative_var = negated_arg
+    if prefix:
+        negative_var = "{0}_{1}".format(prefix, negative_var)
+        positive_var = "{0}_{1}".format(prefix, positive_var)
+    # set the positive first
+    for var_to_set, opposite_var in ((arg, negated_arg), (negated_arg, arg)):
+        os.environ.pop(var_to_set, None)
+        os.environ.pop(opposite_var, None)
+        with temp_environ():
+            is_positive = var_to_set == arg
+            is_negative = not is_positive
+            envvar = positive_var if is_positive else negative_var
+            os.environ[envvar] = "true"
+            main_expected_value = True if is_positive else None
+            if use_negation and not is_positive:
+                main_expected_value = False
+            # use negation means if the normal variable isnt set we will check
+            # for the negated version
+            negative_expected_value = (
+                True if is_negative else None
+            )
+            if is_positive:
+                assert (
+                    environments.get_from_env(
+                        var_to_set, prefix, check_for_negation=use_negation
+                    )
+                    is main_expected_value
+                )
+                assert (
+                    environments.get_from_env(
+                        opposite_var, prefix, check_for_negation=use_negation
+                    )
+                    is negative_expected_value
+                )
+            else:
+                # var_to_set = negative version i.e. NO_xxxx
+                # opposite_var = positive_version i.e. XXXX
+
+                # get NO_BLAH -- expecting this to be True
+                assert (
+                    environments.get_from_env(
+                        var_to_set, prefix, check_for_negation=use_negation
+                    )
+                    is negative_expected_value
+                )
+                # get BLAH -- expecting False if checking for negation
+                # but otherwise should be None
+                assert (
+                    environments.get_from_env(
+                        opposite_var, prefix, check_for_negation=use_negation
+                    )
+                    is main_expected_value
+                )