From 1fc5b937856810cc1aa5b3469f639cd2aed9bc3c Mon Sep 17 00:00:00 2001
From: Bartolome Sanchez <bartolome.sanchez@ntt.eu>
Date: Fri, 2 Mar 2018 17:55:50 +0100
Subject: [PATCH] Fix bug with incorrectly defactorized dependencies

https://github.com/tox-dev/tox/issues/706

By performing dependencies replacement before applying factors,
since previously this function was being incorrectly called with
unrolled dependencies.
---
 changelog/706.bugfix.rst |  1 +
 doc/config.rst           | 26 ++++++++++++++++++++++++++
 tests/test_config.py     | 31 ++++++++++++++++++++++++++++++-
 tox/config.py            |  9 +++++++--
 4 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 changelog/706.bugfix.rst

diff --git a/changelog/706.bugfix.rst b/changelog/706.bugfix.rst
new file mode 100644
index 000000000..b0827db66
--- /dev/null
+++ b/changelog/706.bugfix.rst
@@ -0,0 +1 @@
+Fix bug with incorrectly defactorized dependencies - by @bartsanchez
diff --git a/doc/config.rst b/doc/config.rst
index 2e12fe50d..cf1d9ec47 100644
--- a/doc/config.rst
+++ b/doc/config.rst
@@ -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
 =====================
 
diff --git a/tests/test_config.py b/tests/test_config.py
index 26421fa50..61d106bfe 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -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 = """
diff --git a/tox/config.py b/tox/config.py
index ad72bdf30..f8cb9ff47 100755
--- a/tox/config.py
+++ b/tox/config.py
@@ -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
 
@@ -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'):
+            x = self._replace(x, name=name, crossonly=crossonly)
+        return x
+
 
 class Replacer:
     RE_ITEM_REF = re.compile(