Repository: tox-dev/tox
Pull request #708 (2017-12-10 10:07:07)

trivial interpreters py touchups and test coverage improvements
Trivial interpreters py touchups and test coverage improvements.

codecov[bot]: # [Codecov](https://codecov.io/gh/tox-dev/tox/pull/708?src=pr&el=h1) Report
> Merging [#708](https://codecov.io/gh/tox-dev/tox/pull/708?src=pr&el=desc) into [master](https://codecov.io/gh/tox-dev/tox/commit/060758d8972a10687f0869ac9ced4c7eabaa904d?src=pr&el=desc) will **increase** coverage by `0.57%`.
> The diff coverage is `50%`.

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

```diff
@@            Coverage Diff             @@
##           master     #708      +/-   ##
==========================================
+ Coverage   93.95%   94.53%   +0.57%     
==========================================
  Files          11       11              
  Lines        2416     2414       -2     
==========================================
+ Hits         2270     2282      +12     
+ Misses        146      132      -14
```


| [Impacted Files](https://codecov.io/gh/tox-dev/tox/pull/708?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tox/interpreters.py](https://codecov.io/gh/tox-dev/tox/pull/708/diff?src=pr&el=tree#diff-dG94L2ludGVycHJldGVycy5weQ==) | `92.85% <50%> (+10.82%)` | :arrow_up: |

------

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

gaborbernat: hi, the lint fix failed 👍 
jurko-gospodnetic: doh... I hate that 'reorder imports' failure... wish it would tell me what it wants them to be reordered to...

I really have to get fix-lint to run on my machine...
gaborbernat: @asottile I feel the same as @jurko-gospodnetic  the ``pre-commit`` often gives fairly cryptic errors; also i would prefer to group together imports (instead one per line)
jurko-gospodnetic: Ok, somehow hacked together `fix-lint` to work on my machine (that whole thing is totally unfriendly to my setup... 😞) and finally found the culprit reordering... seems the ordering it wants is case-sensitive :-)
asottile: The brevity of the error messages is intentional. The general guidance is : if it gets automatically fixed there's no need to spam a bunch of text, the filename is sufficient. But as it's apparent that you're not running them locally, I'm adding this so you can at least see the diff : https://github.com/tox-dev/tox/pull/710

As for the imports: it's impossible to agree on style when there are this many maintainers so we opted for a style that eliminates merge conflicts. If you're unhappy with this let's create an issue and discuss the trade offs of removing the import ordering entirely -- though I imagine this will just make things worse ;) 

Patch:
diff --git a/changelog/708.trivial.rst b/changelog/708.trivial.rst
new file mode 100644
index 00000000..3af7ea41
--- /dev/null
+++ b/changelog/708.trivial.rst
@@ -0,0 +1,1 @@
+touched up ``interpreters.py`` code and added some missing tests for it - by @jurko-gospodnetic
diff --git a/tox.ini b/tox.ini
index 258f5c93..4c9ac636 100644
--- a/tox.ini
+++ b/tox.ini
@@ -32,7 +32,7 @@ extras = lint
 description = run static analysis and style check using flake8
 commands = python -m flake8 --show-source tox setup.py {posargs}
            python -m flake8 --show-source doc tests {posargs}
-           pre-commit run --all-files
+           pre-commit run --all-files --show-diff-on-failure
            python -c 'print("hint: run {envdir}/bin/pre-commit install to add checks as pre-commit hook")'
 
 
diff --git a/tox/interpreters.py b/tox/interpreters.py
index 7eedb15a..49ea431c 100644
--- a/tox/interpreters.py
+++ b/tox/interpreters.py
@@ -48,9 +48,8 @@ class Interpreters:
             res = exec_on_interpreter(info.executable,
                                       [inspect.getsource(sitepackagesdir),
                                        "print(sitepackagesdir(%r))" % envdir])
-        except ExecFailed:
-            val = sys.exc_info()[1]
-            print("execution failed: %s -- %s" % (val.out, val.err))
+        except ExecFailed as e:
+            print("execution failed: %s -- %s" % (e.out, e.err))
             return ""
         else:
             return res["dir"]
@@ -59,12 +58,11 @@ class Interpreters:
 def run_and_get_interpreter_info(name, executable):
     assert executable
     try:
-        result = exec_on_interpreter(executable,
-                                     [inspect.getsource(pyinfo), "print(pyinfo())"])
-    except ExecFailed:
-        val = sys.exc_info()[1]
-        return NoInterpreterInfo(name, executable=val.executable,
-                                 out=val.out, err=val.err)
+        result = exec_on_interpreter(executable, [inspect.getsource(pyinfo),
+                                                  "print(pyinfo())"])
+    except ExecFailed as e:
+        return NoInterpreterInfo(name, executable=e.executable,
+                                 out=e.out, err=e.err)
     else:
         return InterpreterInfo(name, executable, **result)
 
@@ -123,7 +121,7 @@ class NoInterpreterInfo:
 
     def __str__(self):
         if self.executable:
-            return "<executable at %s, not runnable>"
+            return "<executable at %s, not runnable>" % self.executable
         else:
             return "<executable not found for: %s>" % self.name
 
@@ -184,5 +182,5 @@ def pyinfo():
 
 
 def sitepackagesdir(envdir):
-    from distutils.sysconfig import get_python_lib
-    return dict(dir=get_python_lib(prefix=envdir))
+    import distutils.sysconfig
+    return dict(dir=distutils.sysconfig.get_python_lib(prefix=envdir))


Test patch:
diff --git a/tests/test_interpreters.py b/tests/test_interpreters.py
index 4ad75d67..fcc5a7a1 100644
--- a/tests/test_interpreters.py
+++ b/tests/test_interpreters.py
@@ -7,8 +7,13 @@ import py
 import pytest
 
 from tox.config import get_plugin_manager
+from tox.interpreters import ExecFailed
+from tox.interpreters import InterpreterInfo
 from tox.interpreters import Interpreters
+from tox.interpreters import NoInterpreterInfo
+from tox.interpreters import pyinfo
 from tox.interpreters import run_and_get_interpreter_info
+from tox.interpreters import sitepackagesdir
 from tox.interpreters import tox_get_python_executable
 
 
@@ -70,7 +75,8 @@ def test_tox_get_python_executable():
         envconfig.basepython = name
         p = tox_get_python_executable(envconfig)
         assert p
-        popen = subprocess.Popen([str(p), '-V'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        popen = subprocess.Popen([str(p), '-V'], stderr=subprocess.PIPE,
+                                 stdout=subprocess.PIPE)
         stdout, stderr = popen.communicate()
         assert not stdout or not stderr
         assert ver in stderr.decode('ascii') or ver in stdout.decode('ascii')
@@ -130,3 +136,92 @@ class TestInterpreters:
         info = interpreters.get_info(envconfig)
         s = interpreters.get_sitepackagesdir(info, "")
         assert s
+
+
+def test_pyinfo(monkeypatch):
+    monkeypatch.setattr(sys, "version_info", [42, 4242])
+    monkeypatch.setattr(sys, "platform", "an unladen swallow")
+    result = pyinfo()
+    assert len(result) == 2
+    assert result["version_info"] == (42, 4242)
+    assert result["sysplatform"] == "an unladen swallow"
+
+
+def test_sitepackagesdir(monkeypatch):
+    import distutils.sysconfig as sysconfig
+    test_envdir = "holy grail"
+    test_dir = "Now go away or I will taunt you a second time."
+
+    def dummy_get_python_lib(prefix):
+        assert prefix is test_envdir
+        return test_dir
+
+    monkeypatch.setattr(sysconfig, "get_python_lib", dummy_get_python_lib)
+    assert sitepackagesdir(test_envdir) == {"dir": test_dir}
+
+
+def test_exec_failed():
+    x = ExecFailed("my-executable", "my-source", "my-out", "my-err")
+    assert isinstance(x, Exception)
+    assert x.executable == "my-executable"
+    assert x.source == "my-source"
+    assert x.out == "my-out"
+    assert x.err == "my-err"
+
+
+class TestInterpreterInfo:
+
+    def info(self, name="my-name", executable="my-executable",
+             version_info="my-version-info", sysplatform="my-sys-platform"):
+        return InterpreterInfo(name, executable, version_info, sysplatform)
+
+    def test_runnable(self):
+        assert self.info().runnable
+
+    @pytest.mark.parametrize("missing_arg", ("executable", "version_info"))
+    def test_assert_on_missing_args(self, missing_arg):
+        with pytest.raises(AssertionError):
+            self.info(**{missing_arg: None})
+
+    def test_data(self):
+        x = self.info("larry", "moe", "shemp", "curly")
+        assert x.name == "larry"
+        assert x.executable == "moe"
+        assert x.version_info == "shemp"
+        assert x.sysplatform == "curly"
+
+    def test_str(self):
+        x = self.info(executable="foo", version_info="bar")
+        assert str(x) == "<executable at foo, version_info bar>"
+
+
+class TestNoInterpreterInfo:
+
+    def test_runnable(self):
+        assert not NoInterpreterInfo("foo").runnable
+        assert not NoInterpreterInfo("foo", executable=sys.executable).runnable
+
+    def test_default_data(self):
+        x = NoInterpreterInfo("foo")
+        assert x.name == "foo"
+        assert x.executable is None
+        assert x.version_info is None
+        assert x.out is None
+        assert x.err == "not found"
+
+    def test_set_data(self):
+        x = NoInterpreterInfo("migraine", executable="my-executable",
+                              out="my-out", err="my-err")
+        assert x.name == "migraine"
+        assert x.executable == "my-executable"
+        assert x.version_info is None
+        assert x.out == "my-out"
+        assert x.err == "my-err"
+
+    def test_str_without_executable(self):
+        x = NoInterpreterInfo("coconut")
+        assert str(x) == "<executable not found for: coconut>"
+
+    def test_str_with_executable(self):
+        x = NoInterpreterInfo("coconut", executable="bang/em/together")
+        assert str(x) == "<executable at bang/em/together, not runnable>"