Simplify reporters log checks

Author: gaborbernat

azure-pipelines.yml

@@ -7,6 +7,7 @@ variables:
   CI_BUILD_URL: "https://toxdev.visualstudio.com/tox/_build/results?buildId=$(Build.BuildId)"
   GIT_BRANCH: $(Build.SourceBranch)
   GIT_COMMIT_SHA: $(Build.SourceVersion)
+  PYTEST_ADDOPTS: "-vv -ra --showlocals"
 
 trigger:
   batch: true

changelog/1026.feature.rst

@@ -5,5 +5,3 @@ install it). The hard link is removed at the end of the run (what it points thou
 inside ``distdir``). This ensures that a tox session operates on the same package it built, even
 if a parallel tox run builds another version. Note ``distdir`` will contain only the last built
 package in such cases. - by :user:`gaborbernat`
-
-

doc/config.rst

@@ -60,12 +60,16 @@ Global settings are defined under the ``tox`` section as:
    Indicates where the packaging root file exists (historically the ``setup.py`` for ``setuptools``).
    This will be the working directory when performing the packaging.
 
-
 .. conf:: distdir ^ PATH ^ {toxworkdir}/dist
 
    Directory where the packaged source distribution should be put. Note this is cleaned at the start of
    every packaging invocation.
 
+.. conf:: sdistsrc ^ PATH ^ {toxworkdir}/dist
+
+   Do not build the package, but instead use teh latest package available under this path.
+   You can override it via the command line flag ``--installpkg``.
+
 .. conf:: distshare ^ PATH ^ {homedir}/.tox/distshare
 
    Folder where the packaged source distribution will be moved, this is not cleaned between packaging

src/tox/package.py

@@ -20,7 +20,7 @@
 def tox_package(session, venv):
     """Build an sdist at first call return that for all calls"""
     if not hasattr(session, "package"):
-        session.package = get_package(session)
+        session.package, session.dist = get_package(session)
     return session.package
 
 
@@ -41,7 +41,8 @@ def get_package(session):
             report.verbosity0("lock file {} present, will block until released".format(lock_file))
             lock.acquire()
         package = acquire_package(config, report, session)
-        return create_session_view(package, config.temp_dir, report)
+        session_package = create_session_view(package, config.temp_dir, report)
+        return session_package, package
     finally:
         lock.release(force=True)
 
