Fix bug with incorrectly defactorized dependencies #706 (#1058)

New version with improved changes, fixing errors raised with the previous version:

https://github.com/tox-dev/tox/issues/899
https://github.com/tox-dev/tox/issues/906

4 files changed, 259 insertions, 4 deletions

diff --git a/docs/changelog/706.bugfix.rst b/docs/changelog/706.bugfix.rst
new file mode 100644
index 00000000..b0827db6
--- /dev/null
+++ b/docs/changelog/706.bugfix.rst
@@ -0,0 +1 @@
+Fix bug with incorrectly defactorized dependencies - by @bartsanchez
diff --git a/docs/config.rst b/docs/config.rst
index c13cd6d3..748c7b59 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -834,6 +834,31 @@ the following:
       ``mysql-py36``,
     - 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.
+
 Advanced settings
 -----------------
 
diff --git a/src/tox/config.py b/src/tox/config.py
index 32304299..b499a8aa 100755
--- a/src/tox/config.py
+++ b/src/tox/config.py
@@ -1336,11 +1336,20 @@ class SectionReader:
         if x is None:
             x = default
         else:
+            # It is needed to apply factors before unwrapping
+            # dependencies, otherwise it can break the substitution
+            # process. Once they are unwrapped, we call apply factors
+            # again for those new dependencies.
             x = self._apply_factors(x)
+            x = self._replace_if_needed(x, name, replace, crossonly)
+            x = self._apply_factors(x)
+
+        x = self._replace_if_needed(x, name, replace, crossonly)
+        return x
 
+    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)
-        # print "getstring", self.section_name, name, "returned", repr(x)
         return x
 
     def _apply_factors(self, s):
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 9d88dd63..86f36319 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -1444,11 +1444,9 @@ class TestConfigTestEnv:
         )
         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"]
+        assert packages == ["pytest", "pytest-cov", "fun", "frob>1.0,<2.0"]
 
     # https://github.com/tox-dev/tox/issues/706
