Repository: tox-dev/tox
Pull request #1022 (2018-09-27 15:56:19)

[WIP] "requires" config improvements
If requirements are not satisfied the error is now represented by a custom exception and appropriately caught to make the error look like what it is: a misconfiguration of the environment rather than an error in tox.

so this:

```console
tox
ERROR: MissingRequirement: Packages tox-venv, tox-travis need to be installed alongside tox in [...]/bin/python
```

rather than:

```console
tox                                 (git)-[generic-project/master:613bf7ce2c38+! ]
requirement missing tox-venv
requirement missing tox-travis
Traceback (most recent call last):
  File "[...]/bin/tox", line 11, in <module>
    sys.exit(cmdline())
  File "[...]/lib/python3.7/site-packages/tox/session.py", line 41, in cmdline
    main(args)
  File "[...]/lib/python3.7/site-packages/tox/session.py", line 46, in main
    config = prepare(args)
  File "[...]/lib/python3.7/site-packages/tox/session.py", line 28, in prepare
    config = parseconfig(args)
  File "[...]/lib/python3.7/site-packages/tox/config.py", line 232, in parseconfig
    ParseIni(config, config_file, content)
  File "[...]/lib/python3.7/site-packages/tox/config.py", line 982, in __init__
    self.ensure_requires_satisfied(reader.getlist("requires"))
  File "[...]/lib/python3.7/site-packages/tox/config.py", line 1099, in ensure_requires_satisfied
    raise RuntimeError("not all requirements satisfied, install them alongside tox")
RuntimeError: not all requirements satisfied, install them alongside tox
```

The docs give explicit info now also, that require is not doing what you might expect if you draw an analogy to the require options in `setuptools.setup()`.

Marked this wip as I am not sure whether we really need a new exception or how to deal in general with our internal exceptions. I feel we should catch them all on the upper level and output a friendly error rather than a traceback (but maybe this should be tackled in another PR).


