Fix substitution problems introduced in 2.8

tests/test_config.py

@@ -353,7 +353,7 @@ def test_regression_issue595(self, newconfig):
             [testenv:bar]
             setenv = {[testenv]setenv}
             [testenv:baz]
-            setenv = 
+            setenv =
         """)
         assert config.envconfigs['foo'].setenv['VAR'] == 'x'
         assert config.envconfigs['bar'].setenv['VAR'] == 'x'
@@ -444,18 +444,20 @@ def test_getdict(self, tmpdir, newconfig):
         x = reader.getdict("key3", {1: 2})
         assert x == {1: 2}
 
-    def test_getstring_environment_substitution(self, monkeypatch, newconfig):
-        monkeypatch.setenv("KEY1", "hello")
-        config = newconfig("""
-            [section]
-            key1={env:KEY1}
-            key2={env:KEY2}
-        """)
-        reader = SectionReader("section", config._cfg)
-        x = reader.getstring("key1")
-        assert x == "hello"
+    def test_normal_env_sub_works(self, monkeypatch, newconfig):
+        monkeypatch.setenv("VAR", "hello")
+        config = newconfig("[section]\nkey={env:VAR}")
+        assert SectionReader("section", config._cfg).getstring("key") == "hello"
+
+    def test_missing_env_sub_raises_config_error_in_non_testenv(self, newconfig):
+        config = newconfig("[section]\nkey={env:VAR}")
         with pytest.raises(tox.exception.ConfigError):
-            reader.getstring("key2")
+            SectionReader("section", config._cfg).getstring("key")
+
+    def test_missing_env_sub_populates_missing_subs(self, newconfig):
+        config = newconfig("[testenv:foo]\ncommands={env:VAR}")
+        print(SectionReader("section", config._cfg).getstring("commands"))
+        assert config.envconfigs['foo'].missing_subs == ['VAR']
 
     def test_getstring_environment_substitution_with_default(self, monkeypatch, newconfig):
         monkeypatch.setenv("KEY1", "hello")

tests/test_z_cmdline.py

@@ -871,3 +871,10 @@ def test_envtmpdir(initproj, cmd):
 
     result = cmd.run("tox")
     assert not result.ret
+
+
+def test_missing_env_fails(initproj, cmd):
+    initproj("foo", filedefs={'tox.ini': "[testenv:foo]\ncommands={env:VAR}"})
+    result = cmd.run("tox")
+    assert result.ret == 1
+    result.stdout.fnmatch_lines(["*foo: unresolvable substitution(s): 'VAR'*"])

tox/__init__.py

@@ -14,12 +14,18 @@ class Error(Exception):
         def __str__(self):
             return "%s: %s" % (self.__class__.__name__, self.args[0])
 
+    class MissingSubstitution(Exception):
+        FLAG = 'TOX_MISSING_SUBSTITUTION'
+        """placeholder for debugging configurations"""
+        def __init__(self, name):
+            self.name = name
+
     class ConfigError(Error):
         """ error in tox configuration. """
     class UnsupportedInterpreter(Error):
-        "signals an unsupported Interpreter"
+        """signals an unsupported Interpreter"""
     class InterpreterNotFound(Error):
-        "signals that an interpreter could not be found"
+        """signals that an interpreter could not be found"""
     class InvocationError(Error):
         """ an error while invoking a script. """
     class MissingFile(Error):
@@ -35,4 +41,5 @@ def __init__(self, message):
             self.message = message
             super(exception.MinVersionError, self).__init__(message)
 
+
 from tox.session import main as cmdline  # noqa

tox/config.py

@@ -609,6 +609,13 @@ def __init__(self, envname, config, factors, reader):
         #: set of factors
         self.factors = factors
         self._reader = reader
+        self.missing_subs = []
+        """Holds substitutions that could not be resolved.
+
+        Pre 2.8.1 missing substitutions crashed with a ConfigError although this would not be a
+        problem if the env is not part of the current testrun. So we need to remember this and
+        check later when the testenv is actually run and crash only then.
+        """
 
     def get_envbindir(self):
         """ path to directory where scripts/binaries reside. """
@@ -791,9 +798,7 @@ def __init__(self, config, inipath):
             section = testenvprefix + name
             factors = set(name.split('-'))
             if section in self._cfg or factors <= known_factors:
-                config.envconfigs[name] = \
-                    self.make_envconfig(name, section, reader._subs, config,
-                                        replace=name in config.envlist)
+                config.envconfigs[name] = self.make_envconfig(name, section, reader._subs, config)
 
         all_develop = all(name in config.envconfigs
                           and config.envconfigs[name].usedevelop
@@ -813,33 +818,31 @@ def make_envconfig(self, name, section, subs, config, replace=True):
         factors = set(name.split('-'))
         reader = SectionReader(section, self._cfg, fallbacksections=["testenv"],
                                factors=factors)
-        vc = TestenvConfig(config=config, envname=name, factors=factors, reader=reader)
-        reader.addsubstitutions(**subs)
-        reader.addsubstitutions(envname=name)
-        reader.addsubstitutions(envbindir=vc.get_envbindir,
-                                envsitepackagesdir=vc.get_envsitepackagesdir,
-                                envpython=vc.get_envpython)
-
+        tc = TestenvConfig(name, config, factors, reader)
+        reader.addsubstitutions(
+            envname=name, envbindir=tc.get_envbindir, envsitepackagesdir=tc.get_envsitepackagesdir,
+            envpython=tc.get_envpython, **subs)
         for env_attr in config._testenv_attr:
             atype = env_attr.type
-            if atype in ("bool", "path", "string", "dict", "dict_setenv", "argv", "argvlist"):
-                meth = getattr(reader, "get" + atype)
-                res = meth(env_attr.name, env_attr.default, replace=replace)
-            elif atype == "space-separated-list":
-                res = reader.getlist(env_attr.name, sep=" ")
-            elif atype == "line-list":
-                res = reader.getlist(env_attr.name, sep="\n")
-            else:
-                raise ValueError("unknown type %r" % (atype,))
-
-            if env_attr.postprocess:
-                res = env_attr.postprocess(testenv_config=vc, value=res)
-            setattr(vc, env_attr.name, res)
-
+            try:
+                if atype in ("bool", "path", "string", "dict", "dict_setenv", "argv", "argvlist"):
+                    meth = getattr(reader, "get" + atype)
+                    res = meth(env_attr.name, env_attr.default, replace=replace)
+                elif atype == "space-separated-list":
+                    res = reader.getlist(env_attr.name, sep=" ")
+                elif atype == "line-list":
+                    res = reader.getlist(env_attr.name, sep="\n")
+                else:
+                    raise ValueError("unknown type %r" % (atype,))
+                if env_attr.postprocess:
+                    res = env_attr.postprocess(testenv_config=tc, value=res)
+            except tox.exception.MissingSubstitution as e:
+                tc.missing_subs.append(e.name)
+                res = e.FLAG
+            setattr(tc, env_attr.name, res)
             if atype in ("path", "string"):
                 reader.addsubstitutions(**{env_attr.name: res})
-
-        return vc
+        return tc
 
     def _getenvdata(self, reader):
         envstr = self.config.option.env                                \
@@ -961,8 +964,7 @@ def getdict(self, name, default=None, sep="\n", replace=True):
 
     def getdict_setenv(self, name, default=None, sep="\n", replace=True):
         value = self.getstring(name, None, replace=replace, crossonly=True)
-        definitions = self._getdict(value, default=default, sep=sep,
-                                    replace=replace)
+        definitions = self._getdict(value, default=default, sep=sep, replace=replace)
         self._setenv = SetenvDict(definitions, reader=self)
         return self._setenv
 
@@ -1042,9 +1044,15 @@ def _replace(self, value, name=None, section_name=None, crossonly=False):
         section_name = section_name if section_name else self.section_name
         self._subststack.append((section_name, name))
         try:
-            return Replacer(self, crossonly=crossonly).do_replace(value)
-        finally:
+            replaced = Replacer(self, crossonly=crossonly).do_replace(value)
             assert self._subststack.pop() == (section_name, name)
+        except tox.exception.MissingSubstitution:
+            if not section_name.startswith(testenvprefix):
+                raise tox.exception.ConfigError(
+                    "substitution env:%r: unknown or recursive definition in "
+                    "section %r." % (value, section_name))
+            raise
+        return replaced
 
 
 class Replacer:
@@ -1112,20 +1120,14 @@ def _replace_match(self, match):
     def _replace_env(self, match):
         envkey = match.group('substitution_value')
         if not envkey:
-            raise tox.exception.ConfigError(
-                'env: requires an environment variable name')
-
+            raise tox.exception.ConfigError('env: requires an environment variable name')
         default = match.group('default_value')
-
         envvalue = self.reader.get_environ_value(envkey)
-        if envvalue is None:
-            if default is None:
-                raise tox.exception.ConfigError(
-                    "substitution env:%r: unknown environment variable %r "
-                    " or recursive definition." %
-                    (envkey, envkey))
+        if envvalue is not None:
+            return envvalue
+        if default is not None:
             return default
-        return envvalue
+        raise tox.exception.MissingSubstitution(envkey)
 
     def _substitute_from_other_section(self, key):
         if key.startswith("[") and "]" in key:

tox/session.py

@@ -440,6 +440,12 @@ def make_emptydir(self, path):
             path.ensure(dir=1)
 
     def setupenv(self, venv):
+        if venv.envconfig.missing_subs:
+            venv.status = (
+                "unresolvable substitution(s): %s. "
+                "Environment variables are missing or defined recursively." %
+                (','.join(["'%s'" % m for m in venv.envconfig.missing_subs])))
+            return
         if not venv.matching_platform():
             venv.status = "platform mismatch"
             return  # we simply omit non-matching platforms