-    @pytest.mark.xfail(reason="reproduce bug 706")
     @pytest.mark.parametrize("envlist", [["py27", "coverage", "other"]])
     def test_regression_test_issue_706(self, newconfig, envlist):
         inisource = """
@@ -1477,6 +1475,228 @@ class TestConfigTestEnv:
         packages = [dep.name for dep in conf.deps]
         assert packages == ["flake8", "fun"]
 
+    def test_factor_expansion(self, newconfig):
+        inisource = """
+            [tox]
+            envlist = {py27, py37}-cover
+            [testenv]
+            deps=
+              {py27}: foo
+              {py37}: bar
+        """
+        conf = newconfig([], inisource).envconfigs["py27-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["foo"]
+
+        conf = newconfig([], inisource).envconfigs["py37-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["bar"]
+
+    # Regression test https://github.com/tox-dev/tox/issues/899
+    def test_factors_support_curly_braces(self, newconfig):
+        inisource = """
+            [tox]
+            envlist =
+                style
+                sdist
+                bdist_wheel
+                {py27,py34,py35,py36,pypy,pypy3}-cover
+                {py27,py34,py35,py36,pypy,pypy3}-nocov
+
+            [testenv]
+            deps =
+                cover: coverage
+                cover: codecov
+                {py27}: unittest2
+                {py27}: mysql-python
+                {py27,py36}: mmtf-python
+                {py27,py35}: reportlab
+                {py27,py34,py35,py36}: psycopg2-binary
+                {py27,py34,py35,py35}: mysql-connector-python-rf
+                {py27,py35,pypy}: rdflib
+                {pypy,pypy3}: numpy==1.12.1
+                {py27,py34,py36}: numpy
+                {py36}: scipy
+                {py27}: networkx
+                {py36}: matplotlib
+        """
+        conf = newconfig([], inisource).envconfigs["style"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == []
+
+        conf = newconfig([], inisource).envconfigs["py27-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            "coverage",
+            "codecov",
+            "unittest2",
+            "mysql-python",
+            "mmtf-python",
+            "reportlab",
+            "psycopg2-binary",
+            "mysql-connector-python-rf",
+            "rdflib",
+            "numpy",
+            "networkx",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["py34-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            "coverage",
+            "codecov",
+            "psycopg2-binary",
+            "mysql-connector-python-rf",
+            "numpy",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["py35-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            "coverage",
+            "codecov",
+            "reportlab",
+            "psycopg2-binary",
+            "mysql-connector-python-rf",
+            "rdflib",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["py36-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            "coverage",
+            "codecov",
+            "mmtf-python",
+            "psycopg2-binary",
+            "numpy",
+            "scipy",
+            "matplotlib",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["pypy-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["coverage", "codecov", "rdflib", "numpy==1.12.1"]
+
+        conf = newconfig([], inisource).envconfigs["pypy3-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["coverage", "codecov", "numpy==1.12.1"]
+
+        conf = newconfig([], inisource).envconfigs["py27-nocov"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            "unittest2",
+            "mysql-python",
+            "mmtf-python",
+            "reportlab",
+            "psycopg2-binary",
+            "mysql-connector-python-rf",
+            "rdflib",
+            "numpy",
+            "networkx",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["py34-nocov"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["psycopg2-binary", "mysql-connector-python-rf", "numpy"]
+
+        conf = newconfig([], inisource).envconfigs["py35-nocov"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["reportlab", "psycopg2-binary", "mysql-connector-python-rf", "rdflib"]
+
+        conf = newconfig([], inisource).envconfigs["py36-nocov"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["mmtf-python", "psycopg2-binary", "numpy", "scipy", "matplotlib"]
+
+        conf = newconfig([], inisource).envconfigs["pypy-nocov"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["rdflib", "numpy==1.12.1"]
+
+        conf = newconfig([], inisource).envconfigs["pypy3-cover"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == ["coverage", "codecov", "numpy==1.12.1"]
+
+    # Regression test https://github.com/tox-dev/tox/issues/906
+    def test_do_not_substitute_more_than_needed(self, newconfig):
+        inisource = """
+            [tox]
+            envlist =
+                django_master-py{36,35}
+                django20-py{36,35,34,py3}
+                django111-py{36,35,34,27,py}
+                django18-py{35,34,27,py}
+                lint
+                docs
+
+            [testenv]
+            deps =
+                .[test]
+                django18: {[django]1.8.x}
+                django111: {[django]1.11.x}
+                django20: {[django]2.0.x}
+                django_master: {[django]master}
+
+            [django]
+            1.8.x  =
+                   Django>=1.8.0,<1.9.0
+                   django-reversion==1.10.0
+                   djangorestframework>=3.3.3,<3.7.0
+            1.11.x  =
+                   Django>=1.11.0,<2.0.0
+                   django-reversion>=2.0.8
+                   djangorestframework>=3.6.2
+            2.0.x  =
+                   Django>=2.0,<2.1
+                   django-reversion>=2.0.8
+                   djangorestframework>=3.7.3
+            master =
+                   https://github.com/django/django/tarball/master
+                   django-reversion>=2.0.8
+                   djangorestframework>=3.6.2
+        """
+        conf = newconfig([], inisource).envconfigs["django_master-py36"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            ".[test]",
+            "https://github.com/django/django/tarball/master",
+            "django-reversion>=2.0.8",
+            "djangorestframework>=3.6.2",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["django20-pypy3"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            ".[test]",
+            "Django>=2.0,<2.1",
+            "django-reversion>=2.0.8",
+            "djangorestframework>=3.7.3",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["django111-py34"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            ".[test]",
+            "Django>=1.11.0,<2.0.0",
+            "django-reversion>=2.0.8",
+            "djangorestframework>=3.6.2",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["django18-py27"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [
+            ".[test]",
+            "Django>=1.8.0,<1.9.0",
+            "django-reversion==1.10.0",
+            "djangorestframework>=3.3.3,<3.7.0",
+        ]
+
+        conf = newconfig([], inisource).envconfigs["lint"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [".[test]"]
+
+        conf = newconfig([], inisource).envconfigs["docs"]
+        packages = [dep.name for dep in conf.deps]
+        assert packages == [".[test]"]
+
     def test_take_dependencies_from_other_section(self, newconfig):
         inisource = """
             [testing:pytest]