#595 fix by updating implementation of #515

tests/test_config.py

@@ -444,18 +444,38 @@ 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")
+    def test_normal_env_sub_works(self, monkeypatch, newconfig):
+        monkeypatch.setenv("VAR", "hello")
         config = newconfig("""
             [section]
-            key1={env:KEY1}
-            key2={env:KEY2}
+            key={env:VAR}
+        """)
+        assert SectionReader("section", config._cfg).getstring("key") == "hello"
+
+    def test_missing_env_sub_crashes_early_in_non_testenv(self, newconfig):
+        config = newconfig("""
+            [section]
+            key={env:VAR}
         """)
-        reader = SectionReader("section", config._cfg)
-        x = reader.getstring("key1")
-        assert x == "hello"
         with pytest.raises(tox.exception.ConfigError):
-            reader.getstring("key2")
+            SectionReader("section", config._cfg).getstring("key")
+
+    def test_missing_env_sub_does_not_crash_in_testenv(self, newconfig):
+        class fakematch:
+            @staticmethod
+            def group(key):
+                if key == 'substitution_value':
+                    return 'VAR'
+                return None
+
+        config = newconfig("""
+            [testenv:foo]
+            key={env:VAR}
+        """)
+        reader = SectionReader("testenv:foo", config._cfg)
+        replacer = tox.config.Replacer(reader)
+        res = replacer._replace_env(fakematch)
+        assert res == 'TOX_MISSING_ENV_SUBSTITUTION'
 
     def test_getstring_environment_substitution_with_default(self, monkeypatch, newconfig):
         monkeypatch.setenv("KEY1", "hello")

tox/__init__.py

@@ -1,3 +1,5 @@
+from collections import defaultdict
+
 from pkg_resources import get_distribution, DistributionNotFound
 
 from .hookspecs import hookspec, hookimpl  # noqa
@@ -35,4 +37,13 @@ def __init__(self, message):
             self.message = message
             super(exception.MinVersionError, self).__init__(message)
 
+
+missing_env_substitution_map = defaultdict(list)  # FIXME - UGLY HACK
+"""Map section name to env variables that would be needed in that section but are not provided.
+
+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.
+"""
+
 from tox.session import main as cmdline  # noqa

tox/config.py

@@ -791,9 +791,8 @@ 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, replace=True)
 
         all_develop = all(name in config.envconfigs
                           and config.envconfigs[name].usedevelop
@@ -1041,10 +1040,16 @@ 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:
-            assert self._subststack.pop() == (section_name, name)
+        replaced = Replacer(self, crossonly=crossonly).do_replace(value)
+        assert self._subststack.pop() == (section_name, name)
+        if replaced == 'TOX_MISSING_ENV_SUBSTITUTION':
+            if ':' not in section_name:
+                raise tox.exception.ConfigError(
+                    "substitution env:%r: unknown or recursive definition in "
+                    "section %r." % (value, section_name))
+            envname = section_name.split(':')[-1]
+            tox.missing_env_substitution_map[envname].append(value)
+        return replaced
 
 
 class Replacer:
@@ -1112,20 +1117,16 @@ 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
+        # Don't crash yet, because this is only a problem if the tox environment
+        # is part of the current run
+        return 'TOX_MISSING_ENV_SUBSTITUTION'
 
     def _substitute_from_other_section(self, key):
         if key.startswith("[") and "]" in key:

tox/session.py

@@ -550,6 +550,12 @@ def subcommand_test(self):
             return
         for venv in self.venvlist:
             if self.setupenv(venv):
+                missing_vars = tox.missing_env_substitution_map.get(venv.name)
+                if missing_vars:
+                    raise tox.exception.ConfigError(
+                        "%s contains unresolvable substitution(s): %s. "
+                        "Environment variables are missing or defined recursively." %
+                        (venv.name, missing_vars))
                 if venv.envconfig.usedevelop:
                     self.developpkg(venv, self.config.setupdir)
                 elif self.config.skipsdist or venv.envconfig.skip_install: