Fix for --result-json with --parallel (#1309)

* Correct ``--result-json`` output with ``--parallel``. (#1295)

When using ``--parallel`` with ``--result-json`` the test results are now included the same way as with serial runs.
This is accomplished by generating json result output for each individual run and at the end copy the data into the main json result output.

* avoid duplication in code, improve coverage

9 files changed, 140 insertions, 25 deletions

diff --git a/docs/changelog/1184.feature.rst b/docs/changelog/1184.feature.rst
index 736a7a58..388f3f5a 100644
--- a/docs/changelog/1184.feature.rst
+++ b/docs/changelog/1184.feature.rst
@@ -1 +1 @@
-Adding ```TOX_PARALLEL_NO_SPINNER``` environment variable to disable the spinner in parallel mode for the purposes of clean output when using CI tools - by :user:`zeroshift`
+Adding ``TOX_PARALLEL_NO_SPINNER`` environment variable to disable the spinner in parallel mode for the purposes of clean output when using CI tools - by :user:`zeroshift`
diff --git a/docs/changelog/1295.bugfix.rst b/docs/changelog/1295.bugfix.rst
new file mode 100644
index 00000000..c201f127
--- /dev/null
+++ b/docs/changelog/1295.bugfix.rst
@@ -0,0 +1 @@
+When using ``--parallel`` with ``--result-json`` the test results are now included the same way as with serial runs - by :user:`fschulze`
diff --git a/src/tox/constants.py b/src/tox/constants.py
index a2c2ed15..8dcc86bf 100644
--- a/src/tox/constants.py
+++ b/src/tox/constants.py
@@ -86,3 +86,5 @@ VERSION_QUERY_SCRIPT = os.path.join(_HELP_DIR, "get_version.py")
 SITE_PACKAGE_QUERY_SCRIPT = os.path.join(_HELP_DIR, "get_site_package_dir.py")
 BUILD_REQUIRE_SCRIPT = os.path.join(_HELP_DIR, "build_requires.py")
 BUILD_ISOLATED = os.path.join(_HELP_DIR, "build_isolated.py")
+PARALLEL_RESULT_JSON_PREFIX = ".tox-result"
+PARALLEL_RESULT_JSON_SUFFIX = ".json"
diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py
index de885c8f..e9388684 100644
--- a/src/tox/session/__init__.py
+++ b/src/tox/session/__init__.py
@@ -4,7 +4,9 @@ Python2 and Python3 based virtual environments. Environments are
 setup by using virtualenv. Configuration is generally done through an
 INI-style "tox.ini" file.
 """
+from __future__ import absolute_import, unicode_literals
 
+import json
 import os
 import re
 import subprocess
@@ -220,6 +222,24 @@ class Session(object):
             retcode = self._summary()
         return retcode
 
+    def _add_parallel_summaries(self):
+        if self.config.option.parallel != PARALLEL_OFF and "testenvs" in self.resultlog.dict:
+            result_log = self.resultlog.dict["testenvs"]
+            for tox_env in self.venv_dict.values():
+                data = self._load_parallel_env_report(tox_env)
+                if data and "testenvs" in data and tox_env.name in data["testenvs"]:
+                    result_log[tox_env.name] = data["testenvs"][tox_env.name]
+
+    @staticmethod
+    def _load_parallel_env_report(tox_env):
+        """Load report data into memory, remove disk file"""
+        result_json_path = tox_env.get_result_json_path()
+        if result_json_path and result_json_path.exists():
+            with result_json_path.open("r") as file_handler:
+                data = json.load(file_handler)
+            result_json_path.remove()
+            return data
+
     def _summary(self):
         is_parallel_child = PARALLEL_ENV_VAR_KEY in os.environ
         if not is_parallel_child:
@@ -254,12 +274,14 @@ class Session(object):
                 report(msg)
         if not exit_code and not is_parallel_child:
             reporter.good("  congratulations :)")
-        if not is_parallel_child:
-            path = self.config.option.resultjson
-            if path:
-                path = py.path.local(path)
-                path.write(self.resultlog.dumps_json())
-                reporter.line("wrote json report at: {}".format(path))
+        path = self.config.option.resultjson
+        if path:
+            if not is_parallel_child:
+                self._add_parallel_summaries()
+            path = py.path.local(path)
+            data = self.resultlog.dumps_json()
+            reporter.line("write json report at: {}".format(path))
+            path.write(data)
         return exit_code
 
     def showconfig(self):
diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py
index e307c1cf..76db9793 100644
--- a/src/tox/session/commands/run/parallel.py
+++ b/src/tox/session/commands/run/parallel.py
@@ -42,6 +42,9 @@ def run_parallel(config, venv_dict):
                 if hasattr(tox_env, "package"):
                     args_sub.insert(position, str(tox_env.package))
                     args_sub.insert(position, "--installpkg")
+                if tox_env.get_result_json_path():
+                    result_json_index = args_sub.index("--result-json")
+                    args_sub[result_json_index + 1] = "{}".format(tox_env.get_result_json_path())
                 with tox_env.new_action("parallel {}".format(tox_env.name)) as action:
 
                     def collect_process(process):
diff --git a/src/tox/util/lock.py b/src/tox/util/lock.py
index 00c86b5e..fd647340 100644
--- a/src/tox/util/lock.py
+++ b/src/tox/util/lock.py
@@ -36,6 +36,6 @@ def get_unique_file(path, prefix, suffix):
                 max_value = max(max_value, int(candidate.basename[len(prefix) : -len(suffix)]))
             except ValueError:
                 continue
-        winner = path.join("{}{}.log".format(prefix, max_value + 1))
+        winner = path.join("{}{}{}".format(prefix, max_value + 1, suffix))
         winner.ensure(dir=0)
         return winner
diff --git a/src/tox/venv.py b/src/tox/venv.py
index 1f786043..7ad2d00f 100644
--- a/src/tox/venv.py
+++ b/src/tox/venv.py
@@ -13,7 +13,9 @@ import tox
 from tox import reporter
 from tox.action import Action
 from tox.config.parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY
+from tox.constants import PARALLEL_RESULT_JSON_PREFIX, PARALLEL_RESULT_JSON_SUFFIX
 from tox.package.local import resolve_package
+from tox.util.lock import get_unique_file
 from tox.util.path import ensure_empty_dir
 
 from .config import DepConfig
@@ -113,6 +115,7 @@ class VirtualEnv(object):
         self.popen = popen
         self._actions = []
         self.env_log = env_log
+        self._result_json_path = None
 
     def new_action(self, msg, *args):
         config = self.envconfig.config
@@ -130,6 +133,14 @@ class VirtualEnv(object):
             self.envconfig.envpython,
         )
 
+    def get_result_json_path(self):
+        if self._result_json_path is None:
+            if self.envconfig.config.option.resultjson:
+                self._result_json_path = get_unique_file(
+                    self.path, PARALLEL_RESULT_JSON_PREFIX, PARALLEL_RESULT_JSON_SUFFIX
+                )
+        return self._result_json_path
+
     @property
     def hook(self):
         return self.envconfig.config.pluginmanager.hook
diff --git a/tests/unit/session/test_parallel.py b/tests/unit/session/test_parallel.py
index 3d8746b5..fcb51a21 100644
--- a/tests/unit/session/test_parallel.py
+++ b/tests/unit/session/test_parallel.py
@@ -1,10 +1,16 @@
 from __future__ import absolute_import, unicode_literals
 
+import json
+import os
+import subprocess
 import sys
+import threading
 
 import pytest
 from flaky import flaky
 
+from tox._pytestplugin import RunResult
+
 
 def test_parallel(cmd, initproj):
     initproj(
@@ -26,7 +32,7 @@ def test_parallel(cmd, initproj):
                         """,
         },
     )
