From da3a86ace5b34e85dc29c49e84956d37763494a8 Mon Sep 17 00:00:00 2001 From: Oliver Bestwalter Date: Sat, 2 Sep 2017 19:21:33 +0200 Subject: [PATCH 1/2] #595 fix by updating implementation of #515 The way #515 was implemented did not play nicely with env var substitution. This might render the former additions completely or partially obsolete but I did not check this yet. First let's fix that bug. --- tests/test_config.py | 36 ++++++++++++++++++++++++++++-------- tox/__init__.py | 11 +++++++++++ tox/config.py | 37 +++++++++++++++++++------------------ tox/session.py | 6 ++++++ 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 67da2e1cba..0f684621e1 100644 --- a/tests/test_config.py +++ b/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") diff --git a/tox/__init__.py b/tox/__init__.py index 73d095eb1b..b80533fd66 100644 --- a/tox/__init__.py +++ b/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 diff --git a/tox/config.py b/tox/config.py index 1fba542509..57b5aac614 100755 --- a/tox/config.py +++ b/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: diff --git a/tox/session.py b/tox/session.py index 269a770575..7e39d87fee 100644 --- a/tox/session.py +++ b/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: From 247f346628f92df2d9e998a90e7195c0aa95f6f1 Mon Sep 17 00:00:00 2001 From: Oliver Bestwalter Date: Sat, 2 Sep 2017 21:38:38 +0200 Subject: [PATCH 2/2] fix flakes (cherry picked from commit e902f04) --- tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index 0f684621e1..49cfc785bf 100644 --- a/tests/test_config.py +++ b/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'