@@ -52,12 +53,19 @@ def create_session_view(package, temp_dir, report):
     return a view of the file that it's not prone to deletion and can be removed when the
     session ends
     """
+    if not package:
+        return package
+    temp_dir.ensure(dir=True)
+
+    # we'll prefix it with a unique number, note adding as suffix can cause conflicts
+    # with tools that check the files extension (e.g. pip)
     exists = [
-        i.basename[len(package.basename) + 1 :]
-        for i in temp_dir.listdir(fil="{}.*".format(package.basename))
+        i.basename[: -len(package.basename) - 1]
+        for i in temp_dir.listdir(fil="*-{}".format(package.basename))
     ]
     file_id = max(chain((0,), (int(i) for i in exists if six.text_type(i).isnumeric())))
-    session_package = temp_dir.join("{}.{}".format(package.basename, file_id + 1))
+    session_package = temp_dir.join("{}-{}".format(file_id + 1, package.basename))
+
     # if we can do hard links do that, otherwise just copy
     operation = "links"
     if hasattr(os, "link"):
@@ -67,20 +75,17 @@ def create_session_view(package, temp_dir, report):
         package.copy(session_package)
     common = session_package.common(package)
     report.verbosity1(
-        "cwd {} package {} {} to {}".format(
-            common, common.bestrelpath(session_package), operation, common.bestrelpath(package)
+        "package {} {} to {} ({})".format(
+            common.bestrelpath(session_package), operation, common.bestrelpath(package), common
         )
     )
     return session_package
 
 
 def acquire_package(config, report, session):
+    """acquire a source distribution (either by loading a local file or triggering a build)"""
     if not config.option.sdistonly and (config.sdistsrc or config.option.installpkg):
-        path = config.option.installpkg
-        if not path:
-            path = config.sdistsrc
-        path = session._resolve_package(path)
-        report.info("using package {!r}, skipping 'sdist' activity ".format(str(path)))
+        path = get_local_package(config, report, session)
     else:
         try:
             path = build_package(config, report, session)
@@ -99,6 +104,15 @@ def acquire_package(config, report, session):
     return path
 
 
+def get_local_package(config, report, session):
+    path = config.option.installpkg
+    if not path:
+        path = config.sdistsrc
+    path = session._resolve_package(path)
+    report.info("using package {!r}, skipping 'sdist' activity ".format(str(path)))
+    return path
+
+
 def build_package(config, report, session):
     if not config.isolated_build:
         return make_sdist_legacy(report, config, session)

src/tox/result.py

@@ -45,7 +45,7 @@ def __init__(self, reportlog, name, dict):
 
     def set_python_info(self, python_executable):
         cmd = [
-            python_executable,
+            str(python_executable),
             "-c",
             "import sys; import json;"
             "print(json.dumps({"

src/tox/session.py

@@ -318,13 +318,14 @@ def startsummary(self):
         if self.verbosity >= Verbosity.QUIET:
             self.tw.sep("_", "summary")
 
-    def info(self, msg):
-        if self.verbosity >= Verbosity.DEBUG:
-            self.logline(msg)
+    def logline_if(self, level, msg, key=None, **kwargs):
+        if self.verbosity >= level:
+            message = str(msg) if key is None else "{}{}".format(key, msg)
+            self.logline(message, **kwargs)
 
-    def using(self, msg):
-        if self.verbosity >= Verbosity.INFO:
-            self.logline("using {}".format(msg), bold=True)
+    def logline(self, msg, **opts):
+        self.reported_lines.append(msg)
+        self.tw.line("{}".format(msg), **opts)
 
     def keyboard_interrupt(self):
         self.error("KEYBOARDINTERRUPT")
@@ -339,40 +340,32 @@ def keyvalue(self, name, value):
     def line(self, msg, **opts):
         self.logline(msg, **opts)
 
+    def info(self, msg):
+        self.logline_if(Verbosity.DEBUG, msg)
+
+    def using(self, msg):
+        self.logline_if(Verbosity.INFO, msg, "using ", bold=True)
+
     def good(self, msg):
-        if self.verbosity >= Verbosity.QUIET:
-            self.logline(msg, green=True)
+        self.logline_if(Verbosity.QUIET, msg, green=True)
 
     def warning(self, msg):
-        if self.verbosity >= Verbosity.QUIET:
-            self.logline("WARNING: {}".format(msg), red=True)
+        self.logline_if(Verbosity.QUIET, msg, "WARNING: ", red=True)
 
     def error(self, msg):
-        if self.verbosity >= Verbosity.QUIET:
-            self.logline("ERROR: {}".format(msg), red=True)
+        self.logline_if(Verbosity.QUIET, msg, "ERROR: ", red=True)
 
     def skip(self, msg):
-        if self.verbosity >= Verbosity.QUIET:
-            self.logline("SKIPPED: {}".format(msg), yellow=True)
-
-    def logline(self, msg, **opts):
-        self.reported_lines.append(msg)
-        self.tw.line("{}".format(msg), **opts)
+        self.logline_if(Verbosity.QUIET, msg, "SKIPPED: ", yellow=True)
 
     def verbosity0(self, msg, **opts):
-        if self.verbosity >= Verbosity.DEFAULT:
-            self.logline("{}".format(msg), **opts)
+        self.logline_if(Verbosity.DEFAULT, msg, **opts)
 
     def verbosity1(self, msg, **opts):
-        if self.verbosity >= Verbosity.INFO:
-            self.logline("{}".format(msg), **opts)
+        self.logline_if(Verbosity.INFO, msg, **opts)
 
     def verbosity2(self, msg, **opts):
-        if self.verbosity >= Verbosity.DEBUG:
-            self.logline("{}".format(msg), **opts)
-
-    # def log(self, msg):
-    #    print(msg, file=sys.stderr)
+        self.logline_if(Verbosity.DEBUG, msg, **opts)
 
 
 class Session:
@@ -459,7 +452,11 @@ def cleanup(self):
             yield
         finally:
             for tox_env in self.venvlist:
-                if hasattr(tox_env, "package") and tox_env.package.exists():
+                if (
+                    hasattr(tox_env, "package")
+                    and isinstance(tox_env.package, py.path.local)
+                    and tox_env.package.exists()
+                ):
                     self.report.verbosity2("cleanup {}".format(tox_env.package))
                     tox_env.package.remove()
 

tests/unit/test_package.py

@@ -1,3 +1,4 @@
+import os
 import re
 import traceback
 from functools import partial
@@ -21,15 +22,15 @@ def test_make_sdist(initproj):
     )
     config = parseconfig([])
     session = Session(config)
-    sdist = get_package(session)
+    _, sdist = get_package(session)
     assert sdist.check()
     assert sdist.ext == ".zip"
     assert sdist == config.distdir.join(sdist.basename)
-    sdist2 = get_package(session)
+    _, sdist2 = get_package(session)
     assert sdist2 == sdist
     sdist.write("hello")
     assert sdist.stat().size < 10
-    sdist_new = get_package(Session(config))
+    _, sdist_new = get_package(Session(config))
     assert sdist_new == sdist
     assert sdist_new.stat().size > 10
 
@@ -50,13 +51,18 @@ def test_make_sdist_distshare(tmpdir, initproj):
     )
     config = parseconfig([])
     session = Session(config)
-    sdist = get_package(session)
-    assert sdist.check()
-    assert sdist.ext == ".zip"
-    assert sdist == config.distdir.join(sdist.basename)
-    sdist_share = config.distshare.join(sdist.basename)
+    package, dist = get_package(session)
+    assert package.check()
+    assert package.ext == ".zip"
+    assert package == config.temp_dir.join(package.basename)
+
+    assert dist == config.distdir.join(package.basename[len("1-") :])
+    assert dist.check()
+    assert os.stat(str(dist)).st_ino == os.stat(str(package)).st_ino
+
+    sdist_share = config.distshare.join(package.basename[len("1-") :])
     assert sdist_share.check()
-    assert sdist_share.read("rb") == sdist.read("rb"), (sdist_share, sdist)
+    assert sdist_share.read("rb") == dist.read("rb"), (sdist_share, package)
 
 
 def test_sdistonly(initproj, cmd):
@@ -110,12 +116,21 @@ def test_separate_sdist(cmd, initproj, tmpdir):
     )
     result = cmd("--sdistonly")
     assert not result.ret
-    sdistfiles = distshare.listdir()
-    assert len(sdistfiles) == 1
-    sdistfile = sdistfiles[0]
+    dist_share_files = distshare.listdir()
+    assert len(dist_share_files) == 1
+    assert dist_share_files[0].check()
+
     result = cmd("-v", "--notest")
-    assert not result.ret
-    assert "python inst: {}".format(sdistfile) in result.out
+    assert not result.ret, result.out
+    msg = "python inst: {}".format(result.session.package)
+    assert msg in result.out, result.out
+    operation = "copied" if not hasattr(os, "link") else "links"
+    msg = "package {} {} to {}".format(
+        os.sep.join(("pkg123", ".tox", ".tmp", "1-pkg123-0.7.zip")),
+        operation,
+        os.sep.join(("distshare", "pkg123-0.7.zip")),
+    )
+    assert msg in result.out, result.out
 
 
 def test_sdist_latest(tmpdir, newconfig):
@@ -133,15 +148,15 @@ def test_sdist_latest(tmpdir, newconfig):
     p = distshare.ensure("pkg123-1.4.5.zip")
     distshare.ensure("pkg123-1.4.5a1.zip")
     session = Session(config)
-    sdist_path = get_package(session)
-    assert sdist_path == p
+    _, dist = get_package(session)
+    assert dist == p
 
 
 def test_installpkg(tmpdir, newconfig):
     p = tmpdir.ensure("pkg123-1.0.zip")
     config = newconfig(["--installpkg={}".format(p)], "")
     session = Session(config)
-    sdist_path = get_package(session)
+    _, sdist_path = get_package(session)
     assert sdist_path == p
 
 
@@ -347,10 +362,14 @@ def build_package(config, report, session):
     for val in invoke_result.values():
         if isinstance(val, tuple):
             assert False, "{!r}\n{}".format(val[0], val[1])
-
-    # output has no error
-    err = "\n".join((invoke_result["t1"].err, invoke_result["t2"].err)).strip()
-    out = "\n".join((invoke_result["t1"].out, invoke_result["t2"].out)).strip()
+    err = "\n".join(
+        "{}=\n{}".format(k, v.err).strip() for k, v in invoke_result.items() if v.err.strip()
+    )
+    out = "\n".join(
+        "{}=\n{}".format(k, v.out).strip() for k, v in invoke_result.items() if v.out.strip()
+    )
+    for val in invoke_result.values():
+        assert not val.ret, "{}\n{}".format(err, out)
     assert not err
 
     # when the lock is hit we notify