-    result = cmd("--parallel", "all")
+    result = cmd("-p", "all")
     result.assert_success()
 
 
@@ -49,7 +55,7 @@ def test_parallel_live(cmd, initproj):
                         """,
         },
     )
-    result = cmd("--parallel", "all", "--parallel-live")
+    result = cmd("-p", "all", "-o")
     result.assert_success()
 
 
@@ -73,7 +79,7 @@ def test_parallel_circular(cmd, initproj):
                         """,
         },
     )
-    result = cmd("--parallel", "1")
+    result = cmd("-p", "1")
     result.assert_fail()
     assert result.out == "ERROR: circular dependency detected: a | b\n"
 
@@ -191,26 +197,96 @@ parallel_show_output = True
     assert "stderr always" in result.out, result.output()
 
 
-def test_parallel_no_spinner(cmd, initproj, monkeypatch):
-    monkeypatch.setenv(str("TOX_PARALLEL_NO_SPINNER"), str("1"))
-    initproj(
+@pytest.fixture()
+def parallel_project(initproj):
+    return initproj(
         "pkg123-0.7",
         filedefs={
             "tox.ini": """
             [tox]
+            skipsdist = True
             envlist = a, b
-            isolated_build = true
             [testenv]
+            skip_install = True
             commands=python -c "import sys; print(sys.executable)"
-            [testenv:b]
-            depends = a
-        """,
-            "pyproject.toml": """
-            [build-system]
-            requires = ["setuptools >= 35.0.2"]
-            build-backend = 'setuptools.build_meta'
-                        """,
+        """
         },
     )
-    result = cmd("--parallel", "all")
+
+
+def test_parallel_no_spinner_on(cmd, parallel_project, monkeypatch):
+    monkeypatch.setenv(str("TOX_PARALLEL_NO_SPINNER"), str("1"))
+    result = cmd("-p", "all")
+    result.assert_success()
+    assert "[2] a | b" not in result.out
+
+
+def test_parallel_no_spinner_off(cmd, parallel_project, monkeypatch):
+    monkeypatch.setenv(str("TOX_PARALLEL_NO_SPINNER"), str("0"))
+    result = cmd("-p", "all")
     result.assert_success()
+    assert "[2] a | b" in result.out
+
+
+def test_parallel_no_spinner_not_set(cmd, parallel_project, monkeypatch):
+    monkeypatch.delenv(str("TOX_PARALLEL_NO_SPINNER"), raising=False)
+    result = cmd("-p", "all")
+    result.assert_success()
+    assert "[2] a | b" in result.out
+
+
+def test_parallel_result_json(cmd, parallel_project, tmp_path):
+    parallel_result_json = tmp_path / "parallel.json"
+    result = cmd("-p", "all", "--result-json", "{}".format(parallel_result_json))
+    ensure_result_json_ok(result, parallel_result_json)
+
+
+def ensure_result_json_ok(result, json_path):
+    if isinstance(result, RunResult):
+        result.assert_success()
+    else:
+        assert not isinstance(result, subprocess.CalledProcessError)
+    assert json_path.exists()
+    serial_data = json.loads(json_path.read_text())
+    ensure_key_in_env(serial_data)
+
+
+def ensure_key_in_env(serial_data):
+    for env in ("a", "b"):
+        for key in ("setup", "test"):
+            assert key in serial_data["testenvs"][env], json.dumps(
+                serial_data["testenvs"], indent=2
+            )
+
+
+def test_parallel_result_json_concurrent(cmd, parallel_project, tmp_path):
+    # first run to set up the environments (env creation is not thread safe)
+    result = cmd("-p", "all")
+    result.assert_success()
+
+    invoke_result = {}
+
+    def invoke_tox_in_thread(thread_name, result_json):
+        try:
+            # needs to be process to have it's own stdout
+            invoke_result[thread_name] = subprocess.check_output(
+                [sys.executable, "-m", "tox", "-p", "all", "--result-json", str(result_json)],
+                universal_newlines=True,
+            )
+        except subprocess.CalledProcessError as exception:
+            invoke_result[thread_name] = exception
+
+    # now concurrently
+    parallel1_result_json = tmp_path / "parallel1.json"
+    parallel2_result_json = tmp_path / "parallel2.json"
+    threads = [
+        threading.Thread(target=invoke_tox_in_thread, args=(k, p))
+        for k, p in (("t1", parallel1_result_json), ("t2", parallel2_result_json))
+    ]
+    [t.start() for t in threads]
+    [t.join() for t in threads]
+
+    ensure_result_json_ok(invoke_result["t1"], parallel1_result_json)
+    ensure_result_json_ok(invoke_result["t2"], parallel2_result_json)
+    # our set_os_env_var is not thread-safe so clean-up TOX_WORK_DIR
+    os.environ.pop("TOX_WORK_DIR", None)
diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py
index ed889809..60941971 100644
--- a/tests/unit/test_z_cmdline.py
+++ b/tests/unit/test_z_cmdline.py
@@ -500,7 +500,7 @@ def test_result_json(cmd, initproj, example123):
         assert isinstance(pyinfo["version_info"], list)
         assert pyinfo["version"]
         assert pyinfo["executable"]
-    assert "wrote json report at: {}".format(json_path) == result.outlines[-1]
+    assert "write json report at: {}".format(json_path) == result.outlines[-1]
 
 
 def test_developz(initproj, cmd):