Skip to content

Fix bug with incorrectly defactorized dependencies #768 #1057

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/706.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug with incorrectly defactorized dependencies - by @bartsanchez
25 changes: 25 additions & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------

Expand Down
8 changes: 7 additions & 1 deletion src/tox/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1337,10 +1337,16 @@ def getstring(self, name, default=None, replace=True, crossonly=False):
x = default
else:
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)
Copy link
Member

Choose a reason for hiding this comment

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

we should have here some explanation of why the repetition of same commands is needed as a comment

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll try to summarize the issue as much as I can.

# print "getstring", self.section_name, name, "returned", repr(x)
return x
Copy link
Member

Choose a reason for hiding this comment

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

the comment here is redundant

Copy link
Author

Choose a reason for hiding this comment

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

The comment is not mine, I kept it as it was already in the code.

I would prefer to not having commented code. If you do prefer I can simply remove it.

Copy link
Member

Choose a reason for hiding this comment

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

thanks


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):
Expand Down
224 changes: 221 additions & 3 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1444,11 +1444,9 @@ def test_take_dependencies_from_other_testenv(self, newconfig, envlist, deps):
)
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 = """
Expand Down Expand Up @@ -1477,6 +1475,226 @@ def test_regression_test_issue_706(self, newconfig, envlist):
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"]

# https://github.com/tox-dev/tox/issues/899
def test_regression_test__issue_899(self, newconfig):
Copy link
Member

Choose a reason for hiding this comment

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

would prefer a name that explains the edge case comparison, so maintainers don't need to feed the issue in detail

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll change it.

inisource = """
[tox]
minversion = 2.0
skipsdist = True
envlist =
style
sdist
bdist_wheel
{py27,py34,py35,py36,pypy,pypy3}-cover
{py27,py34,py35,py36,pypy,pypy3}-nocov

[testenv]
skip_install = True
sitepackages = True
passenv =
Copy link
Member

Choose a reason for hiding this comment

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

also please remove lines not essential to reproduce the bug, it just makes hard to read the test case

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll change it.

CI
TRAVIS
TRAVIS_*
TOXENV
CODECOV_*
whitelist_externals =
bash
echo
install_command = pip install --only-binary=scipy {opts} {packages}
deps =
#Lines startings xxx: are filtered by the environment.
#Leaving py34 without any soft dependencies (just numpy)
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",
]

# https://github.com/tox-dev/tox/issues/906
def test_regression_test__issue_906(self, newconfig):
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll change it.

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
skipsdist = true

[testenv]
deps =
.[test]
django18: {[django]1.8.x}
django111: {[django]1.11.x}
django20: {[django]2.0.x}
django_master: {[django]master}
commands = py.test --ds=tests.settings --cov=./djmoney {posargs}
usedevelop = false

[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]
Expand Down