Repository: tox-dev/tox

Issue:
deps passed to pip are not de-factorized
Given this `tox.ini`:

```ini
[testenv]
deps =
  foo
  coverage: coverage

[testenv:sub]
deps =
  {[testenv]deps}
```

Running `tox -e sub` results in:

```
sub create: /tmp/t1/.tox/sub
sub installdeps: foo, coverage: coverage
ERROR: invocation failed (exit code 1), logfile: /tmp/t1/.tox/sub/log/sub-1.log
ERROR: actionid: sub
msg: getenv
cmdargs: ['/tmp/t1/.tox/sub/bin/pip', 'install', 'foo', 'coverage: coverage']

Invalid requirement: 'coverage: coverage'
Traceback (most recent call last):
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/packaging/requirements.py", line 92, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 1617, in parseString
    raise exc
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 1607, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 3376, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 1383, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/pyparsing.py", line 3164, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pip._vendor.pyparsing.ParseException: Expected stringEnd (at char 8), (line:1, col:9)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/req/req_install.py", line 82, in __init__
    req = Requirement(req)
  File "/tmp/tox/tmp/t1/sub/lib/python3.6/site-packages/pip/_vendor/packaging/requirements.py", line 96, in __init__
    requirement_string[e.loc:e.loc + 8]))
pip._vendor.packaging.requirements.InvalidRequirement: Invalid requirement, parse error at "': covera'"


ERROR: could not install deps [foo, coverage: coverage]; v = InvocationError('/tmp/t1/.tox/sub/bin/pip install foo coverage: coverage (see /tmp/t1/.tox/sub/log/sub-1.log)', 1)
__________________________________________ summary __________________________________________
ERROR:   sub: could not install deps [foo, coverage: coverage]; v = InvocationError('/tmp/t1/.tox/sub/bin/pip install foo coverage: coverage (see /tmp/t1/.tox/sub/log/sub-1.log)', 1)
```

`tox 2.9.2.dev35` (current master).

1. it should only install 'coverage' and not 'coverage: coverage'
2. it should only install it for when the "coverage" factor is used.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=h1) Report
> Merging [#772](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=desc) into [master](https://codecov.io/gh/tox-dev/tox/commit/9a9ea31922c75c275fc2e28ff842b3aad5664257?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/tox-dev/tox/pull/772/graphs/tree.svg?height=150&width=650&token=DYodAwDCZ5&src=pr)](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #772      +/-   ##
==========================================
+ Coverage   94.83%   94.84%   +<.01%     
==========================================
  Files          11       11              
  Lines        2402     2406       +4     
==========================================
+ Hits         2278     2282       +4     
  Misses        124      124
```


| [Impacted Files](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tox/config.py](https://codecov.io/gh/tox-dev/tox/pull/772/diff?src=pr&el=tree#diff-dG94L2NvbmZpZy5weQ==) | `97.76% <100%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=footer). Last update [9a9ea31...1fc5b93](https://codecov.io/gh/tox-dev/tox/pull/772?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/changelog/706.bugfix.rst b/changelog/706.bugfix.rst
new file mode 100644
index 00000000..b0827db6
--- /dev/null
+++ b/changelog/706.bugfix.rst
@@ -0,0 +1,1 @@
+Fix bug with incorrectly defactorized dependencies - by @bartsanchez
diff --git a/doc/config.rst b/doc/config.rst
index 2e12fe50..cf1d9ec4 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/tox/config.py b/tox/config.py
index ad72bdf3..f8cb9ff4 100755
--- a/tox/config.py
+++ b/tox/config.py
@@ -1076,10 +1076,10 @@ class SectionReader:
         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 @@ class SectionReader:
             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(


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 26421fa5..61d106bf 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -1294,7 +1294,36 @@ 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']
+
+    # 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 = """