Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with incorrectly defactorized dependencies #772

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/706.bugfix.rst
@@ -0,0 +1 @@
Fix bug with incorrectly defactorized dependencies - by @bartsanchez
26 changes: 26 additions & 0 deletions doc/config.rst
Expand Up @@ -674,6 +674,32 @@ the following:
- but not ``py2``, ``py36-sql`` or ``py36-mysql-dev``.


Factors and values substitution are compatible
++++++++++++++++++++++++++++++++++++++++++++++

It is possible to mix both values substitution and factor expressions.
For example::

[tox]
envlist = py27,py36,coverage

[testenv]
deps =
flake8
coverage: coverage

[testenv:py27]
deps =
{{[testenv]deps}}
pytest

With the previous configuration, it will install:

- ``flake8`` and ``pytest`` packages for ``py27`` environment.
- ``flake8`` package for ``py36`` environment.
- ``flake8`` and ``coverage`` packages for ``coverage`` environment.


Other Rules and notes
=====================

Expand Down
31 changes: 30 additions & 1 deletion tests/test_config.py
Expand Up @@ -1294,7 +1294,36 @@ def test_take_dependencies_from_other_testenv(
)
conf = newconfig([], inisource).envconfigs['py27']
packages = [dep.name for dep in conf.deps]
assert packages == list(deps) + ['fun', 'frob>1.0,<2.0']
assert packages == ['pytest', 'pytest-cov', 'fun', 'frob>1.0,<2.0']

# https://github.com/tox-dev/tox/issues/706
@pytest.mark.parametrize('envlist', [['py27', 'coverage', 'other']])
def test_regression_test_issue_706(self, newconfig, envlist):
inisource = """
[tox]
envlist = {envlist}
[testenv]
deps=
flake8
coverage: coverage
[testenv:py27]
deps=
{{[testenv]deps}}
fun
""".format(
envlist=','.join(envlist),
)
conf = newconfig([], inisource).envconfigs['coverage']
packages = [dep.name for dep in conf.deps]
assert packages == ['flake8', 'coverage']

conf = newconfig([], inisource).envconfigs['other']
packages = [dep.name for dep in conf.deps]
assert packages == ['flake8']

conf = newconfig([], inisource).envconfigs['py27']
packages = [dep.name for dep in conf.deps]
assert packages == ['flake8', 'fun']

def test_take_dependencies_from_other_section(self, newconfig):
inisource = """
Expand Down
9 changes: 7 additions & 2 deletions tox/config.py
Expand Up @@ -1076,10 +1076,10 @@ def getstring(self, name, default=None, replace=True, crossonly=False):
if x is None:
x = default
else:
x = self._replace_if_needed(x, name, replace, crossonly)
x = self._apply_factors(x)

if replace and x and hasattr(x, 'replace'):
x = self._replace(x, name=name, crossonly=crossonly)
x = self._replace_if_needed(x, name, replace, crossonly)
# print "getstring", self.section_name, name, "returned", repr(x)
return x

Expand Down Expand Up @@ -1115,6 +1115,11 @@ def _replace(self, value, name=None, section_name=None, crossonly=False):
raise
return replaced

def _replace_if_needed(self, x, name, replace, crossonly):
if replace and x and hasattr(x, 'replace'):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this check for the _replace attr?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, does not help.

x = self._replace(x, name=name, crossonly=crossonly)
return x


class Replacer:
RE_ITEM_REF = re.compile(
Expand Down