codecov[bot]: # [Codecov](https://codecov.io/gh/tox-dev/tox/pull/1022?src=pr&el=h1) Report
> Merging [#1022](https://codecov.io/gh/tox-dev/tox/pull/1022?src=pr&el=desc) into [master](https://codecov.io/gh/tox-dev/tox/commit/d2b27393f2a0a54d7e0e146376c411b0da683ba9?src=pr&el=desc) will **decrease** coverage by `0.97%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master    #1022      +/-   ##
==========================================
- Coverage   70.39%   69.42%   -0.98%     
==========================================
  Files          14       14              
  Lines        3486     3486              
  Branches      465      465              
==========================================
- Hits         2454     2420      -34     
- Misses        971     1003      +32     
- Partials       61       63       +2
```


| [Impacted Files](https://codecov.io/gh/tox-dev/tox/pull/1022?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/tox/interpreters.py](https://codecov.io/gh/tox-dev/tox/pull/1022/diff?src=pr&el=tree#diff-c3JjL3RveC9pbnRlcnByZXRlcnMucHk=) | `64.74% <ø> (-17.99%)` | :arrow_down: |
| [src/tox/config.py](https://codecov.io/gh/tox-dev/tox/pull/1022/diff?src=pr&el=tree#diff-c3JjL3RveC9jb25maWcucHk=) | `64.95% <100%> (-0.26%)` | :arrow_down: |
| [src/tox/session.py](https://codecov.io/gh/tox-dev/tox/pull/1022/diff?src=pr&el=tree#diff-c3JjL3RveC9zZXNzaW9uLnB5) | `77.81% <100%> (-0.32%)` | :arrow_down: |
| [src/tox/exception.py](https://codecov.io/gh/tox-dev/tox/pull/1022/diff?src=pr&el=tree#diff-c3JjL3RveC9leGNlcHRpb24ucHk=) | `79.16% <100%> (+0.44%)` | :arrow_up: |
| [src/tox/venv.py](https://codecov.io/gh/tox-dev/tox/pull/1022/diff?src=pr&el=tree#diff-c3JjL3RveC92ZW52LnB5) | `78.58% <0%> (-0.89%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tox-dev/tox/pull/1022?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/1022?src=pr&el=footer). Last update [d2b2739...f1c1646](https://codecov.io/gh/tox-dev/tox/pull/1022?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

gaborbernat: Fair catch, however will be rendered not needed by https://github.com/tox-dev/tox/issues/998. We can merge it in the meantime.
obestwalter: ah - didn't see that one yet. Definitely a good way to go. I just started catching up properly today and did not really go through all the issues :)
gaborbernat: Looks like an improvement on its own so I'll merge it.

Patch:
diff --git a/doc/config.rst b/doc/config.rst
index ef9ac5cc..db3ca799 100644
--- a/doc/config.rst
+++ b/doc/config.rst
@@ -120,8 +120,12 @@ Global settings are defined under the ``tox`` section as:
     .. code-block:: ini
 
         [tox]
-        requires = setuptools >= 30.0.0
-                   py
+        requires = tox-venv
+                   setuptools >= 30.0.0
+
+    .. note:: tox does **not** install those required packages for you. tox only checks if the
+              requirements are satisfied and crashes early with an helpful error rather then later
+              in the process.
 
 .. conf:: isolated_build ^ true|false ^ false
 
diff --git a/src/tox/config.py b/src/tox/config.py
index e560c221..2bcd0f7b 100755
--- a/src/tox/config.py
+++ b/src/tox/config.py
@@ -1084,19 +1084,20 @@ class ParseIni(object):
 
     @staticmethod
     def ensure_requires_satisfied(specified):
-        fail = False
+        missing_requirements = []
         for s in specified:
             try:
                 pkg_resources.get_distribution(s)
             except pkg_resources.RequirementParseError:
                 raise
             except Exception:
-                fail = True
-                print(
-                    "requirement missing {}".format(pkg_resources.Requirement(s)), file=sys.stderr
+                missing_requirements.append(str(pkg_resources.Requirement(s)))
+        if missing_requirements:
+            raise tox.exception.MissingRequirement(
+                "Packages {} need to be installed alongside tox in {}".format(
+                    ", ".join(missing_requirements), sys.executable
                 )
-        if fail:
-            raise RuntimeError("not all requirements satisfied, install them alongside tox")
+            )
 
     def _list_section_factors(self, section):
         factors = set()
diff --git a/src/tox/exception.py b/src/tox/exception.py
index 246fca7f..4ad07e46 100644
--- a/src/tox/exception.py
+++ b/src/tox/exception.py
@@ -80,6 +80,10 @@ class MissingDependency(Error):
     """A dependency could not be found or determined."""
 
 
+class MissingRequirement(Error):
+    """A requirement defined in :config:`require` is not met."""
+
+
 class MinVersionError(Error):
     """The installed tox version is lower than requested minversion."""
 
diff --git a/src/tox/interpreters.py b/src/tox/interpreters.py
index ccdbfdb5..e3fd9902 100644
--- a/src/tox/interpreters.py
+++ b/src/tox/interpreters.py
@@ -151,6 +151,7 @@ else:
             # The standard names are in predictable places.
             actual = r"c:\python{}\python.exe".format("".join(groups))
         else:
+
             actual = win32map.get(name, None)
         if actual:
             actual = py.path.local(actual)
diff --git a/src/tox/session.py b/src/tox/session.py
index 4d5c7823..e2c89f67 100644
--- a/src/tox/session.py
+++ b/src/tox/session.py
@@ -51,7 +51,7 @@ def main(args):
         raise SystemExit(retcode)
     except KeyboardInterrupt:
         raise SystemExit(2)
-    except tox.exception.MinVersionError as e:
+    except (tox.exception.MinVersionError, tox.exception.MissingRequirement) as e:
         r = Reporter(None)
         r.error(str(e))
         raise SystemExit(1)


Test patch:
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 1981c2b8..9fbeefb0 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -2644,26 +2644,20 @@ class TestCommandParser:
         assert envconfig.commands[0] == ["some", r"hello\world"]
 
 
-def test_plugin_require(newconfig, capsys):
+def test_plugin_require(newconfig):
     inisource = """
         [tox]
         requires = tox
                    name[foo,bar]>=2,<3; python_version>"2.0" and os_name=='a'
                    b
     """
-    with pytest.raises(
-        RuntimeError, match="not all requirements satisfied, install them alongside tox"
-    ):
+    with pytest.raises(tox.exception.MissingRequirement) as exc_info:
         newconfig([], inisource)
 
-    out, err = capsys.readouterr()
-    assert err.strip() == "\n".join(
-        [
-            'requirement missing name[bar,foo]<3,>=2; python_version > "2.0" and os_name == "a"',
-            "requirement missing b",
-        ]
+    assert exc_info.value.args[0] == (
+        r'Packages name[bar,foo]<3,>=2; python_version > "2.0" and os_name == "a", b '
+        r"need to be installed alongside tox in {}".format(sys.executable)
     )
-    assert not out
 
 
 def test_isolated_build_env_cannot_be_in_envlist(newconfig, capsys):
diff --git a/tests/unit/test_docs.py b/tests/unit/test_docs.py
index c3e9b7b7..4d19e8b3 100644
--- a/tests/unit/test_docs.py
+++ b/tests/unit/test_docs.py
@@ -4,6 +4,7 @@ import textwrap
 
 import pytest
 
+import tox
 from tox.config import parseconfig
 
 INI_BLOCK_RE = re.compile(
@@ -20,9 +21,9 @@ INI_BLOCK_RE = re.compile(
 RST_FILES = []
 TOX_ROOT = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
 for root, _, filenames in os.walk(os.path.join(TOX_ROOT, "doc")):
-    for f in filenames:
-        if f.endswith(".rst"):
-            RST_FILES.append(os.path.join(root, f))
+    for filename in filenames:
+        if filename.endswith(".rst"):
+            RST_FILES.append(os.path.join(root, filename))
 
 
 def test_some_files_exist():
@@ -35,10 +36,12 @@ def test_all_rst_ini_blocks_parse(filename, tmpdir):
         contents = f.read()
     for match in INI_BLOCK_RE.finditer(contents):
         code = textwrap.dedent(match.group("code"))
-        f = tmpdir.join("tox.ini")
-        f.write(code)
+        config_path = tmpdir / "tox.ini"
+        config_path.write(code)
         try:
-            parseconfig(("-c", str(f)))
+            parseconfig(["-c", str(config_path)])
+        except tox.exception.MissingRequirement:
+            assert "requires = tox-venv" in str(code)
         except Exception as e:
             raise AssertionError(
                 "Error parsing ini block\n\n"