From 0dd7a5e7a85beaeb34c39bc1edc920f63530177a Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 12 Mar 2019 11:40:27 +0100 Subject: [PATCH 1/9] fix paralell runs keyboard interrupt leaves children running --- docs/changelog/1172.bugfix.rst | 2 + setup.cfg | 1 + src/tox/reporter.py | 2 +- src/tox/session/__init__.py | 2 +- src/tox/session/commands/run/parallel.py | 71 ++++++++++++++------ tests/integration/test_parallel_interrupt.py | 66 ++++++++++++++++++ 6 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 docs/changelog/1172.bugfix.rst create mode 100644 tests/integration/test_parallel_interrupt.py diff --git a/docs/changelog/1172.bugfix.rst b/docs/changelog/1172.bugfix.rst new file mode 100644 index 000000000..9188dc459 --- /dev/null +++ b/docs/changelog/1172.bugfix.rst @@ -0,0 +1,2 @@ +Interrupting parallel runs via keyboard interrupt no longer leaves the child runs running +(now forwards the keyboard interrupt to the children and waits until they finish) - by :user:`gaborbernat`. diff --git a/setup.cfg b/setup.cfg index a4a69df4f..cde21736f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -63,6 +63,7 @@ testing = pytest-timeout >= 1.3.0, <2 pytest-xdist >= 1.22.2, <2 pytest-randomly >= 1.2.3, <2 + psutil >= 5.6.1, < 6; python_version != "3.4" docs = sphinx >= 1.8.0, < 2 towncrier >= 18.5.0 diff --git a/src/tox/reporter.py b/src/tox/reporter.py index ed287957e..193492130 100644 --- a/src/tox/reporter.py +++ b/src/tox/reporter.py @@ -67,7 +67,7 @@ def logline_if(self, level, of, msg, key=None, **kwargs): def logline(self, of, msg, **opts): self.reported_lines.append((of, msg)) - self.tw.line("{}".format(msg), **opts) + self.tw.write("{}\n".format(msg), **opts) def keyvalue(self, name, value): if name.endswith(":"): diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py index 251d74ca3..c5ca191f6 100644 --- a/src/tox/session/__init__.py +++ b/src/tox/session/__init__.py @@ -218,7 +218,7 @@ def _summary(self): exit_code = 0 for venv in self.venv_dict.values(): report = reporter.good - status = venv.status + status = getattr(venv, "status", "undefined") if isinstance(status, tox.exception.InterpreterNotFound): msg = " {}: {}".format(venv.envconfig.envname, str(status)) if self.config.option.skip_missing_interpreters == "true": diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py index 739a0ac72..12ea21dcc 100644 --- a/src/tox/session/commands/run/parallel.py +++ b/src/tox/session/commands/run/parallel.py @@ -1,4 +1,5 @@ import os +import signal import subprocess import sys import tempfile @@ -40,7 +41,7 @@ def run_parallel(config, venv_dict): show_progress = not live_out and reporter.verbosity() > reporter.Verbosity.QUIET with Spinner(enabled=show_progress) as spinner, ctx() as sink: - def run_in_thread(tox_env, os_env): + def run_in_thread(tox_env, os_env, processes): res = None env_name = tox_env.envconfig.envname try: @@ -57,6 +58,8 @@ def run_in_thread(tox_env, os_env): stdin=None, universal_newlines=True, ) + processes[env_name] = process + reporter.verbosity2("started {} with pid {}".format(env_name, process.pid)) res = process.wait() finally: semaphore.release() @@ -87,25 +90,55 @@ def run_in_thread(tox_env, os_env): reporter.quiet(message) threads = [] + processes = {} todo_keys = set(venv_dict.keys()) todo = OrderedDict((n, todo_keys & set(v.envconfig.depends)) for n, v in venv_dict.items()) done = set() - while todo: - for name, depends in list(todo.items()): - if depends - done: - # skip if has unfinished dependencies + try: + while todo: + for name, depends in list(todo.items()): + if depends - done: + # skip if has unfinished dependencies + continue + del todo[name] + venv = venv_dict[name] + semaphore.acquire(blocking=True) + spinner.add(name) + thread = Thread( + target=run_in_thread, args=(venv, os.environ.copy(), processes) + ) + thread.daemon = True + thread.start() + threads.append(thread) + if todo: + # wait until someone finishes and retry queuing jobs + finished.wait() + finished.clear() + for thread in threads: + while thread.is_alive(): + thread.join(0.1) + # join suspends signal handling (ctrl+c), periodically time-out to check for it + except KeyboardInterrupt: + reporter.verbosity0("keyboard interrupt") + while True: + # do not allow to interrupt until children interrupt + try: + # first try to request a graceful shutdown + for name, proc in list(processes.items()): + if proc.returncode is None: + proc.send_signal( + signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT + ) + reporter.verbosity2("send CTRL+C {}-{}".format(name, proc.pid)) + for thread in threads: + thread.join(0.1) + # now if being gentle did not work now be forceful + for name, proc in list(processes.items()): + if proc.returncode is None: + proc.terminate() + reporter.verbosity2("terminate {}-{}".format(name, proc.pid)) + for thread in threads: + thread.join() + except KeyboardInterrupt: continue - del todo[name] - venv = venv_dict[name] - semaphore.acquire(blocking=True) - spinner.add(name) - thread = Thread(target=run_in_thread, args=(venv, os.environ.copy())) - thread.start() - threads.append(thread) - if todo: - # wait until someone finishes and retry queuing jobs - finished.wait() - finished.clear() - - for thread in threads: - thread.join() + raise KeyboardInterrupt diff --git a/tests/integration/test_parallel_interrupt.py b/tests/integration/test_parallel_interrupt.py new file mode 100644 index 000000000..e3ba67eba --- /dev/null +++ b/tests/integration/test_parallel_interrupt.py @@ -0,0 +1,66 @@ +from __future__ import absolute_import, unicode_literals + +import signal +import subprocess +import sys +from datetime import datetime + +from pathlib2 import Path + + +def test_parallel_interrupt(initproj, cmd): + + start = datetime.now() + initproj( + "pkg123-0.7", + filedefs={ + "tox.ini": """ + [tox] + skipsdist=True + envlist = a, b + [testenv] + commands = python -c "open('{envname}', 'w').write('done'); \ + import time; time.sleep(3)" + + """ + }, + ) + cmd = [sys.executable, "-m", "tox", "-v", "-v", "-p", "all"] + process = subprocess.Popen( + cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True + ) + try: + import psutil + + current_process = psutil.Process(process.pid) + except ImportError: + current_process = None + + # we wait until all environments are up and running + signal_files = [Path() / "a", Path() / "b"] + found = False + while True: + if all(signal_file.exists() for signal_file in signal_files): + found = True + break + if process.poll() is not None: + break + if not found: + out, _ = process.communicate() + out = out.encode("utf-8") + missing = [f for f in signal_files if not f.exists()] + assert len(missing), out + + if current_process: + all_children = current_process.children(recursive=True) + assert all_children + + end = datetime.now() - start + assert end + process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + out, _ = process.communicate() + assert "\nERROR: a: parallel child exit code " in out, out + assert "\nERROR: b: parallel child exit code " in out, out + + if current_process: + assert all(not children.is_running() for children in all_children) From 934ce923bbfe5429be7c62647dcd3af50f5718a8 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Fri, 15 Mar 2019 19:46:17 +0100 Subject: [PATCH 2/9] add time report, use host envs --- src/tox/config/__init__.py | 3 +- src/tox/reporter.py | 13 +++++- src/tox/session/commands/run/parallel.py | 26 +++++------ src/tox/util/stdlib.py | 11 +++++ tests/integration/test_parallel_interrupt.py | 49 ++++++++++++-------- 5 files changed, 66 insertions(+), 36 deletions(-) create mode 100644 src/tox/util/stdlib.py diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index 16d513371..86feab9bc 100644 --- a/src/tox/config/__init__.py +++ b/src/tox/config/__init__.py @@ -21,7 +21,7 @@ import tox from tox.constants import INFO from tox.interpreters import Interpreters, NoInterpreterInfo -from tox.reporter import update_default_reporter +from tox.reporter import REPORTER_TIMESTAMP_ON_ENV, update_default_reporter from .parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY from .parallel import add_parallel_config, add_parallel_flags @@ -657,6 +657,7 @@ def passenv(testenv_config, value): "LANGUAGE", "LD_LIBRARY_PATH", "TOX_WORK_DIR", + str(REPORTER_TIMESTAMP_ON_ENV), str(PARALLEL_ENV_VAR_KEY), } diff --git a/src/tox/reporter.py b/src/tox/reporter.py index 193492130..10f6cfcd4 100644 --- a/src/tox/reporter.py +++ b/src/tox/reporter.py @@ -1,8 +1,10 @@ """A progress reporter inspired from the logging modules""" from __future__ import absolute_import, unicode_literals +import os import time from contextlib import contextmanager +from datetime import datetime import py @@ -15,6 +17,11 @@ class Verbosity(object): EXTRA_QUIET = -2 +REPORTER_TIMESTAMP_ON_ENV = "TOX_REPORTER_TIMESTAMP" +REPORTER_TIMESTAMP_ON = os.environ.get(REPORTER_TIMESTAMP_ON_ENV, False) == "1" +START = datetime.now().timestamp() + + class Reporter(object): def __init__(self, verbose_level=None, quiet_level=None): kwargs = {} @@ -67,7 +74,11 @@ def logline_if(self, level, of, msg, key=None, **kwargs): def logline(self, of, msg, **opts): self.reported_lines.append((of, msg)) - self.tw.write("{}\n".format(msg), **opts) + timestamp = "" + if REPORTER_TIMESTAMP_ON: + timestamp = "{} ".format(datetime.now().timestamp() - START) + line_msg = "{}{}\n".format(timestamp, msg) + self.tw.write(line_msg, **opts) def keyvalue(self, name, value): if name.endswith(":"): diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py index 12ea21dcc..660ebe682 100644 --- a/src/tox/session/commands/run/parallel.py +++ b/src/tox/session/commands/run/parallel.py @@ -1,29 +1,26 @@ +from collections import OrderedDict + +import inspect import os import signal import subprocess import sys import tempfile -from collections import OrderedDict from threading import Event, Semaphore, Thread +from tox import __main__ as main from tox import reporter from tox.config.parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY from tox.util.spinner import Spinner +from tox.util.stdlib import nullcontext -if sys.version_info >= (3, 7): - from contextlib import nullcontext -else: - import contextlib - - @contextlib.contextmanager - def nullcontext(enter_result=None): - yield enter_result +MAIN_FILE = inspect.getsourcefile(main) def run_parallel(config, venv_dict): """here we'll just start parallel sub-processes""" live_out = config.option.parallel_live - args = [sys.executable, "-m", "tox"] + config.args + args = [sys.executable, MAIN_FILE] + config.args try: position = args.index("--") except ValueError: @@ -116,10 +113,10 @@ def run_in_thread(tox_env, os_env, processes): finished.clear() for thread in threads: while thread.is_alive(): - thread.join(0.1) + thread.join(0.05) # join suspends signal handling (ctrl+c), periodically time-out to check for it except KeyboardInterrupt: - reporter.verbosity0("keyboard interrupt") + reporter.verbosity0("keyboard interrupt parallel - stopping children") while True: # do not allow to interrupt until children interrupt try: @@ -130,8 +127,9 @@ def run_in_thread(tox_env, os_env, processes): signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT ) reporter.verbosity2("send CTRL+C {}-{}".format(name, proc.pid)) - for thread in threads: - thread.join(0.1) + if len(threads): + threads[0].join(0.2) # wait at most 200ms for all to finish + # now if being gentle did not work now be forceful for name, proc in list(processes.items()): if proc.returncode is None: diff --git a/src/tox/util/stdlib.py b/src/tox/util/stdlib.py new file mode 100644 index 000000000..c9c052773 --- /dev/null +++ b/src/tox/util/stdlib.py @@ -0,0 +1,11 @@ +import sys + +if sys.version_info >= (3, 7): + from contextlib import nullcontext +else: + import contextlib + + + @contextlib.contextmanager + def nullcontext(enter_result=None): + yield enter_result diff --git a/tests/integration/test_parallel_interrupt.py b/tests/integration/test_parallel_interrupt.py index e3ba67eba..02e25df7e 100644 --- a/tests/integration/test_parallel_interrupt.py +++ b/tests/integration/test_parallel_interrupt.py @@ -7,25 +7,29 @@ from pathlib2 import Path +from tox import __main__ as main +from tox import reporter -def test_parallel_interrupt(initproj, cmd): +def test_parallel_interrupt(initproj, cmd, monkeypatch): + monkeypatch.setenv(reporter.REPORTER_TIMESTAMP_ON_ENV, "1") + monkeypatch.setattr(reporter, "REPORTER_TIMESTAMP_ON", True) start = datetime.now() initproj( "pkg123-0.7", filedefs={ "tox.ini": """ [tox] - skipsdist=True envlist = a, b [testenv] + host_env = True commands = python -c "open('{envname}', 'w').write('done'); \ import time; time.sleep(3)" """ }, ) - cmd = [sys.executable, "-m", "tox", "-v", "-v", "-p", "all"] + cmd = [sys.executable, main.__file__, "-v", "-v", "-p", "all", "-o"] process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True ) @@ -36,22 +40,10 @@ def test_parallel_interrupt(initproj, cmd): except ImportError: current_process = None - # we wait until all environments are up and running - signal_files = [Path() / "a", Path() / "b"] - found = False - while True: - if all(signal_file.exists() for signal_file in signal_files): - found = True - break - if process.poll() is not None: - break - if not found: - out, _ = process.communicate() - out = out.encode("utf-8") - missing = [f for f in signal_files if not f.exists()] - assert len(missing), out + wait_for_env_startup(process) - if current_process: + all_children = [] + if current_process is not None: all_children = current_process.children(recursive=True) assert all_children @@ -59,8 +51,25 @@ def test_parallel_interrupt(initproj, cmd): assert end process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) out, _ = process.communicate() + assert "keyboard interrupt parallel - stopping children" in out, out assert "\nERROR: a: parallel child exit code " in out, out assert "\nERROR: b: parallel child exit code " in out, out - if current_process: - assert all(not children.is_running() for children in all_children) + assert all(not children.is_running() for children in all_children) + + +def wait_for_env_startup(process): + """the environments will write files once they are up""" + signal_files = [Path() / "a", Path() / "b"] + found = False + while True: + if all(signal_file.exists() for signal_file in signal_files): + found = True + break + if process.poll() is not None: + break + if not found or process.poll() is not None: + missing = [f for f in signal_files if not f.exists()] + out, _ = process.communicate() + assert len(missing), out + assert False, out From c9b891df78422406ed0139238fa53004f9ef1863 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 19 Mar 2019 16:13:51 +0000 Subject: [PATCH 3/9] fix parallel run --- azure-pipelines.yml | 3 ++- src/tox/action.py | 14 +++++++++++++- src/tox/reporter.py | 6 +++--- src/tox/session/commands/run/parallel.py | 2 +- src/tox/venv.py | 3 +++ tests/integration/test_parallel_interrupt.py | 17 +++++++++-------- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 5c2f8cb6e..78b800020 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -5,9 +5,10 @@ variables: CI_NAME: Azure Pipelines CI_BUILD_ID: $(Build.BuildId) CI_BUILD_URL: "https://toxdev.visualstudio.com/tox/_build/results?buildId=$(Build.BuildId)" - PYTEST_ADDOPTS: "-vv -ra --showlocals" + PYTEST_ADDOPTS: "-s -v -v -ra --showlocals" GIT_BRANCH: $[ coalesce(variables['System.PullRequest.SourceBranch'], variables['Build.SourceBranchName'], 'not-found') ] GIT_COMMIT_SHA: $[ coalesce(variables['System.PullRequest.SourceCommitId'], variables['Build.SourceVersion'], 'not-found') ] + PYTEST_XDIST_PROC_NR: 0 trigger: batch: true diff --git a/src/tox/action.py b/src/tox/action.py index 801aa5261..1fb4a6ccc 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -2,6 +2,7 @@ import os import pipes +import signal import subprocess import sys import time @@ -78,6 +79,12 @@ def popen( env=os.environ.copy() if env is None else env, universal_newlines=True, shell=False, + creationflags=( + subprocess.CREATE_NEW_PROCESS_GROUP + if sys.platform == "win32" + else 0 + # needed for Windows signal send ability (CTRL+C) + ), ) except OSError as e: reporter.error( @@ -140,7 +147,12 @@ def feed_stdin(self, fin, process, redirect): else: out, err = process.communicate() except KeyboardInterrupt: - process.wait() + process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + try: + process.wait(0.1) + except subprocess.TimeoutExpired: + process.terminate() + process.wait() raise return out diff --git a/src/tox/reporter.py b/src/tox/reporter.py index 10f6cfcd4..2c2a52451 100644 --- a/src/tox/reporter.py +++ b/src/tox/reporter.py @@ -17,9 +17,9 @@ class Verbosity(object): EXTRA_QUIET = -2 -REPORTER_TIMESTAMP_ON_ENV = "TOX_REPORTER_TIMESTAMP" +REPORTER_TIMESTAMP_ON_ENV = str("TOX_REPORTER_TIMESTAMP") REPORTER_TIMESTAMP_ON = os.environ.get(REPORTER_TIMESTAMP_ON_ENV, False) == "1" -START = datetime.now().timestamp() +START = datetime.now() class Reporter(object): @@ -76,7 +76,7 @@ def logline(self, of, msg, **opts): self.reported_lines.append((of, msg)) timestamp = "" if REPORTER_TIMESTAMP_ON: - timestamp = "{} ".format(datetime.now().timestamp() - START) + timestamp = "{} ".format(datetime.now() - START) line_msg = "{}{}\n".format(timestamp, msg) self.tw.write(line_msg, **opts) diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py index 660ebe682..2382eb7c7 100644 --- a/src/tox/session/commands/run/parallel.py +++ b/src/tox/session/commands/run/parallel.py @@ -128,7 +128,7 @@ def run_in_thread(tox_env, os_env, processes): ) reporter.verbosity2("send CTRL+C {}-{}".format(name, proc.pid)) if len(threads): - threads[0].join(0.2) # wait at most 200ms for all to finish + threads[0].join(1.2) # wait at most 200ms for all to finish # now if being gentle did not work now be forceful for name, proc in list(processes.items()): diff --git a/src/tox/venv.py b/src/tox/venv.py index 9289af68b..0290035a5 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -636,10 +636,13 @@ def prepend_shebang_interpreter(args): NO_DOWNLOAD = False +_SKIP_VENV_CREATION = os.environ.get("_TOX_SKIP_ENV_CREATION_TEST", False) == "1" @tox.hookimpl def tox_testenv_create(venv, action): + if _SKIP_VENV_CREATION is True: + return True config_interpreter = venv.getsupportedinterpreter() args = [sys.executable, "-m", "virtualenv"] if venv.envconfig.sitepackages: diff --git a/tests/integration/test_parallel_interrupt.py b/tests/integration/test_parallel_interrupt.py index 02e25df7e..af6052a02 100644 --- a/tests/integration/test_parallel_interrupt.py +++ b/tests/integration/test_parallel_interrupt.py @@ -8,12 +8,10 @@ from pathlib2 import Path from tox import __main__ as main -from tox import reporter def test_parallel_interrupt(initproj, cmd, monkeypatch): - monkeypatch.setenv(reporter.REPORTER_TIMESTAMP_ON_ENV, "1") - monkeypatch.setattr(reporter, "REPORTER_TIMESTAMP_ON", True) + monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) start = datetime.now() initproj( "pkg123-0.7", @@ -21,12 +19,16 @@ def test_parallel_interrupt(initproj, cmd, monkeypatch): "tox.ini": """ [tox] envlist = a, b + [testenv] - host_env = True - commands = python -c "open('{envname}', 'w').write('done'); \ - import time; time.sleep(3)" + skip_install = True + commands = python -c "open('{{envname}}', 'w').write('done'); \ + import time; time.sleep(10)" + whitelist_externals = {} - """ + """.format( + sys.executable + ) }, ) cmd = [sys.executable, main.__file__, "-v", "-v", "-p", "all", "-o"] @@ -54,7 +56,6 @@ def test_parallel_interrupt(initproj, cmd, monkeypatch): assert "keyboard interrupt parallel - stopping children" in out, out assert "\nERROR: a: parallel child exit code " in out, out assert "\nERROR: b: parallel child exit code " in out, out - assert all(not children.is_running() for children in all_children) From ad864a94adf190ecbf56da83bc27c58d7722dc5c Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Mon, 25 Mar 2019 12:41:33 +0000 Subject: [PATCH 4/9] more fixes --- src/tox/_pytestplugin.py | 7 + src/tox/action.py | 79 ++++++++---- src/tox/config/__init__.py | 20 ++- src/tox/exception.py | 4 + src/tox/package/__init__.py | 2 +- src/tox/reporter.py | 4 +- src/tox/session/__init__.py | 7 +- src/tox/session/commands/provision.py | 25 +--- src/tox/session/commands/run/parallel.py | 128 +++++++++---------- src/tox/util/lock.py | 12 +- src/tox/util/main.py | 5 + src/tox/util/stdlib.py | 16 +-- src/tox/venv.py | 1 - tests/integration/test_parallel_interrupt.py | 31 +++-- tests/integration/test_provision_int.py | 51 ++++++-- tests/unit/session/test_parallel.py | 37 ++++-- tests/unit/session/test_provision.py | 16 ++- tox.ini | 12 +- 18 files changed, 275 insertions(+), 182 deletions(-) create mode 100644 src/tox/util/main.py diff --git a/src/tox/_pytestplugin.py b/src/tox/_pytestplugin.py index b297fa466..828ee5126 100644 --- a/src/tox/_pytestplugin.py +++ b/src/tox/_pytestplugin.py @@ -102,6 +102,11 @@ def run_command(self): result.ret = exception.code except OSError as e: result.ret = e.errno + except tox.exception.InvocationError as exception: + result.ret = exception.exit_code + if exception.out is not None: + with open(exception.out, "rt") as file_handler: + tox.reporter.verbosity0(file_handler.read()) return result yield run @@ -207,6 +212,8 @@ def __init__(self, args, cwd, env, stdout, stderr, shell): self.stdout = stdout self.stderr = stderr self.shell = shell + self.pid = os.getpid() + self.returncode = 0 @staticmethod def communicate(): diff --git a/src/tox/action.py b/src/tox/action.py index 1fb4a6ccc..1df5c144f 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -7,6 +7,7 @@ import sys import time from contextlib import contextmanager +from threading import Thread import py @@ -14,6 +15,10 @@ from tox.constants import INFO from tox.exception import InvocationError from tox.util.lock import get_unique_file +from tox.util.stdlib import is_main_thread + +WAIT_INTERRUPT = 0.3 +WAIT_TERMINATE = 0.2 class Action(object): @@ -60,6 +65,7 @@ def popen( returnout=False, ignore_ret=False, capture_err=True, + callback=None, ): """this drives an interaction with a subprocess""" cmd_args = [str(x) for x in args] @@ -69,8 +75,8 @@ def popen( ) cwd = os.getcwd() if cwd is None else cwd with stream_getter as (fin, out_path, stderr, stdout): + args = self._rewrite_args(cwd, args) try: - args = self._rewrite_args(cwd, args) process = self.via_popen( args, stdout=stdout, @@ -93,9 +99,12 @@ def popen( ) ) raise - reporter.log_popen(cwd, out_path, cmd_args_shell) - output = self.feed_stdin(fin, process, redirect) - exit_code = process.wait() + if callback is not None: + callback(process) + reporter.log_popen(cwd, out_path, cmd_args_shell, process.pid) + + output = self.evaluate_cmd(fin, process, redirect) + exit_code = process.returncode if exit_code and not ignore_ret: invoked = " ".join(map(str, args)) if out_path: @@ -113,7 +122,7 @@ def popen( self.command_log.add_command(args, output, exit_code) return output - def feed_stdin(self, fin, process, redirect): + def evaluate_cmd(self, input_file_handler, process, redirect): try: if self.generate_tox_log and not redirect: if process.stderr is not None: @@ -126,7 +135,7 @@ def feed_stdin(self, fin, process, redirect): while True: # we have to read one byte at a time, otherwise there # might be no output for a long time with slow tests - data = fin.read(1) + data = input_file_handler.read(1) if data: buf.write(data) if b"\n" in data or (time.time() - last_time) > 1: @@ -142,48 +151,72 @@ def feed_stdin(self, fin, process, redirect): else: time.sleep(0.1) # the seek updates internal read buffers - fin.seek(0, 1) - fin.close() + input_file_handler.seek(0, 1) + input_file_handler.close() + process.wait() # wait to finish else: - out, err = process.communicate() - except KeyboardInterrupt: - process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + out, err = process.communicate() # wait to finish + except KeyboardInterrupt as exception: + main_thread = is_main_thread() + while True: + try: + if main_thread: + # spin up a new thread to disable further interrupt on main thread + stopper = Thread(target=self.handle_interrupt, args=(process,)) + stopper.start() + stopper.join() + else: + self.handle_interrupt(process) + except KeyboardInterrupt: + continue + break + raise exception + return out + + def handle_interrupt(self, process): + """A three level stop mechanism for children - INT -> TERM -> KILL""" + msg = "from {} {{}} pid {}".format(os.getpid(), process.pid) + self.info("KeyboardInterrupt", msg.format("SIGINT")) + process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + try: + process.wait(WAIT_INTERRUPT) + except subprocess.TimeoutExpired: + self.info("KeyboardInterrupt", msg.format("SIGTERM")) + process.terminate() try: - process.wait(0.1) + process.wait(WAIT_TERMINATE) except subprocess.TimeoutExpired: - process.terminate() + self.info("KeyboardInterrupt", msg.format("SIGKILL")) + process.kill() process.wait() - raise - return out @contextmanager def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout): - stdout = out_path = fin = None + stdout = out_path = input_file_handler = None stderr = subprocess.STDOUT if capture_err else None if self.generate_tox_log or redirect: out_path = self.get_log_path(self.name) - with out_path.open("wt") as stdout, out_path.open("rb") as fin: + with out_path.open("wt") as stdout, out_path.open("rb") as input_file_handler: stdout.write( "actionid: {}\nmsg: {}\ncmdargs: {!r}\n\n".format( self.name, self.msg, cmd_args_shell ) ) stdout.flush() - fin.read() # read the header, so it won't be written to stdout + input_file_handler.read() # read the header, so it won't be written to stdout - yield fin, out_path, stderr, stdout + yield input_file_handler, out_path, stderr, stdout return if returnout: stdout = subprocess.PIPE - yield fin, out_path, stderr, stdout + yield input_file_handler, out_path, stderr, stdout def get_log_path(self, actionid): - return get_unique_file( - self.log_dir, prefix=actionid, suffix=".logs", report=reporter.verbosity1 - ) + log_file = get_unique_file(self.log_dir, prefix=actionid, suffix=".log") + return log_file def _rewrite_args(self, cwd, args): new_args = [] diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index 86feab9bc..5f82e78cf 100644 --- a/src/tox/config/__init__.py +++ b/src/tox/config/__init__.py @@ -21,7 +21,13 @@ import tox from tox.constants import INFO from tox.interpreters import Interpreters, NoInterpreterInfo -from tox.reporter import REPORTER_TIMESTAMP_ON_ENV, update_default_reporter +from tox.reporter import ( + REPORTER_TIMESTAMP_ON_ENV, + error, + update_default_reporter, + using, + verbosity1, +) from .parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY from .parallel import add_parallel_config, add_parallel_flags @@ -950,6 +956,7 @@ def make_hashseed(): class ParseIni(object): def __init__(self, config, ini_path, ini_data): # noqa config.toxinipath = ini_path + using("tox.ini: {} (pid {})".format(config.toxinipath, os.getpid())) config.toxinidir = config.toxinipath.dirpath() self._cfg = py.iniconfig.IniConfig(config.toxinipath, ini_data) @@ -1081,14 +1088,19 @@ def handle_provision(self, config, reader): def ensure_requires_satisfied(config, env_config): missing_requirements = [] deps = env_config.deps + failed_to_parse = False for require in deps: # noinspection PyBroadException try: pkg_resources.get_distribution(require.name) - except pkg_resources.RequirementParseError: - raise - except Exception: + except pkg_resources.RequirementParseError as exception: + failed_to_parse = True + error("failed to parse {!r}".format(exception)) + except Exception as exception: + verbosity1("could not satisfy requires {!r}".format(exception)) missing_requirements.append(str(pkg_resources.Requirement(require.name))) + if failed_to_parse: + raise tox.exception.BadRequirement() config.run_provision = bool(missing_requirements) if missing_requirements: config.envconfigs[config.provision_tox_env] = env_config diff --git a/src/tox/exception.py b/src/tox/exception.py index e48cfbb66..4c9a3262d 100644 --- a/src/tox/exception.py +++ b/src/tox/exception.py @@ -86,3 +86,7 @@ def __init__(self, config): def __str__(self): return " ".join(pipes.quote(i) for i in self.config.requires) + + +class BadRequirement(Error): + """A requirement defined in :config:`require` cannot be parsed.""" diff --git a/src/tox/package/__init__.py b/src/tox/package/__init__.py index e3bc19573..f6925162a 100644 --- a/src/tox/package/__init__.py +++ b/src/tox/package/__init__.py @@ -2,7 +2,7 @@ import tox from tox.reporter import error, info, verbosity0, verbosity2, warning -from tox.util.lock import get as hold_lock +from tox.util.lock import hold_lock from .builder import build_package from .local import resolve_package diff --git a/src/tox/reporter.py b/src/tox/reporter.py index 2c2a52451..859ff085e 100644 --- a/src/tox/reporter.py +++ b/src/tox/reporter.py @@ -41,9 +41,9 @@ def _reset(self, verbose_level=0, quiet_level=0): def verbosity(self): return self.verbose_level - self.quiet_level - def log_popen(self, cwd, outpath, cmd_args_shell): + def log_popen(self, cwd, outpath, cmd_args_shell, pid): """ log information about the action.popen() created process. """ - msg = " {}$ {} ".format(cwd, cmd_args_shell) + msg = "[{}] {}$ {} ".format(pid, cwd, cmd_args_shell) if outpath: msg = "{} >{}".format(msg, outpath) self.verbosity1(msg, of="logpopen") diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py index c5ca191f6..ef06b525e 100644 --- a/src/tox/session/__init__.py +++ b/src/tox/session/__init__.py @@ -56,7 +56,6 @@ def main(args): setup_reporter(args) try: config = load_config(args) - reporter.using("tox.ini: {}".format(config.toxinipath)) config.logdir.ensure(dir=1) ensure_empty_dir(config.logdir) with set_os_env_var("TOX_WORK_DIR", config.toxworkdir): @@ -64,6 +63,8 @@ def main(args): if retcode is None: retcode = 0 raise SystemExit(retcode) + except tox.exception.BadRequirement: + raise SystemExit(1) except KeyboardInterrupt: raise SystemExit(2) @@ -166,7 +167,9 @@ def newaction(self, name, msg, *args): ) def runcommand(self): - reporter.using("tox-{} from {}".format(tox.__version__, tox.__file__)) + reporter.using( + "tox-{} from {} (pid {})".format(tox.__version__, tox.__file__, os.getpid()) + ) show_description = reporter.has_level(reporter.Verbosity.DEFAULT) if self.config.run_provision: provision_tox_venv = self.getvenv(self.config.provision_tox_env) diff --git a/src/tox/session/commands/provision.py b/src/tox/session/commands/provision.py index f6779efe2..a694eb72a 100644 --- a/src/tox/session/commands/provision.py +++ b/src/tox/session/commands/provision.py @@ -1,31 +1,14 @@ """In case the tox environment is not correctly setup provision it and delegate execution""" -import signal -import subprocess +from tox.util.main import MAIN_FILE def provision_tox(provision_venv, args): ensure_meta_env_up_to_date(provision_venv) - process = start_meta_tox(args, provision_venv) - result_out = wait_for_meta_tox(process) - raise SystemExit(result_out) + with provision_venv.new_action("provision") as action: + provision_args = [str(provision_venv.envconfig.envpython), MAIN_FILE] + args + action.popen(provision_args, redirect=False) def ensure_meta_env_up_to_date(provision_venv): if provision_venv.setupenv(): provision_venv.finishvenv() - - -def start_meta_tox(args, provision_venv): - provision_args = [str(provision_venv.envconfig.envpython), "-m", "tox"] + args - process = subprocess.Popen(provision_args) - return process - - -def wait_for_meta_tox(process): - try: - result_out = process.wait() - except KeyboardInterrupt: - # if we try to interrupt delegate interrupt to meta tox - process.send_signal(signal.SIGINT) - result_out = process.wait() - return result_out diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py index 2382eb7c7..2ace99171 100644 --- a/src/tox/session/commands/run/parallel.py +++ b/src/tox/session/commands/run/parallel.py @@ -1,20 +1,13 @@ -from collections import OrderedDict - -import inspect import os -import signal -import subprocess import sys -import tempfile +from collections import OrderedDict from threading import Event, Semaphore, Thread -from tox import __main__ as main from tox import reporter from tox.config.parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY +from tox.exception import InvocationError +from tox.util.main import MAIN_FILE from tox.util.spinner import Spinner -from tox.util.stdlib import nullcontext - -MAIN_FILE = inspect.getsourcefile(main) def run_parallel(config, venv_dict): @@ -32,60 +25,45 @@ def run_parallel(config, venv_dict): semaphore = Semaphore(max_parallel) finished = Event() - ctx = nullcontext if live_out else tempfile.NamedTemporaryFile - stderr = None if live_out else subprocess.STDOUT - show_progress = not live_out and reporter.verbosity() > reporter.Verbosity.QUIET - with Spinner(enabled=show_progress) as spinner, ctx() as sink: + with Spinner(enabled=show_progress) as spinner: def run_in_thread(tox_env, os_env, processes): - res = None env_name = tox_env.envconfig.envname + status = "skipped tests" if config.option.notest else None try: os_env[str(PARALLEL_ENV_VAR_KEY)] = str(env_name) args_sub = list(args) if hasattr(tox_env, "package"): args_sub.insert(position, str(tox_env.package)) args_sub.insert(position, "--installpkg") - process = subprocess.Popen( - args_sub, - env=os_env, - stdout=sink, - stderr=stderr, - stdin=None, - universal_newlines=True, - ) - processes[env_name] = process - reporter.verbosity2("started {} with pid {}".format(env_name, process.pid)) - res = process.wait() + with tox_env.new_action("parallel {}".format(tox_env.name)) as action: + + def collect_process(process): + processes[tox_env] = (action, process) + + action.popen( + args=args_sub, + env=os_env, + redirect=not live_out, + capture_err=live_out, + callback=collect_process, + ) + + except InvocationError as err: + status = "parallel child exit code {}".format(err.exit_code) finally: semaphore.release() finished.set() - tox_env.status = ( - "skipped tests" - if config.option.notest - else ("parallel child exit code {}".format(res) if res else res) - ) + tox_env.status = status done.add(env_name) outcome = spinner.succeed if config.option.notest: outcome = spinner.skip - elif res: + elif status is not None: outcome = spinner.fail outcome(env_name) - if not live_out: - sink.seek(0) - out = sink.read().decode("UTF-8", errors="replace") - if res or tox_env.envconfig.parallel_show_output: - outcome = ( - "Failed {} under process {}, stdout:\n".format(env_name, process.pid) - if res - else "" - ) - message = "{}{}".format(outcome, out).rstrip() - reporter.quiet(message) - threads = [] processes = {} todo_keys = set(venv_dict.keys()) @@ -111,32 +89,50 @@ def run_in_thread(tox_env, os_env, processes): # wait until someone finishes and retry queuing jobs finished.wait() finished.clear() - for thread in threads: - while thread.is_alive(): - thread.join(0.05) - # join suspends signal handling (ctrl+c), periodically time-out to check for it + + wait_parallel_children_finish(threads) except KeyboardInterrupt: - reporter.verbosity0("keyboard interrupt parallel - stopping children") + reporter.verbosity0( + "[{}] KeyboardInterrupt parallel - stopping children".format(os.getpid()) + ) while True: # do not allow to interrupt until children interrupt try: - # first try to request a graceful shutdown - for name, proc in list(processes.items()): - if proc.returncode is None: - proc.send_signal( - signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT - ) - reporter.verbosity2("send CTRL+C {}-{}".format(name, proc.pid)) - if len(threads): - threads[0].join(1.2) # wait at most 200ms for all to finish - - # now if being gentle did not work now be forceful - for name, proc in list(processes.items()): - if proc.returncode is None: - proc.terminate() - reporter.verbosity2("terminate {}-{}".format(name, proc.pid)) - for thread in threads: - thread.join() + # putting it inside a thread so it's not interrupted + stopper = Thread(target=_stop_child_processes, args=(processes, threads)) + stopper.start() + stopper.join() except KeyboardInterrupt: continue raise KeyboardInterrupt + + +def wait_parallel_children_finish(threads): + signal_done = Event() + + def wait_while_finish(): + """wait on background thread to still allow signal handling""" + for thread_ in threads: + thread_.join() + signal_done.set() + + wait_for_done = Thread(target=wait_while_finish) + wait_for_done.start() + signal_done.wait() + + +def _stop_child_processes(processes, main_threads): + """A three level stop mechanism for children - INT (250ms) -> TERM (100ms) -> KILL""" + # first stop children + def shutdown(tox_env, action, process): + action.handle_interrupt(process) + + threads = [Thread(target=shutdown, args=(n, a, p)) for n, (a, p) in processes.items()] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + # then its threads + for thread in main_threads: + thread.join() diff --git a/src/tox/util/lock.py b/src/tox/util/lock.py index 0cff0137b..00c86b5e7 100644 --- a/src/tox/util/lock.py +++ b/src/tox/util/lock.py @@ -6,32 +6,34 @@ import py from filelock import FileLock, Timeout +from tox.reporter import verbosity1 + @contextmanager -def get(lock_file, report): +def hold_lock(lock_file, reporter=verbosity1): py.path.local(lock_file.dirname).ensure(dir=1) lock = FileLock(str(lock_file)) try: try: lock.acquire(0.0001) except Timeout: - report("lock file {} present, will block until released".format(lock_file)) + reporter("lock file {} present, will block until released".format(lock_file)) lock.acquire() yield finally: lock.release(force=True) -def get_unique_file(path, prefix, suffix, report): +def get_unique_file(path, prefix, suffix): """get a unique file in a folder having a given prefix and suffix, with unique number in between""" lock_file = path.join(".lock") prefix = "{}-".format(prefix) - with get(lock_file, report): + with hold_lock(lock_file): max_value = -1 for candidate in path.listdir("{}*{}".format(prefix, suffix)): try: - max_value = max(max_value, int(candidate.name[len(prefix) : -len(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)) diff --git a/src/tox/util/main.py b/src/tox/util/main.py new file mode 100644 index 000000000..d17a7f802 --- /dev/null +++ b/src/tox/util/main.py @@ -0,0 +1,5 @@ +import inspect + +from tox import __main__ + +MAIN_FILE = inspect.getfile(__main__) diff --git a/src/tox/util/stdlib.py b/src/tox/util/stdlib.py index c9c052773..714c7f591 100644 --- a/src/tox/util/stdlib.py +++ b/src/tox/util/stdlib.py @@ -1,11 +1,11 @@ import sys +import threading -if sys.version_info >= (3, 7): - from contextlib import nullcontext -else: - import contextlib - - @contextlib.contextmanager - def nullcontext(enter_result=None): - yield enter_result +def is_main_thread(): + cur_thread = threading.current_thread() + if sys.version_info >= (3, 4): + return cur_thread is threading.main_thread() + else: + # noinspection PyUnresolvedReferences + return isinstance(cur_thread, threading._MainThread) diff --git a/src/tox/venv.py b/src/tox/venv.py index 0290035a5..c31c9002a 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -518,7 +518,6 @@ def test( break # Don't process remaining commands except KeyboardInterrupt: self.status = "keyboardinterrupt" - reporter.error(self.status) raise def _pcall( diff --git a/tests/integration/test_parallel_interrupt.py b/tests/integration/test_parallel_interrupt.py index af6052a02..5a8ffbfef 100644 --- a/tests/integration/test_parallel_interrupt.py +++ b/tests/integration/test_parallel_interrupt.py @@ -7,11 +7,12 @@ from pathlib2 import Path -from tox import __main__ as main +from tox.util.main import MAIN_FILE -def test_parallel_interrupt(initproj, cmd, monkeypatch): +def test_parallel_interrupt(initproj, monkeypatch, capfd): monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) + monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1")) start = datetime.now() initproj( "pkg123-0.7", @@ -23,7 +24,7 @@ def test_parallel_interrupt(initproj, cmd, monkeypatch): [testenv] skip_install = True commands = python -c "open('{{envname}}', 'w').write('done'); \ - import time; time.sleep(10)" + import time; time.sleep(100)" whitelist_externals = {} """.format( @@ -31,9 +32,14 @@ def test_parallel_interrupt(initproj, cmd, monkeypatch): ) }, ) - cmd = [sys.executable, main.__file__, "-v", "-v", "-p", "all", "-o"] process = subprocess.Popen( - cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True + [sys.executable, MAIN_FILE, "-p", "all", "-o"], + creationflags=( + subprocess.CREATE_NEW_PROCESS_GROUP + if sys.platform == "win32" + else 0 + # needed for Windows signal send ability (CTRL+C) + ), ) try: import psutil @@ -44,18 +50,19 @@ def test_parallel_interrupt(initproj, cmd, monkeypatch): wait_for_env_startup(process) - all_children = [] + all_children = [current_process] if current_process is not None: - all_children = current_process.children(recursive=True) - assert all_children + all_children.extend(current_process.children(recursive=True)) + assert len(all_children) >= 1 + 2 + 2, all_children end = datetime.now() - start assert end process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) - out, _ = process.communicate() - assert "keyboard interrupt parallel - stopping children" in out, out - assert "\nERROR: a: parallel child exit code " in out, out - assert "\nERROR: b: parallel child exit code " in out, out + process.wait() + out, err = capfd.readouterr() + assert "KeyboardInterrupt parallel - stopping children" in out, out + assert "ERROR: a: parallel child exit code " in out, out + assert "ERROR: b: parallel child exit code " in out, out assert all(not children.is_running() for children in all_children) diff --git a/tests/integration/test_provision_int.py b/tests/integration/test_provision_int.py index 6cb3b93c1..93005f95d 100644 --- a/tests/integration/test_provision_int.py +++ b/tests/integration/test_provision_int.py @@ -3,9 +3,10 @@ import sys import time -import pytest from pathlib2 import Path +from tox.util.main import MAIN_FILE + def test_provision_missing(initproj, cmd): initproj( @@ -27,10 +28,8 @@ def test_provision_missing(initproj, cmd): assert meta_python.exists() -@pytest.mark.skipif( - sys.platform == "win32", reason="no easy way to trigger CTRL+C on windows for a process" -) -def test_provision_interrupt_child(initproj, cmd): +def test_provision_interrupt_child(initproj, capfd, monkeypatch): + monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1")) initproj( "pkg123-0.7", filedefs={ @@ -39,18 +38,30 @@ def test_provision_interrupt_child(initproj, cmd): skipsdist=True minversion = 3.7.0 requires = setuptools == 40.6.3 - [testenv] - commands=python -c "file_h = open('a', 'w').write('b'); \ - import time; time.sleep(10)" [testenv:b] - basepython=python + commands=python -c "import time; open('a', 'w').write('content'); \ + time.sleep(10)" + basepython = python """ }, ) - cmd = [sys.executable, "-m", "tox", "-v", "-v", "-e", "python"] + cmd = [sys.executable, MAIN_FILE, "-v", "-v", "-e", "b"] process = subprocess.Popen( - cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True + cmd, + creationflags=( + subprocess.CREATE_NEW_PROCESS_GROUP + if sys.platform == "win32" + else 0 + # needed for Windows signal send ability (CTRL+C) + ), ) + try: + import psutil + + current_process = psutil.Process(process.pid) + except ImportError: + current_process = None + signal_file = Path() / "a" while not signal_file.exists() and process.poll() is None: time.sleep(0.1) @@ -58,6 +69,18 @@ def test_provision_interrupt_child(initproj, cmd): out, err = process.communicate() assert False, out - process.send_signal(signal.SIGINT) - out, _ = process.communicate() - assert "\nERROR: keyboardinterrupt\n" in out, out + all_process = [current_process] + if current_process is not None: + all_process.extend(current_process.children(recursive=False)) + # 1 process for the host tox, 1 for the provisioned + assert len(all_process) >= 2, all_process + + process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + process.wait() + out, err = capfd.readouterr() + assert ".tox KeyboardInterrupt: from" in out, out + + for process in all_process: + assert not process.is_running(), "{}{}".format( + out, "\n".join(repr(i) for i in all_process) + ) diff --git a/tests/unit/session/test_parallel.py b/tests/unit/session/test_parallel.py index 51cc9b524..266e90723 100644 --- a/tests/unit/session/test_parallel.py +++ b/tests/unit/session/test_parallel.py @@ -1,6 +1,9 @@ from __future__ import absolute_import, unicode_literals import os +import sys + +import pytest def test_parallel(cmd, initproj): @@ -74,7 +77,9 @@ def test_parallel_circular(cmd, initproj): assert result.out == "ERROR: circular dependency detected: a | b\n" -def test_parallel_error_report(cmd, initproj): +@pytest.mark.parametrize("live", [True, False]) +def test_parallel_error_report(cmd, initproj, monkeypatch, live): + monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) initproj( "pkg123-0.7", filedefs={ @@ -83,22 +88,24 @@ def test_parallel_error_report(cmd, initproj): isolated_build = true envlist = a [testenv] + skip_install = true commands=python -c "import sys, os; sys.stderr.write(str(12345) + os.linesep);\ raise SystemExit(17)" - """, - "pyproject.toml": """ - [build-system] - requires = ["setuptools >= 35.0.2"] - build-backend = 'setuptools.build_meta' - """, + whitelist_externals = {} + """.format( + sys.executable + ) }, ) - result = cmd("-p", "all") + args = ["-o"] if live else [] + result = cmd("-p", "all", *args) msg = result.out assert result.ret == 1, msg - # we print output + + # for live we print the failure logfile, otherwise just stream through (no logfile present) assert "(exited with code 17)" in result.out, msg - assert "Failed a under process " in result.out, msg + if not live: + assert "ERROR: invocation failed (exit code 1), logfile:" in result.out, msg assert any(line for line in result.outlines if line == "12345") @@ -109,12 +116,16 @@ def test_parallel_error_report(cmd, initproj): assert result.outlines[summary_lines[0] + 1 :] == ["ERROR: a: parallel child exit code 1"] -def test_parallel_deadlock(cmd, initproj): +def test_parallel_deadlock(cmd, initproj, monkeypatch): + monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) tox_ini = """\ [tox] envlist = e1,e2 skipsdist = true +[testenv] +whitelist_externals = {} + [testenv:e1] commands = python -c '[print("hello world") for _ in range(5000)]' @@ -122,7 +133,9 @@ def test_parallel_deadlock(cmd, initproj): [testenv:e2] commands = python -c '[print("hello world") for _ in range(5000)]' -""" +""".format( + sys.executable + ) initproj("pkg123-0.7", filedefs={"tox.ini": tox_ini}) cmd("-p", "2") # used to hang indefinitely diff --git a/tests/unit/session/test_provision.py b/tests/unit/session/test_provision.py index 50e6f0dc9..ae4b4abc7 100644 --- a/tests/unit/session/test_provision.py +++ b/tests/unit/session/test_provision.py @@ -2,7 +2,7 @@ import pytest -from tox.exception import MissingRequirement +from tox.exception import BadRequirement, MissingRequirement @pytest.fixture(scope="session") @@ -80,3 +80,17 @@ def test_provision_basepython_local(newconfig, next_tox_major): config = context.value.config base_python = config.envconfigs[".tox"].basepython assert base_python == "what" + + +def test_provision_bad_requires(newconfig, capsys, monkeypatch): + with pytest.raises(BadRequirement): + newconfig( + [], + """ + [tox] + requires = sad >sds d ok + """, + ) + out, err = capsys.readouterr() + assert "ERROR: failed to parse RequirementParseError" in out + assert not err diff --git a/tox.ini b/tox.ini index f53768ede..0e3b8676e 100644 --- a/tox.ini +++ b/tox.ini @@ -57,7 +57,7 @@ passenv = {[testenv]passenv} # `error setting certificate verify locations` error PROGRAMDATA extras = lint -deps = pre-commit == 1.10.3 +deps = pre-commit >= 1.14.4, < 2 skip_install = True commands = pre-commit run --all-files --show-diff-on-failure python -c 'import pathlib; print("hint: run \{\} install to add checks as pre-commit hook".format(pathlib.Path(r"{envdir}") / "bin" / "pre-commit"))' @@ -82,15 +82,6 @@ commands = coverage erase depends = py27, py34, py35, py36, py37, pypy, pypy3 parallel_show_output = True -[testenv:coveralls] -description = [only run on CI]: upload coverage data to codecov (depends on coverage running first) -passenv = CI_* - COVERALLS_* -setenv = COVERAGE_FILE={toxworkdir}/.coverage -deps = coveralls[yaml]>= 1.5.1, <2 -skip_install = True -commands = coveralls --rcfile=tox.ini -v - [testenv:exit_code] # to see how the InvocationError is displayed, use # PYTHONPATH=.:$PYTHONPATH python3 -m tox -e exit_code @@ -113,6 +104,7 @@ max-line-length = 99 [coverage:run] branch = true +parallel = true [coverage:report] skip_covered = True From f07b41b7391df0ce2ad59cf89fc854b3dd9421e8 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Mon, 25 Mar 2019 18:27:38 +0000 Subject: [PATCH 5/9] more fix --- azure-pipelines.yml | 3 +- docs/changelog/1137.bugfix.rst | 1 + docs/changelog/1139.feature.rst | 3 +- docs/changelog/1150.bugfix.rst | 2 + docs/changelog/1163.doc.rst | 2 +- docs/changelog/1172.bugfix.rst | 5 +- docs/changelog/1177.bugfix.rst | 1 + docs/changelog/1203.feature.rst | 2 + docs/changelog/998.feature.rst | 2 +- src/tox/_pytestplugin.py | 4 +- src/tox/action.py | 50 ++++++++++++-------- src/tox/session/commands/provision.py | 4 +- src/tox/session/commands/run/parallel.py | 25 +++------- src/tox/venv.py | 4 +- tests/integration/test_parallel_interrupt.py | 30 ++++++++---- tests/integration/test_provision_int.py | 9 +++- tests/unit/session/test_parallel.py | 3 +- tests/unit/session/test_session.py | 5 +- tests/unit/test_venv.py | 20 ++++---- tests/unit/test_z_cmdline.py | 6 +-- tox.ini | 11 ++--- 21 files changed, 108 insertions(+), 84 deletions(-) create mode 100644 docs/changelog/1137.bugfix.rst create mode 100644 docs/changelog/1150.bugfix.rst create mode 100644 docs/changelog/1177.bugfix.rst create mode 100644 docs/changelog/1203.feature.rst diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 78b800020..7f7b8dc12 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -5,10 +5,9 @@ variables: CI_NAME: Azure Pipelines CI_BUILD_ID: $(Build.BuildId) CI_BUILD_URL: "https://toxdev.visualstudio.com/tox/_build/results?buildId=$(Build.BuildId)" - PYTEST_ADDOPTS: "-s -v -v -ra --showlocals" + PYTEST_ADDOPTS: "-v -v -ra --showlocals" GIT_BRANCH: $[ coalesce(variables['System.PullRequest.SourceBranch'], variables['Build.SourceBranchName'], 'not-found') ] GIT_COMMIT_SHA: $[ coalesce(variables['System.PullRequest.SourceCommitId'], variables['Build.SourceVersion'], 'not-found') ] - PYTEST_XDIST_PROC_NR: 0 trigger: batch: true diff --git a/docs/changelog/1137.bugfix.rst b/docs/changelog/1137.bugfix.rst new file mode 100644 index 000000000..f750ff4a5 --- /dev/null +++ b/docs/changelog/1137.bugfix.rst @@ -0,0 +1 @@ +Fixed bug of children process calls logs clashing (log already exists) - by :user:`gaborbernat` diff --git a/docs/changelog/1139.feature.rst b/docs/changelog/1139.feature.rst index b8385004c..f8f86b1b4 100644 --- a/docs/changelog/1139.feature.rst +++ b/docs/changelog/1139.feature.rst @@ -1 +1,2 @@ -tox will inject the ``TOX_PARALLEL_ENV`` environment variable, set to the current running tox environment name, only when running in parallel mode. +tox will inject the ``TOX_PARALLEL_ENV`` environment variable, set to the current running tox environment name, +only when running in parallel mode. - by :user:`gaborbernat` diff --git a/docs/changelog/1150.bugfix.rst b/docs/changelog/1150.bugfix.rst new file mode 100644 index 000000000..0d752e674 --- /dev/null +++ b/docs/changelog/1150.bugfix.rst @@ -0,0 +1,2 @@ +Interpreter discovery and virtualenv creation process calls that failed will now print out on the screen their output +(via the logfile we automatically save) - by :user:`gaborbernat` diff --git a/docs/changelog/1163.doc.rst b/docs/changelog/1163.doc.rst index 866b9c3d4..b6bdb8ab7 100644 --- a/docs/changelog/1163.doc.rst +++ b/docs/changelog/1163.doc.rst @@ -1 +1 @@ -Add a ``poetry`` examples to packaging. +Add a ``poetry`` examples to packaging - by :user:`gaborbernat` diff --git a/docs/changelog/1172.bugfix.rst b/docs/changelog/1172.bugfix.rst index 9188dc459..a49773a06 100644 --- a/docs/changelog/1172.bugfix.rst +++ b/docs/changelog/1172.bugfix.rst @@ -1,2 +1,3 @@ -Interrupting parallel runs via keyboard interrupt no longer leaves the child runs running -(now forwards the keyboard interrupt to the children and waits until they finish) - by :user:`gaborbernat`. +Interrupting a tox call (e.g. via CTRL+C) now will ensure that spawn child processes (test calls, interpreter discovery, +parallel sub-instances, provisioned hosts) are correctly stopped before exiting (via the pattern of INTERRUPT - 300 ms, +TERMINATE - 200 ms, KILL signals) - by :user:`gaborbernat` diff --git a/docs/changelog/1177.bugfix.rst b/docs/changelog/1177.bugfix.rst new file mode 100644 index 000000000..afb5591a9 --- /dev/null +++ b/docs/changelog/1177.bugfix.rst @@ -0,0 +1 @@ +Parallel children now save their output to a disk logfile - by :user:`gaborbernat` diff --git a/docs/changelog/1203.feature.rst b/docs/changelog/1203.feature.rst new file mode 100644 index 000000000..617c2aede --- /dev/null +++ b/docs/changelog/1203.feature.rst @@ -0,0 +1,2 @@ +Setting the environment variable ``TOX_REPORTER_TIMESTAMP`` to ``1`` will enable showing for each output line its delta +since the tox startup. This can be especially handy when debugging parallel runs.- by :user:`gaborbernat` diff --git a/docs/changelog/998.feature.rst b/docs/changelog/998.feature.rst index 089ac8d77..a8422134d 100644 --- a/docs/changelog/998.feature.rst +++ b/docs/changelog/998.feature.rst @@ -1,2 +1,2 @@ tox now auto-provisions itself if needed (see :ref:`auto-provision`). Plugins or minimum version of tox no longer -need to be manually satisfied by the user, increasing their ease of use. +need to be manually satisfied by the user, increasing their ease of use. - by :user:`gaborbernat` diff --git a/src/tox/_pytestplugin.py b/src/tox/_pytestplugin.py index 828ee5126..b8d77d0ec 100644 --- a/src/tox/_pytestplugin.py +++ b/src/tox/_pytestplugin.py @@ -136,7 +136,9 @@ def _read(self, out, pos): @property def outlines(self): - return self.out.splitlines() + out = [] if self.out is None else self.out.splitlines() + err = [] if self.err is None else self.err.splitlines() + return err + out def __repr__(self): return "RunResult(ret={}, args={}, out=\n{}\n, err=\n{})".format( diff --git a/src/tox/action.py b/src/tox/action.py index 1df5c144f..398634fed 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -106,7 +106,6 @@ def popen( output = self.evaluate_cmd(fin, process, redirect) exit_code = process.returncode if exit_code and not ignore_ret: - invoked = " ".join(map(str, args)) if out_path: reporter.error( "invocation failed (exit code {:d}), logfile: {}".format(exit_code, out_path) @@ -114,9 +113,9 @@ def popen( output = out_path.read() reporter.error(output) self.command_log.add_command(args, output, exit_code) - raise InvocationError(invoked, exit_code, out_path) + raise InvocationError(cmd_args_shell, exit_code, out_path) else: - raise InvocationError(invoked, exit_code) + raise InvocationError(cmd_args_shell, exit_code) if not output and out_path: output = out_path.read() self.command_log.add_command(args, output, exit_code) @@ -130,7 +129,6 @@ def evaluate_cmd(self, input_file_handler, process, redirect): raise ValueError("stderr must not be piped here") # we read binary from the process and must write using a binary stream buf = getattr(sys.stdout, "buffer", sys.stdout) - out = None last_time = time.time() while True: # we have to read one byte at a time, otherwise there @@ -153,9 +151,7 @@ def evaluate_cmd(self, input_file_handler, process, redirect): # the seek updates internal read buffers input_file_handler.seek(0, 1) input_file_handler.close() - process.wait() # wait to finish - else: - out, err = process.communicate() # wait to finish + out, _ = process.communicate() # wait to finish except KeyboardInterrupt as exception: main_thread = is_main_thread() while True: @@ -176,19 +172,32 @@ def evaluate_cmd(self, input_file_handler, process, redirect): def handle_interrupt(self, process): """A three level stop mechanism for children - INT -> TERM -> KILL""" msg = "from {} {{}} pid {}".format(os.getpid(), process.pid) - self.info("KeyboardInterrupt", msg.format("SIGINT")) - process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) - try: - process.wait(WAIT_INTERRUPT) - except subprocess.TimeoutExpired: - self.info("KeyboardInterrupt", msg.format("SIGTERM")) - process.terminate() + if process.poll() is None: + self.info("KeyboardInterrupt", msg.format("SIGINT")) + process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) + if self._wait(process, WAIT_INTERRUPT) is None: + self.info("KeyboardInterrupt", msg.format("SIGTERM")) + process.terminate() + if self._wait(process, WAIT_TERMINATE) is None: + self.info("KeyboardInterrupt", msg.format("SIGKILL")) + process.kill() + process.communicate() + + @staticmethod + def _wait(process, timeout): + if sys.version_info >= (3, 3): + # python 3 has timeout feature built-in try: - process.wait(WAIT_TERMINATE) + process.communicate(timeout=WAIT_INTERRUPT) except subprocess.TimeoutExpired: - self.info("KeyboardInterrupt", msg.format("SIGKILL")) - process.kill() - process.wait() + pass + else: + # on Python 2 we need to simulate it + delay = 0.01 + while process.poll() is None and timeout > 0: + time.sleep(delay) + timeout -= delay + return process.poll() @contextmanager def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout): @@ -220,9 +229,10 @@ def get_log_path(self, actionid): def _rewrite_args(self, cwd, args): new_args = [] + cwd = py.path.local(cwd) for arg in args: - if not INFO.IS_WIN and isinstance(arg, py.path.local): - cwd = py.path.local(cwd) + arg_path = py.path.local(arg) + if arg_path.exists(): arg = cwd.bestrelpath(arg) new_args.append(str(arg)) # subprocess does not always take kindly to .py scripts so adding the interpreter here diff --git a/src/tox/session/commands/provision.py b/src/tox/session/commands/provision.py index a694eb72a..7237a5355 100644 --- a/src/tox/session/commands/provision.py +++ b/src/tox/session/commands/provision.py @@ -1,11 +1,11 @@ """In case the tox environment is not correctly setup provision it and delegate execution""" -from tox.util.main import MAIN_FILE +from __future__ import absolute_import, unicode_literals def provision_tox(provision_venv, args): ensure_meta_env_up_to_date(provision_venv) with provision_venv.new_action("provision") as action: - provision_args = [str(provision_venv.envconfig.envpython), MAIN_FILE] + args + provision_args = [str(provision_venv.envconfig.envpython), "-m", "tox"] + args action.popen(provision_args, redirect=False) diff --git a/src/tox/session/commands/run/parallel.py b/src/tox/session/commands/run/parallel.py index 2ace99171..9322a4960 100644 --- a/src/tox/session/commands/run/parallel.py +++ b/src/tox/session/commands/run/parallel.py @@ -1,6 +1,6 @@ import os import sys -from collections import OrderedDict +from collections import OrderedDict, deque from threading import Event, Semaphore, Thread from tox import reporter @@ -64,7 +64,7 @@ def collect_process(process): outcome = spinner.fail outcome(env_name) - threads = [] + threads = deque() processes = {} todo_keys = set(venv_dict.keys()) todo = OrderedDict((n, todo_keys & set(v.envconfig.depends)) for n, v in venv_dict.items()) @@ -89,8 +89,10 @@ def collect_process(process): # wait until someone finishes and retry queuing jobs finished.wait() finished.clear() - - wait_parallel_children_finish(threads) + while threads: + threads = [ + thread for thread in threads if not thread.join(0.1) and thread.is_alive() + ] except KeyboardInterrupt: reporter.verbosity0( "[{}] KeyboardInterrupt parallel - stopping children".format(os.getpid()) @@ -107,22 +109,9 @@ def collect_process(process): raise KeyboardInterrupt -def wait_parallel_children_finish(threads): - signal_done = Event() - - def wait_while_finish(): - """wait on background thread to still allow signal handling""" - for thread_ in threads: - thread_.join() - signal_done.set() - - wait_for_done = Thread(target=wait_while_finish) - wait_for_done.start() - signal_done.wait() - - def _stop_child_processes(processes, main_threads): """A three level stop mechanism for children - INT (250ms) -> TERM (100ms) -> KILL""" + # first stop children def shutdown(tox_env, action, process): action.handle_interrupt(process) diff --git a/src/tox/venv.py b/src/tox/venv.py index c31c9002a..cc39fb9fe 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -169,7 +169,9 @@ def getcommandpath(self, name, venv=True, cwd=None): path = self._normal_lookup(name) if path is None: - raise tox.exception.InvocationError("could not find executable {!r}".format(name)) + raise tox.exception.InvocationError( + "could not find executable {}".format(pipes.quote(name)) + ) return str(path) # will not be rewritten for reporting diff --git a/tests/integration/test_parallel_interrupt.py b/tests/integration/test_parallel_interrupt.py index 5a8ffbfef..496579a33 100644 --- a/tests/integration/test_parallel_interrupt.py +++ b/tests/integration/test_parallel_interrupt.py @@ -5,11 +5,15 @@ import sys from datetime import datetime +import pytest from pathlib2 import Path from tox.util.main import MAIN_FILE +@pytest.mark.skipif( + "sys.platform == 'win32'", reason="triggering SIGINT reliably on Windows is hard" +) def test_parallel_interrupt(initproj, monkeypatch, capfd): monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1")) @@ -33,7 +37,7 @@ def test_parallel_interrupt(initproj, monkeypatch, capfd): }, ) process = subprocess.Popen( - [sys.executable, MAIN_FILE, "-p", "all", "-o"], + [sys.executable, MAIN_FILE, "-p", "all"], creationflags=( subprocess.CREATE_NEW_PROCESS_GROUP if sys.platform == "win32" @@ -50,20 +54,23 @@ def test_parallel_interrupt(initproj, monkeypatch, capfd): wait_for_env_startup(process) - all_children = [current_process] + all_children = [] if current_process is not None: + all_children.append(current_process) all_children.extend(current_process.children(recursive=True)) assert len(all_children) >= 1 + 2 + 2, all_children - end = datetime.now() - start assert end process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) process.wait() out, err = capfd.readouterr() - assert "KeyboardInterrupt parallel - stopping children" in out, out - assert "ERROR: a: parallel child exit code " in out, out - assert "ERROR: b: parallel child exit code " in out, out - assert all(not children.is_running() for children in all_children) + output = "{}\n{}".format(out, err) + assert "KeyboardInterrupt parallel - stopping children" in output, output + assert "ERROR: a: parallel child exit code " in output, output + assert "ERROR: b: parallel child exit code " in output, output + for process in all_children: + msg = "{}{}".format(output, "\n".join(repr(i) for i in all_children)) + assert not process.is_running(), msg def wait_for_env_startup(process): @@ -71,11 +78,14 @@ def wait_for_env_startup(process): signal_files = [Path() / "a", Path() / "b"] found = False while True: - if all(signal_file.exists() for signal_file in signal_files): - found = True - break if process.poll() is not None: break + for signal_file in signal_files: + if not signal_file.exists(): + break + else: + found = True + break if not found or process.poll() is not None: missing = [f for f in signal_files if not f.exists()] out, _ = process.communicate() diff --git a/tests/integration/test_provision_int.py b/tests/integration/test_provision_int.py index 93005f95d..e9f531445 100644 --- a/tests/integration/test_provision_int.py +++ b/tests/integration/test_provision_int.py @@ -3,6 +3,7 @@ import sys import time +import pytest from pathlib2 import Path from tox.util.main import MAIN_FILE @@ -28,7 +29,10 @@ def test_provision_missing(initproj, cmd): assert meta_python.exists() -def test_provision_interrupt_child(initproj, capfd, monkeypatch): +@pytest.mark.skipif( + "sys.platform == 'win32'", reason="triggering SIGINT reliably on Windows is hard" +) +def test_provision_interrupt_child(initproj, monkeypatch, capfd): monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1")) initproj( "pkg123-0.7", @@ -69,8 +73,9 @@ def test_provision_interrupt_child(initproj, capfd, monkeypatch): out, err = process.communicate() assert False, out - all_process = [current_process] + all_process = [] if current_process is not None: + all_process.append(current_process) all_process.extend(current_process.children(recursive=False)) # 1 process for the host tox, 1 for the provisioned assert len(all_process) >= 2, all_process diff --git a/tests/unit/session/test_parallel.py b/tests/unit/session/test_parallel.py index 266e90723..16666ea18 100644 --- a/tests/unit/session/test_parallel.py +++ b/tests/unit/session/test_parallel.py @@ -106,8 +106,7 @@ def test_parallel_error_report(cmd, initproj, monkeypatch, live): assert "(exited with code 17)" in result.out, msg if not live: assert "ERROR: invocation failed (exit code 1), logfile:" in result.out, msg - - assert any(line for line in result.outlines if line == "12345") + assert any(line for line in result.outlines if line == "12345"), result.out # single summary at end summary_lines = [j for j, l in enumerate(result.outlines) if " summary " in l] diff --git a/tests/unit/session/test_session.py b/tests/unit/session/test_session.py index 745de8de7..fe5490778 100644 --- a/tests/unit/session/test_session.py +++ b/tests/unit/session/test_session.py @@ -1,4 +1,5 @@ import os +import pipes import sys import textwrap from threading import Thread @@ -343,13 +344,13 @@ def test_command_prev_fail_command_skip_post_run(cmd, initproj, mock_venv): expected = textwrap.dedent( """ py run-test-pre: commands[0] | python -c 'raise SystemExit(2)' - ERROR: InvocationError for command {} -c raise SystemExit(2) (exited with code 2) + ERROR: InvocationError for command {} -c 'raise SystemExit(2)' (exited with code 2) py run-test-post: commands[0] | python -c 'print("post")' post ___________________________________ summary ___________________________________{} ERROR: py: commands failed """.format( - sys.executable, "_" if sys.platform != "win32" else "" + pipes.quote(sys.executable), "_" if sys.platform != "win32" else "" ) ) have = result.out.replace(os.linesep, "\n") diff --git a/tests/unit/test_venv.py b/tests/unit/test_venv.py index 486ede015..0eec6c415 100644 --- a/tests/unit/test_venv.py +++ b/tests/unit/test_venv.py @@ -913,7 +913,7 @@ def tox_runtest_post(self): assert log == ["started", "finished"] -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_empty_instance(tmpdir): testfile = tmpdir.join("check_shebang_empty_instance.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -924,7 +924,7 @@ def test_tox_testenv_interpret_shebang_empty_instance(tmpdir): assert args == base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_empty_interpreter(tmpdir): testfile = tmpdir.join("check_shebang_empty_interpreter.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -935,7 +935,7 @@ def test_tox_testenv_interpret_shebang_empty_interpreter(tmpdir): assert args == base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_empty_interpreter_ws(tmpdir): testfile = tmpdir.join("check_shebang_empty_interpreter_ws.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -946,7 +946,7 @@ def test_tox_testenv_interpret_shebang_empty_interpreter_ws(tmpdir): assert args == base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_non_utf8(tmpdir): testfile = tmpdir.join("check_non_utf8.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -956,7 +956,7 @@ def test_tox_testenv_interpret_shebang_non_utf8(tmpdir): assert args == base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_interpreter_simple(tmpdir): testfile = tmpdir.join("check_shebang_interpreter_simple.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -967,7 +967,7 @@ def test_tox_testenv_interpret_shebang_interpreter_simple(tmpdir): assert args == ["interpreter"] + base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_interpreter_ws(tmpdir): testfile = tmpdir.join("check_shebang_interpreter_ws.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -978,7 +978,7 @@ def test_tox_testenv_interpret_shebang_interpreter_ws(tmpdir): assert args == ["interpreter"] + base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_interpreter_arg(tmpdir): testfile = tmpdir.join("check_shebang_interpreter_arg.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -989,7 +989,7 @@ def test_tox_testenv_interpret_shebang_interpreter_arg(tmpdir): assert args == ["interpreter", "argx"] + base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_interpreter_args(tmpdir): testfile = tmpdir.join("check_shebang_interpreter_args.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -1000,7 +1000,7 @@ def test_tox_testenv_interpret_shebang_interpreter_args(tmpdir): assert args == ["interpreter", "argx argx-part2"] + base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_real(tmpdir): testfile = tmpdir.join("check_shebang_real.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] @@ -1011,7 +1011,7 @@ def test_tox_testenv_interpret_shebang_real(tmpdir): assert args == ["/usr/bin/env", "python"] + base_args -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no shebang on Windows") def test_tox_testenv_interpret_shebang_long_example(tmpdir): testfile = tmpdir.join("check_shebang_long_example.py") base_args = [str(testfile), "arg1", "arg2", "arg3"] diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py index a452c6c58..5b3b42062 100644 --- a/tests/unit/test_z_cmdline.py +++ b/tests/unit/test_z_cmdline.py @@ -672,7 +672,7 @@ def test_alwayscopy_default(initproj, cmd): assert "virtualenv --always-copy" not in result.out -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no echo on Windows") def test_empty_activity_ignored(initproj, cmd): initproj( "example123", @@ -689,7 +689,7 @@ def test_empty_activity_ignored(initproj, cmd): assert "installed:" not in result.out -@pytest.mark.skipif("sys.platform == 'win32'") +@pytest.mark.skipif("sys.platform == 'win32'", reason="no echo on Windows") def test_empty_activity_shown_verbose(initproj, cmd): initproj( "example123", @@ -930,7 +930,7 @@ def test_exit_code(initproj, cmd, exit_code, mocker): assert call_error_name == "InvocationError" # quotes are removed in result.out # do not include "python" as it is changed to python.EXE by appveyor - expected_command_arg = " -c import sys; sys.exit({:d})".format(exit_code) + expected_command_arg = " -c 'import sys; sys.exit({:d})'".format(exit_code) assert expected_command_arg in call_command assert call_exit_code == exit_code else: diff --git a/tox.ini b/tox.ini index 0e3b8676e..414ff0c5a 100644 --- a/tox.ini +++ b/tox.ini @@ -21,13 +21,13 @@ setenv = PIP_DISABLE_VERSION_CHECK = 1 passenv = http_proxy https_proxy no_proxy SSL_CERT_FILE PYTEST_ADDOPTS deps = extras = testing -commands = pytest {posargs:\ +commands = pytest \ --cov="{envsitepackagesdir}/tox" \ - --cov-config="{toxinidir}/tox.ini" \ + --cov-config="{toxinidir}/tox.ini" \ --timeout=180 \ - -n={env:PYTEST_XDIST_PROC_NR:auto} \ --junitxml={env:JUNIT_XML_FILE:{toxworkdir}/.test.{envname}.xml} \ - . } + -n={env:PYTEST_XDIST_PROC_NR:auto} \ + {posargs:.} [testenv:docs] description = invoke sphinx-build to build the HTML docs @@ -73,8 +73,7 @@ skip_install = True passenv = {[testenv]passenv} DIFF_AGAINST setenv = COVERAGE_FILE={toxworkdir}/.coverage -commands = coverage erase - coverage combine +commands = coverage combine coverage report -m coverage xml -o {toxworkdir}/coverage.xml coverage html -d {toxworkdir}/htmlcov From c69d66fc0849a0d7527f75a6706c3c1c8bbe23dc Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 26 Mar 2019 16:24:30 +0000 Subject: [PATCH 6/9] some more fixes --- src/tox/action.py | 21 ++++--- src/tox/config/__init__.py | 55 +++++++++++++------ src/tox/reporter.py | 4 +- src/tox/session/__init__.py | 12 ++-- src/tox/util/main.py | 5 +- src/tox/util/spinner.py | 21 ++++++- src/tox/venv.py | 8 ++- tests/integration/test_provision_int.py | 4 +- .../builder/test_package_builder_isolated.py | 2 +- tests/unit/test_venv.py | 2 +- tests/unit/test_z_cmdline.py | 6 +- tox.ini | 8 +-- 12 files changed, 105 insertions(+), 43 deletions(-) diff --git a/src/tox/action.py b/src/tox/action.py index 398634fed..07a4e265e 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -68,14 +68,14 @@ def popen( callback=None, ): """this drives an interaction with a subprocess""" + args = self._rewrite_args(cwd, args) cmd_args = [str(x) for x in args] cmd_args_shell = " ".join(pipes.quote(i) for i in cmd_args) stream_getter = self._get_standard_streams( capture_err, cmd_args_shell, redirect, returnout ) - cwd = os.getcwd() if cwd is None else cwd + cwd = py.path.local(os.getcwd()) if cwd is None else cwd with stream_getter as (fin, out_path, stderr, stdout): - args = self._rewrite_args(cwd, args) try: process = self.via_popen( args, @@ -110,13 +110,13 @@ def popen( reporter.error( "invocation failed (exit code {:d}), logfile: {}".format(exit_code, out_path) ) - output = out_path.read() + output = out_path.read() if out_path.exists() else None reporter.error(output) self.command_log.add_command(args, output, exit_code) raise InvocationError(cmd_args_shell, exit_code, out_path) else: raise InvocationError(cmd_args_shell, exit_code) - if not output and out_path: + if not output and out_path is not None and out_path.exists(): output = out_path.read() self.command_log.add_command(args, output, exit_code) return output @@ -153,6 +153,7 @@ def evaluate_cmd(self, input_file_handler, process, redirect): input_file_handler.close() out, _ = process.communicate() # wait to finish except KeyboardInterrupt as exception: + reporter.error("got KeyboardInterrupt signal") main_thread = is_main_thread() while True: try: @@ -208,7 +209,7 @@ def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout out_path = self.get_log_path(self.name) with out_path.open("wt") as stdout, out_path.open("rb") as input_file_handler: stdout.write( - "actionid: {}\nmsg: {}\ncmdargs: {!r}\n\n".format( + "action: {}\nmsg: {}\ncmd: {!r}\n\n".format( self.name, self.msg, cmd_args_shell ) ) @@ -231,9 +232,13 @@ def _rewrite_args(self, cwd, args): new_args = [] cwd = py.path.local(cwd) for arg in args: - arg_path = py.path.local(arg) - if arg_path.exists(): - arg = cwd.bestrelpath(arg) + if arg and os.path.isabs(str(arg)): + arg_path = py.path.local(arg) + if arg_path.exists() and arg_path.common(cwd) is not None: + potential_arg = cwd.bestrelpath(arg_path) + if len(potential_arg.split("..")) < 2: + # just one parent directory accepted + arg = potential_arg new_args.append(str(arg)) # subprocess does not always take kindly to .py scripts so adding the interpreter here if INFO.IS_WIN: diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index 5f82e78cf..d894bebae 100644 --- a/src/tox/config/__init__.py +++ b/src/tox/config/__init__.py @@ -960,6 +960,15 @@ def __init__(self, config, ini_path, ini_data): # noqa config.toxinidir = config.toxinipath.dirpath() self._cfg = py.iniconfig.IniConfig(config.toxinipath, ini_data) + previous_line_of = self._cfg.lineof + + def line_of_default_to_zero(section, name=None): + at = previous_line_of(section, name=name) + if at is None: + at = 0 + return at + + self._cfg.lineof = line_of_default_to_zero config._cfg = self._cfg self.config = config @@ -1076,35 +1085,45 @@ def __init__(self, config, ini_path, ini_data): # noqa def handle_provision(self, config, reader): requires_list = reader.getlist("requires") config.minversion = reader.getstring("minversion", None) - requires_list.append("tox >= {}".format(config.minversion or tox.__version__)) config.provision_tox_env = name = reader.getstring("provision_tox_env", ".tox") - env_config = self.make_envconfig( - name, "{}{}".format(testenvprefix, name), reader._subs, config - ) - env_config.deps = [DepConfig(r, None) for r in requires_list] - self.ensure_requires_satisfied(config, env_config) + min_version = "tox >= {}".format(config.minversion or tox.__version__) + deps = self.ensure_requires_satisfied(config, requires_list, min_version) + if config.run_provision: + section_name = "testenv:{}".format(name) + if section_name not in self._cfg.sections: + self._cfg.sections[section_name] = {} + self._cfg.sections[section_name]["description"] = "meta tox" + env_config = self.make_envconfig( + name, "{}{}".format(testenvprefix, name), reader._subs, config + ) + env_config.deps = deps + config.envconfigs[config.provision_tox_env] = env_config + raise tox.exception.MissingRequirement(config) @staticmethod - def ensure_requires_satisfied(config, env_config): + def ensure_requires_satisfied(config, requires, min_version): missing_requirements = [] - deps = env_config.deps failed_to_parse = False - for require in deps: + deps = [] + exists = set() + for require in requires + [min_version]: # noinspection PyBroadException try: - pkg_resources.get_distribution(require.name) + package = pkg_resources.Requirement.parse(require) + if package.project_name not in exists: + deps.append(DepConfig(require, None)) + exists.add(package.project_name) + pkg_resources.get_distribution(package) except pkg_resources.RequirementParseError as exception: failed_to_parse = True error("failed to parse {!r}".format(exception)) except Exception as exception: verbosity1("could not satisfy requires {!r}".format(exception)) - missing_requirements.append(str(pkg_resources.Requirement(require.name))) + missing_requirements.append(str(pkg_resources.Requirement(require))) if failed_to_parse: raise tox.exception.BadRequirement() - config.run_provision = bool(missing_requirements) - if missing_requirements: - config.envconfigs[config.provision_tox_env] = env_config - raise tox.exception.MissingRequirement(config) + config.run_provision = bool(len(missing_requirements)) + return deps def parse_build_isolation(self, config, reader): config.isolated_build = reader.getbool("isolated_build", False) @@ -1112,6 +1131,10 @@ def parse_build_isolation(self, config, reader): if config.isolated_build is True: name = config.isolated_build_env if name not in config.envconfigs: + section_name = "testenv:{}".format(name) + if name not in self._cfg.sections: + self._cfg.sections[section_name] = {} + self._cfg.sections[section_name]["description"] = "isolated packaging environment" config.envconfigs[name] = self.make_envconfig( name, "{}{}".format(testenvprefix, name), reader._subs, config ) @@ -1161,7 +1184,7 @@ def make_envconfig(self, name, section, subs, config, replace=True): tc.missing_subs.append(e.name) res = e.FLAG setattr(tc, env_attr.name, res) - if atype in ("path", "string"): + if atype in ("path", "string", "basepython"): reader.addsubstitutions(**{env_attr.name: res}) return tc diff --git a/src/tox/reporter.py b/src/tox/reporter.py index 859ff085e..47d62adfd 100644 --- a/src/tox/reporter.py +++ b/src/tox/reporter.py @@ -43,8 +43,10 @@ def verbosity(self): def log_popen(self, cwd, outpath, cmd_args_shell, pid): """ log information about the action.popen() created process. """ - msg = "[{}] {}$ {} ".format(pid, cwd, cmd_args_shell) + msg = "[{}] {}$ {}".format(pid, cwd, cmd_args_shell) if outpath: + if outpath.common(cwd) is not None: + outpath = cwd.bestrelpath(outpath) msg = "{} >{}".format(msg, outpath) self.verbosity1(msg, of="logpopen") diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py index ef06b525e..2f658b4ed 100644 --- a/src/tox/session/__init__.py +++ b/src/tox/session/__init__.py @@ -207,11 +207,13 @@ def subcommand_test(self): return within_parallel = PARALLEL_ENV_VAR_KEY in os.environ - if not within_parallel and self.config.option.parallel != PARALLEL_OFF: - run_parallel(self.config, self.venv_dict) - else: - run_sequential(self.config, self.venv_dict) - retcode = self._summary() + try: + if not within_parallel and self.config.option.parallel != PARALLEL_OFF: + run_parallel(self.config, self.venv_dict) + else: + run_sequential(self.config, self.venv_dict) + finally: + retcode = self._summary() return retcode def _summary(self): diff --git a/src/tox/util/main.py b/src/tox/util/main.py index d17a7f802..ebd0faa31 100644 --- a/src/tox/util/main.py +++ b/src/tox/util/main.py @@ -1,5 +1,6 @@ import inspect +import os -from tox import __main__ +import tox -MAIN_FILE = inspect.getfile(__main__) +MAIN_FILE = os.path.join(os.path.dirname(inspect.getfile(tox)), "__main__.py") diff --git a/src/tox/util/spinner.py b/src/tox/util/spinner.py index 870afcb0c..25f632bda 100644 --- a/src/tox/util/spinner.py +++ b/src/tox/util/spinner.py @@ -19,15 +19,34 @@ class _CursorInfo(ctypes.Structure): _fields_ = [("size", ctypes.c_int), ("visible", ctypes.c_byte)] +def _file_support_encoding(chars, file): + encoding = getattr(file, "encoding", None) + if encoding is not None: + for char in chars: + try: + char.encode(encoding) + except UnicodeDecodeError: + break + else: + return True + return False + + class Spinner(object): CLEAR_LINE = "\033[K" max_width = 120 - frames = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] + UNICODE_FRAMES = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] + ASCII_FRAMES = ["|", "-", "+", "x", "*"] def __init__(self, enabled=True, refresh_rate=0.1): self.refresh_rate = refresh_rate self.enabled = enabled self._file = sys.stdout + self.frames = ( + self.UNICODE_FRAMES + if _file_support_encoding(self.UNICODE_FRAMES, sys.stdout) + else self.ASCII_FRAMES + ) self.stream = py.io.TerminalWriter(file=self._file) self._envs = OrderedDict() self._frame_index = 0 diff --git a/src/tox/venv.py b/src/tox/venv.py index cc39fb9fe..80a9f1912 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -658,7 +658,13 @@ def tox_testenv_create(venv, action): basepath = venv.path.dirpath() basepath.ensure(dir=1) args.append(venv.path.basename) - venv._pcall(args, venv=False, action=action, cwd=basepath) + venv._pcall( + args, + venv=False, + action=action, + cwd=basepath, + redirect=reporter.verbosity() < reporter.Verbosity.DEBUG, + ) return True # Return non-None to indicate plugin has completed diff --git a/tests/integration/test_provision_int.py b/tests/integration/test_provision_int.py index e9f531445..7c03920db 100644 --- a/tests/integration/test_provision_int.py +++ b/tests/integration/test_provision_int.py @@ -33,6 +33,7 @@ def test_provision_missing(initproj, cmd): "sys.platform == 'win32'", reason="triggering SIGINT reliably on Windows is hard" ) def test_provision_interrupt_child(initproj, monkeypatch, capfd): + monkeypatch.delenv(str("PYTHONPATH"), raising=False) monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1")) initproj( "pkg123-0.7", @@ -42,6 +43,7 @@ def test_provision_interrupt_child(initproj, monkeypatch, capfd): skipsdist=True minversion = 3.7.0 requires = setuptools == 40.6.3 + tox == 3.7.0 [testenv:b] commands=python -c "import time; open('a', 'w').write('content'); \ time.sleep(10)" @@ -81,7 +83,7 @@ def test_provision_interrupt_child(initproj, monkeypatch, capfd): assert len(all_process) >= 2, all_process process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) - process.wait() + process.communicate() out, err = capfd.readouterr() assert ".tox KeyboardInterrupt: from" in out, out diff --git a/tests/unit/package/builder/test_package_builder_isolated.py b/tests/unit/package/builder/test_package_builder_isolated.py index 4e062c178..471dc52c5 100644 --- a/tests/unit/package/builder/test_package_builder_isolated.py +++ b/tests/unit/package/builder/test_package_builder_isolated.py @@ -22,7 +22,7 @@ def test_verbose_isolated_build(initproj, mock_venv, cmd): """, }, ) - result = cmd("--sdistonly", "-vvv") + result = cmd("--sdistonly", "-v", "-v", "-v", "-e", "py") assert "running sdist" in result.out, result.out assert "running egg_info" in result.out, result.out assert "Writing example123-0.5{}setup.cfg".format(os.sep) in result.out, result.out diff --git a/tests/unit/test_venv.py b/tests/unit/test_venv.py index 0eec6c415..5a4eaf511 100644 --- a/tests/unit/test_venv.py +++ b/tests/unit/test_venv.py @@ -774,7 +774,7 @@ def test_installpkg_upgrade(newmocksession, tmpdir): installpkg(venv, pkg) pcalls = mocksession._pcalls assert len(pcalls) == 1 - index = pcalls[0].args.index(str(pkg)) + index = pcalls[0].args.index(pkg.basename) assert index >= 0 assert "-U" in pcalls[0].args[:index] assert "--no-deps" in pcalls[0].args[:index] diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py index 5b3b42062..8bf5abb53 100644 --- a/tests/unit/test_z_cmdline.py +++ b/tests/unit/test_z_cmdline.py @@ -124,9 +124,11 @@ def test_run_custom_install_command_error(cmd, initproj): }, ) result = cmd() - assert re.match( - r"ERROR: invocation failed \(errno \d+\), args: .*[/\\]tox\.ini", result.outlines[-1] + msg = ( + "ERROR: invocation failed (errno 2), args: tox.ini --exists-action w" + " .tox/.tmp/package/1/interp123-0.5.zip" ) + assert msg in result.out, result.out assert result.ret, "{}\n{}".format(result.err, result.out) diff --git a/tox.ini b/tox.ini index 414ff0c5a..403466b73 100644 --- a/tox.ini +++ b/tox.ini @@ -22,10 +22,10 @@ passenv = http_proxy https_proxy no_proxy SSL_CERT_FILE PYTEST_ADDOPTS deps = extras = testing commands = pytest \ - --cov="{envsitepackagesdir}/tox" \ - --cov-config="{toxinidir}/tox.ini" \ - --timeout=180 \ - --junitxml={env:JUNIT_XML_FILE:{toxworkdir}/.test.{envname}.xml} \ + --cov "{envsitepackagesdir}/tox" \ + --cov-config "{toxinidir}/tox.ini" \ + --timeout 180 \ + --junitxml {env:JUNIT_XML_FILE:{toxworkdir}/.test.{envname}.xml} \ -n={env:PYTEST_XDIST_PROC_NR:auto} \ {posargs:.} From 5a9c98a9cb8ac7c8aafffa1a6e15f4b65a1665e3 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 26 Mar 2019 17:14:45 +0000 Subject: [PATCH 7/9] do not delete logs within parallel --- src/tox/venv.py | 30 +++++++++++------ tests/unit/session/test_parallel.py | 34 +++++++++++++++---- tests/unit/test_z_cmdline.py | 52 ++++++++++++++--------------- 3 files changed, 74 insertions(+), 42 deletions(-) diff --git a/src/tox/venv.py b/src/tox/venv.py index 80a9f1912..cbd5bdbbf 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -12,6 +12,7 @@ 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.package.local import resolve_package from tox.util.path import ensure_empty_dir @@ -642,8 +643,6 @@ def prepend_shebang_interpreter(args): @tox.hookimpl def tox_testenv_create(venv, action): - if _SKIP_VENV_CREATION is True: - return True config_interpreter = venv.getsupportedinterpreter() args = [sys.executable, "-m", "virtualenv"] if venv.envconfig.sitepackages: @@ -654,17 +653,28 @@ def tox_testenv_create(venv, action): args.append("--no-download") # add interpreter explicitly, to prevent using default (virtualenv.ini) args.extend(["--python", str(config_interpreter)]) - ensure_empty_dir(venv.path) + + within_parallel = PARALLEL_ENV_VAR_KEY in os.environ + if within_parallel: + if venv.path.exists(): + # do not delete the log folder as that's used by parent + for content in venv.path.listdir(): + if not content.basename == "log": + content.remove(rec=1, ignore_errors=True) + else: + ensure_empty_dir(venv.path) + basepath = venv.path.dirpath() basepath.ensure(dir=1) args.append(venv.path.basename) - venv._pcall( - args, - venv=False, - action=action, - cwd=basepath, - redirect=reporter.verbosity() < reporter.Verbosity.DEBUG, - ) + if not _SKIP_VENV_CREATION: + venv._pcall( + args, + venv=False, + action=action, + cwd=basepath, + redirect=reporter.verbosity() < reporter.Verbosity.DEBUG, + ) return True # Return non-None to indicate plugin has completed diff --git a/tests/unit/session/test_parallel.py b/tests/unit/session/test_parallel.py index 16666ea18..eb3bf68b9 100644 --- a/tests/unit/session/test_parallel.py +++ b/tests/unit/session/test_parallel.py @@ -124,12 +124,6 @@ def test_parallel_deadlock(cmd, initproj, monkeypatch): [testenv] whitelist_externals = {} - -[testenv:e1] -commands = - python -c '[print("hello world") for _ in range(5000)]' - -[testenv:e2] commands = python -c '[print("hello world") for _ in range(5000)]' """.format( @@ -138,3 +132,31 @@ def test_parallel_deadlock(cmd, initproj, monkeypatch): initproj("pkg123-0.7", filedefs={"tox.ini": tox_ini}) cmd("-p", "2") # used to hang indefinitely + + +def test_parallel_recreate(cmd, initproj, monkeypatch): + monkeypatch.setenv(str("_TOX_SKIP_ENV_CREATION_TEST"), str("1")) + tox_ini = """\ +[tox] +envlist = e1,e2 +skipsdist = true + +[testenv] +whitelist_externals = {} +commands = + python -c '[print("hello world") for _ in range(1)]' +""".format( + sys.executable + ) + cwd = initproj("pkg123-0.7", filedefs={"tox.ini": tox_ini}) + log_dir = cwd / ".tox" / "e1" / "log" + assert not log_dir.exists() + cmd("-p", "2") + after = log_dir.listdir() + assert len(after) >= 2 + + res = cmd("-p", "2", "-rv") + assert res + end = log_dir.listdir() + assert len(end) >= 3 + assert not ({f.basename for f in after} - {f.basename for f in end}) diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py index 8bf5abb53..9231b2b35 100644 --- a/tests/unit/test_z_cmdline.py +++ b/tests/unit/test_z_cmdline.py @@ -87,14 +87,14 @@ def test_notoxini_help_still_works(initproj, cmd): assert result.err == msg assert result.out.startswith("usage: ") assert any("--help" in l for l in result.outlines), result.outlines - assert not result.ret + assert not result.ret, result.out def test_notoxini_help_ini_still_works(initproj, cmd): initproj("example123-0.5", filedefs={"tests": {"test_hello.py": "def test_hello(): pass"}}) result = cmd("--help-ini") assert any("setenv" in l for l in result.outlines), result.outlines - assert not result.ret + assert not result.ret, result.out def test_envdir_equals_toxini_errors_out(cmd, initproj): @@ -190,7 +190,7 @@ def test_skip_platform_mismatch(cmd, initproj): }, ) result = cmd() - assert not result.ret + assert not result.ret, result.out assert any( "SKIPPED: python: platform mismatch ({!r} does not match 'x123')".format(sys.platform) == l @@ -212,7 +212,7 @@ def test_skip_unknown_interpreter(cmd, initproj): }, ) result = cmd("--skip-missing-interpreters") - assert not result.ret + assert not result.ret, result.out msg = "SKIPPED: python: InterpreterNotFound: xyz_unknown_interpreter" assert any(msg == l for l in result.outlines), result.outlines @@ -232,7 +232,7 @@ def test_skip_unknown_interpreter_result_json(cmd, initproj, tmpdir): }, ) result = cmd("--skip-missing-interpreters", "--result-json", report_path) - assert not result.ret + assert not result.ret, result.out msg = "SKIPPED: python: InterpreterNotFound: xyz_unknown_interpreter" assert any(msg == l for l in result.outlines), result.outlines setup_result_from_json = json.load(report_path)["testenvs"]["python"]["setup"] @@ -432,12 +432,12 @@ def test_hello(pytestconfig): def test_toxuone_env(cmd, example123): result = cmd() - assert not result.ret + assert not result.ret, result.out assert re.match( r".*generated\W+xml\W+file.*junit-python\.xml" r".*\W+1\W+passed.*", result.out, re.DOTALL ) result = cmd("-epython") - assert not result.ret + assert not result.ret, result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", result.out, @@ -449,7 +449,7 @@ def test_different_config_cwd(cmd, example123): # see that things work with a different CWD with example123.dirpath().as_cwd(): result = cmd("-c", "example123/tox.ini") - assert not result.ret + assert not result.ret, result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", result.out, @@ -482,7 +482,7 @@ def test_developz(initproj, cmd): }, ) result = cmd("-vv", "--develop") - assert not result.ret + assert not result.ret, result.out assert "sdist-make" not in result.out @@ -497,7 +497,7 @@ def test_usedevelop(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret + assert not result.ret, result.out assert "sdist-make" not in result.out @@ -516,12 +516,12 @@ def test_usedevelop_mixed(initproj, cmd): # running only 'dev' should not do sdist result = cmd("-vv", "-e", "dev") - assert not result.ret + assert not result.ret, result.out assert "sdist-make" not in result.out # running all envs should do sdist result = cmd("-vv") - assert not result.ret + assert not result.ret, result.out assert "sdist-make" in result.out @@ -554,13 +554,13 @@ def test_hello(pytestconfig): }, ) result = cmd("-v") - assert not result.ret + assert not result.ret, result.out assert re.match( r".*generated\W+xml\W+file.*junit-python\.xml" r".*\W+1\W+passed.*", result.out, re.DOTALL ) assert "sdist-make" not in result.out result = cmd("-epython") - assert not result.ret + assert not result.ret, result.out assert "develop-inst-noop" in result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", @@ -571,7 +571,7 @@ def test_hello(pytestconfig): # see that things work with a different CWD with base.dirpath().as_cwd(): result = cmd("-c", "{}/tox.ini".format(name)) - assert not result.ret + assert not result.ret, result.out assert "develop-inst-noop" in result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", @@ -655,7 +655,7 @@ def test_alwayscopy(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret + assert not result.ret, result.out assert "virtualenv --always-copy" in result.out @@ -670,7 +670,7 @@ def test_alwayscopy_default(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret + assert not result.ret, result.out assert "virtualenv --always-copy" not in result.out @@ -687,7 +687,7 @@ def test_empty_activity_ignored(initproj, cmd): }, ) result = cmd() - assert not result.ret + assert not result.ret, result.out assert "installed:" not in result.out @@ -704,7 +704,7 @@ def test_empty_activity_shown_verbose(initproj, cmd): }, ) result = cmd("-v") - assert not result.ret + assert not result.ret, result.out assert "installed:" in result.out @@ -720,7 +720,7 @@ def test_test_piphelp(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret, "{}\n{}".format(result.err, result.err) + assert not result.ret, result.out, "{}\n{}".format(result.err, result.err) def test_notest(initproj, cmd): @@ -735,10 +735,10 @@ def test_notest(initproj, cmd): }, ) result = cmd("-v", "--notest") - assert not result.ret + assert not result.ret, result.out assert re.match(r".*summary.*" r"py26\W+skipped\W+tests.*", result.out, re.DOTALL) result = cmd("-v", "--notest", "-epy26") - assert not result.ret + assert not result.ret, result.out assert re.match(r".*py26\W+reusing.*", result.out, re.DOTALL) @@ -762,7 +762,7 @@ def test_PYC(initproj, cmd, monkeypatch): initproj("example123", filedefs={"tox.ini": ""}) monkeypatch.setenv("PYTHONDOWNWRITEBYTECODE", "1") result = cmd("-v", "--notest") - assert not result.ret + assert not result.ret, result.out assert "create" in result.out @@ -770,7 +770,7 @@ def test_env_VIRTUALENV_PYTHON(initproj, cmd, monkeypatch): initproj("example123", filedefs={"tox.ini": ""}) monkeypatch.setenv("VIRTUALENV_PYTHON", "/FOO") result = cmd("-v", "--notest") - assert not result.ret, result.outlines + assert not result.ret, result.out, result.outlines assert "create" in result.out @@ -870,10 +870,10 @@ def test_envtmpdir(initproj, cmd): ) result = cmd() - assert not result.ret + assert not result.ret, result.out result = cmd() - assert not result.ret + assert not result.ret, result.out def test_missing_env_fails(initproj, cmd): From ed7371aaefacc29e1938a5e6389ad2a4aee93516 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 26 Mar 2019 18:13:00 +0000 Subject: [PATCH 8/9] add assert success/fail tester methods --- src/tox/_pytestplugin.py | 6 + src/tox/package/builder/isolated.py | 2 + src/tox/session/__init__.py | 3 +- src/tox/session/commands/run/sequential.py | 14 ++- src/tox/venv.py | 6 +- tests/integration/test_jython_env_create.py | 2 +- tests/integration/test_package_int.py | 3 +- tests/unit/config/test_config.py | 8 +- .../builder/test_package_builder_isolated.py | 6 +- tests/unit/package/test_package.py | 2 +- tests/unit/package/test_package_view.py | 2 +- tests/unit/session/test_parallel.py | 10 +- tests/unit/session/test_session.py | 8 +- tests/unit/test_z_cmdline.py | 107 +++++++++--------- 14 files changed, 92 insertions(+), 87 deletions(-) diff --git a/src/tox/_pytestplugin.py b/src/tox/_pytestplugin.py index b8d77d0ec..1c852ac1d 100644 --- a/src/tox/_pytestplugin.py +++ b/src/tox/_pytestplugin.py @@ -145,6 +145,12 @@ def __repr__(self): self.ret, " ".join(str(i) for i in self.args), self.out, self.err ) + def assert_success(self): + assert self.ret == 0, "{}\n{}\n{}".format(self.ret, self.err, self.out) + + def assert_fail(self): + assert self.ret, "{}\n{}\n{}".format(self.ret, self.err, self.out) + class ReportExpectMock: def __init__(self): diff --git a/src/tox/package/builder/isolated.py b/src/tox/package/builder/isolated.py index a932674cd..56bb9d0c3 100644 --- a/src/tox/package/builder/isolated.py +++ b/src/tox/package/builder/isolated.py @@ -26,6 +26,8 @@ def build(config, session): if package_venv.setupenv(): package_venv.finishvenv() + if isinstance(package_venv.status, Exception): + raise package_venv.status build_requires = get_build_requires(build_info, package_venv, config.setupdir) # we need to filter out requirements already specified in pyproject.toml or user deps diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py index 2f658b4ed..5d9236e72 100644 --- a/src/tox/session/__init__.py +++ b/src/tox/session/__init__.py @@ -59,7 +59,8 @@ def main(args): config.logdir.ensure(dir=1) ensure_empty_dir(config.logdir) with set_os_env_var("TOX_WORK_DIR", config.toxworkdir): - retcode = build_session(config).runcommand() + session = build_session(config) + retcode = session.runcommand() if retcode is None: retcode = 0 raise SystemExit(retcode) diff --git a/src/tox/session/commands/run/sequential.py b/src/tox/session/commands/run/sequential.py index ccb4c7f58..d2c6625c1 100644 --- a/src/tox/session/commands/run/sequential.py +++ b/src/tox/session/commands/run/sequential.py @@ -18,7 +18,8 @@ def run_sequential(config, venv_dict): installpkg(venv, venv.package) runenvreport(venv, config) - runtestenv(venv, config) + if venv.status == 0: + runtestenv(venv, config) def develop_pkg(venv, setupdir): @@ -54,10 +55,13 @@ def runenvreport(venv, config): Run an environment report to show which package versions are installed in the venv """ - with venv.new_action("envreport") as action: - packages = config.pluginmanager.hook.tox_runenvreport(venv=venv, action=action) - action.setactivity("installed", ",".join(packages)) - venv.env_log.set_installed(packages) + try: + with venv.new_action("envreport") as action: + packages = config.pluginmanager.hook.tox_runenvreport(venv=venv, action=action) + action.setactivity("installed", ",".join(packages)) + venv.env_log.set_installed(packages) + except InvocationError as exception: + venv.status = exception def runtestenv(venv, config, redirect=False): diff --git a/src/tox/venv.py b/src/tox/venv.py index cbd5bdbbf..68a746533 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -577,11 +577,7 @@ def setupenv(self): "not supported by virtualenv. Error details: {!r}".format(e) ) except tox.exception.InvocationError as e: - status = ( - "Error creating virtualenv. Note that some special characters (e.g. ':' and " - "unicode symbols) in paths are not supported by virtualenv. Error details: " - "{!r}".format(e) - ) + status = e except tox.exception.InterpreterNotFound as e: status = e if self.envconfig.config.option.skip_missing_interpreters == "true": diff --git a/tests/integration/test_jython_env_create.py b/tests/integration/test_jython_env_create.py index 0bdb2a296..bccdbd423 100644 --- a/tests/integration/test_jython_env_create.py +++ b/tests/integration/test_jython_env_create.py @@ -16,4 +16,4 @@ def test_jython_create(initproj, cmd): }, ) result = cmd("--notest", "-vvv") - assert not result.ret, "{}\n{}".format(result.err, result.out) + result.assert_success() diff --git a/tests/integration/test_package_int.py b/tests/integration/test_package_int.py index 21a7199ab..e676c60c9 100644 --- a/tests/integration/test_package_int.py +++ b/tests/integration/test_package_int.py @@ -102,8 +102,7 @@ def test_package_poetry(initproj, cmd): def run(cmd, package): result = cmd("--sdistonly", "-e", "py", "-v", "-v") - - assert result.ret == 0, result.out + result.assert_success() package_venv = (Path() / ".tox" / ".package").resolve() assert ".package create: {}".format(package_venv) in result.outlines, result.out assert "write config to {}".format(package_venv / ".tox-config1") in result.out, result.out diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 0fad2df3a..082626b88 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -2727,7 +2727,7 @@ def test_config_specific_ini(self, tmpdir, cmd): def test_no_tox_ini(self, cmd, initproj): initproj("noini-0.5") result = cmd() - assert result.ret + result.assert_fail() msg = "ERROR: tox config file (either pyproject.toml, tox.ini, setup.cfg) not found\n" assert result.err == msg assert not result.out @@ -2768,11 +2768,11 @@ def test_showconfig_with_force_dep_version(self, cmd, initproj): }, ) result = cmd("--showconfig") - assert result.ret == 0 + result.assert_success() assert any(re.match(r".*deps.*dep1==2.3, dep2.*", l) for l in result.outlines) # override dep1 specific version, and force version for dep2 result = cmd("--showconfig", "--force-dep=dep1", "--force-dep=dep2==5.0") - assert result.ret == 0 + result.assert_success() assert any(re.match(r".*deps.*dep1, dep2==5.0.*", l) for l in result.outlines) @@ -3018,7 +3018,7 @@ def test_config_current_py(newconfig, current_tox_py, cmd, tmpdir, monkeypatch): ) assert config.envconfigs[current_tox_py] result = cmd() - assert result.ret == 0, result.out + result.assert_success() def test_posargs_relative_changedir(newconfig, tmpdir): diff --git a/tests/unit/package/builder/test_package_builder_isolated.py b/tests/unit/package/builder/test_package_builder_isolated.py index 471dc52c5..7cec74933 100644 --- a/tests/unit/package/builder/test_package_builder_isolated.py +++ b/tests/unit/package/builder/test_package_builder_isolated.py @@ -44,13 +44,13 @@ def test_dist_exists_version_change(mock_venv, initproj, cmd): }, ) result = cmd("-e", "py") - assert result.ret == 0, result.out + result.assert_success() new_code = base.join("setup.py").read_text("utf-8").replace("0.1", "0.2") base.join("setup.py").write_text(new_code, "utf-8") result = cmd("-e", "py") - assert result.ret == 0, result.out + result.assert_success() def test_package_isolated_no_pyproject_toml(initproj, cmd): @@ -64,7 +64,7 @@ def test_package_isolated_no_pyproject_toml(initproj, cmd): }, ) result = cmd("--sdistonly", "-e", "py") - assert result.ret == 1 + result.assert_fail() assert result.outlines == ["ERROR: missing {}".format(py.path.local().join("pyproject.toml"))] diff --git a/tests/unit/package/test_package.py b/tests/unit/package/test_package.py index 58584bad9..0a1b8bc09 100644 --- a/tests/unit/package/test_package.py +++ b/tests/unit/package/test_package.py @@ -17,7 +17,7 @@ def test_install_via_installpkg(mock_venv, initproj, cmd): ) fake_package = base.ensure(".tox", "dist", "pkg123-0.1.zip") result = cmd("-e", "py", "--notest", "--installpkg", str(fake_package.relto(base))) - assert result.ret == 0, result.out + result.assert_success() def test_installpkg(tmpdir, newconfig): diff --git a/tests/unit/package/test_package_view.py b/tests/unit/package/test_package_view.py index 63ba08e84..7c1cf4693 100644 --- a/tests/unit/package/test_package_view.py +++ b/tests/unit/package/test_package_view.py @@ -56,7 +56,7 @@ def test_separate_sdist(cmd, initproj, tmpdir): assert dist_share_files[0].check() result = cmd("-v", "--notest") - assert not result.ret, result.out + result.assert_success() msg = "python inst: {}".format(result.session.package) assert msg in result.out, result.out operation = "copied" if not hasattr(os, "link") else "links" diff --git a/tests/unit/session/test_parallel.py b/tests/unit/session/test_parallel.py index eb3bf68b9..797cf8218 100644 --- a/tests/unit/session/test_parallel.py +++ b/tests/unit/session/test_parallel.py @@ -1,6 +1,5 @@ from __future__ import absolute_import, unicode_literals -import os import sys import pytest @@ -27,7 +26,7 @@ def test_parallel(cmd, initproj): }, ) result = cmd("--parallel", "all") - assert result.ret == 0, "{}{}{}".format(result.err, os.linesep, result.out) + result.assert_success() def test_parallel_live(cmd, initproj): @@ -49,7 +48,7 @@ def test_parallel_live(cmd, initproj): }, ) result = cmd("--parallel", "all", "--parallel-live") - assert result.ret == 0, "{}{}{}".format(result.err, os.linesep, result.out) + result.assert_success() def test_parallel_circular(cmd, initproj): @@ -73,7 +72,7 @@ def test_parallel_circular(cmd, initproj): }, ) result = cmd("--parallel", "1") - assert result.ret == 1, result.out + result.assert_fail() assert result.out == "ERROR: circular dependency detected: a | b\n" @@ -99,9 +98,8 @@ def test_parallel_error_report(cmd, initproj, monkeypatch, live): ) args = ["-o"] if live else [] result = cmd("-p", "all", *args) + result.assert_fail() msg = result.out - assert result.ret == 1, msg - # for live we print the failure logfile, otherwise just stream through (no logfile present) assert "(exited with code 17)" in result.out, msg if not live: diff --git a/tests/unit/session/test_session.py b/tests/unit/session/test_session.py index fe5490778..e39fcd0c0 100644 --- a/tests/unit/session/test_session.py +++ b/tests/unit/session/test_session.py @@ -85,7 +85,7 @@ def test_skip_sdist(cmd, initproj): }, ) result = cmd() - assert result.ret == 0 + result.assert_success() def test_skip_install_skip_package(cmd, initproj, mock_venv): @@ -103,7 +103,7 @@ def test_skip_install_skip_package(cmd, initproj, mock_venv): }, ) result = cmd("--notest") - assert result.ret == 0 + result.assert_success() @pytest.fixture() @@ -124,7 +124,7 @@ def func(*args): }, ) result = cmd(*args) - assert result.ret == 0 + result.assert_success() active = [i.name for i in result.session.existing_venvs.values()] return active, result @@ -303,7 +303,7 @@ def test_command_prev_post_ok(cmd, initproj, mock_venv): }, ) result = cmd() - assert result.ret == 0 + result.assert_success() expected = textwrap.dedent( """ py run-test-pre: commands[0] | python -c 'print("pre")' diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py index 9231b2b35..baf4a9b20 100644 --- a/tests/unit/test_z_cmdline.py +++ b/tests/unit/test_z_cmdline.py @@ -87,14 +87,14 @@ def test_notoxini_help_still_works(initproj, cmd): assert result.err == msg assert result.out.startswith("usage: ") assert any("--help" in l for l in result.outlines), result.outlines - assert not result.ret, result.out + result.assert_success() def test_notoxini_help_ini_still_works(initproj, cmd): initproj("example123-0.5", filedefs={"tests": {"test_hello.py": "def test_hello(): pass"}}) result = cmd("--help-ini") assert any("setenv" in l for l in result.outlines), result.outlines - assert not result.ret, result.out + result.assert_success() def test_envdir_equals_toxini_errors_out(cmd, initproj): @@ -109,8 +109,10 @@ def test_envdir_equals_toxini_errors_out(cmd, initproj): ) result = cmd() assert result.outlines[1] == "ERROR: ConfigError: envdir must not equal toxinidir" - assert re.match(r"ERROR: venv \'python\' in .* would delete project", result.outlines[0]) - assert result.ret, "{}\n{}".format(result.err, result.out) + assert re.match( + r"ERROR: venv \'python\' in .* would delete project", result.outlines[0] + ), result.outlines[0] + result.assert_fail() def test_run_custom_install_command_error(cmd, initproj): @@ -124,12 +126,9 @@ def test_run_custom_install_command_error(cmd, initproj): }, ) result = cmd() - msg = ( - "ERROR: invocation failed (errno 2), args: tox.ini --exists-action w" - " .tox/.tmp/package/1/interp123-0.5.zip" - ) + result.assert_fail() + msg = "ERROR: invocation failed (errno 2), args: tox.ini --exists-action w" assert msg in result.out, result.out - assert result.ret, "{}\n{}".format(result.err, result.out) def test_unknown_interpreter_and_env(cmd, initproj): @@ -147,13 +146,13 @@ def test_unknown_interpreter_and_env(cmd, initproj): }, ) result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert any( "ERROR: InterpreterNotFound: xyz_unknown_interpreter" == l for l in result.outlines ), result.outlines result = cmd("-exyz") - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.out == "ERROR: unknown environment 'xyz'\n" @@ -171,7 +170,7 @@ def test_unknown_interpreter(cmd, initproj): }, ) result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert any( "ERROR: InterpreterNotFound: xyz_unknown_interpreter" == l for l in result.outlines ), result.outlines @@ -190,7 +189,7 @@ def test_skip_platform_mismatch(cmd, initproj): }, ) result = cmd() - assert not result.ret, result.out + result.assert_success() assert any( "SKIPPED: python: platform mismatch ({!r} does not match 'x123')".format(sys.platform) == l @@ -212,7 +211,7 @@ def test_skip_unknown_interpreter(cmd, initproj): }, ) result = cmd("--skip-missing-interpreters") - assert not result.ret, result.out + result.assert_success() msg = "SKIPPED: python: InterpreterNotFound: xyz_unknown_interpreter" assert any(msg == l for l in result.outlines), result.outlines @@ -232,7 +231,7 @@ def test_skip_unknown_interpreter_result_json(cmd, initproj, tmpdir): }, ) result = cmd("--skip-missing-interpreters", "--result-json", report_path) - assert not result.ret, result.out + result.assert_success() msg = "SKIPPED: python: InterpreterNotFound: xyz_unknown_interpreter" assert any(msg == l for l in result.outlines), result.outlines setup_result_from_json = json.load(report_path)["testenvs"]["python"]["setup"] @@ -254,7 +253,7 @@ def test_unknown_dep(cmd, initproj): }, ) result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.outlines[-1].startswith("ERROR: python: could not install deps [qweqwe123];") @@ -272,7 +271,7 @@ def test_venv_special_chars_issue252(cmd, initproj): }, ) result = cmd() - assert result.ret == 0, "{}\n{}".format(result.err, result.out) + result.assert_success() pattern = re.compile("special&&1 installed: .*pkg123==0.7.*") assert any(pattern.match(line) for line in result.outlines), result.outlines @@ -280,7 +279,7 @@ def test_venv_special_chars_issue252(cmd, initproj): def test_unknown_environment(cmd, initproj): initproj("env123-0.7", filedefs={"tox.ini": ""}) result = cmd("-e", "qpwoei") - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.out == "ERROR: unknown environment 'qpwoei'\n" @@ -295,7 +294,7 @@ def test_unknown_environment_with_envlist(cmd, initproj): }, ) result = cmd("-e", "py36-djagno21") - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.out == "ERROR: unknown environment 'py36-djagno21'\n" @@ -310,7 +309,7 @@ def test_minimal_setup_py_empty(cmd, initproj): }, ) result = cmd() - assert result.ret == 1, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.outlines[-1] == "ERROR: setup.py is empty" @@ -326,7 +325,7 @@ def test_minimal_setup_py_comment_only(cmd, initproj): }, ) result = cmd() - assert result.ret == 1, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.outlines[-1] == "ERROR: setup.py is empty" @@ -343,7 +342,7 @@ def test_minimal_setup_py_non_functional(cmd, initproj): }, ) result = cmd() - assert result.ret == 1, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert any(re.match(r".*ERROR.*check setup.py.*", l) for l in result.outlines), result.outlines @@ -359,7 +358,7 @@ def test_sdist_fails(cmd, initproj): }, ) result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert any( re.match(r".*FAIL.*could not package project.*", l) for l in result.outlines ), result.outlines @@ -377,7 +376,7 @@ def test_no_setup_py_exits(cmd, initproj): ) os.remove("setup.py") result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert any( re.match(r".*ERROR.*No setup.py file found.*", l) for l in result.outlines ), result.outlines @@ -404,7 +403,7 @@ def test_package_install_fails(cmd, initproj): }, ) result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.outlines[-1].startswith("ERROR: python: InvocationError for command ") @@ -432,12 +431,12 @@ def test_hello(pytestconfig): def test_toxuone_env(cmd, example123): result = cmd() - assert not result.ret, result.out + result.assert_success() assert re.match( r".*generated\W+xml\W+file.*junit-python\.xml" r".*\W+1\W+passed.*", result.out, re.DOTALL ) result = cmd("-epython") - assert not result.ret, result.out + result.assert_success() assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", result.out, @@ -449,7 +448,7 @@ def test_different_config_cwd(cmd, example123): # see that things work with a different CWD with example123.dirpath().as_cwd(): result = cmd("-c", "example123/tox.ini") - assert not result.ret, result.out + result.assert_success() assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", result.out, @@ -464,7 +463,7 @@ def test_json(cmd, example123): testfile.write("def test_fail(): assert 0") jsonpath = example123.join("res.json") result = cmd("--result-json", jsonpath) - assert result.ret == 1, "{}\n{}".format(result.err, result.out) + result.assert_fail() with jsonpath.open("r") as f: data = json.load(f) verify_json_report_format(data) @@ -482,7 +481,7 @@ def test_developz(initproj, cmd): }, ) result = cmd("-vv", "--develop") - assert not result.ret, result.out + result.assert_success() assert "sdist-make" not in result.out @@ -497,7 +496,7 @@ def test_usedevelop(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret, result.out + result.assert_success() assert "sdist-make" not in result.out @@ -516,12 +515,12 @@ def test_usedevelop_mixed(initproj, cmd): # running only 'dev' should not do sdist result = cmd("-vv", "-e", "dev") - assert not result.ret, result.out + result.assert_success() assert "sdist-make" not in result.out # running all envs should do sdist result = cmd("-vv") - assert not result.ret, result.out + result.assert_success() assert "sdist-make" in result.out @@ -554,13 +553,13 @@ def test_hello(pytestconfig): }, ) result = cmd("-v") - assert not result.ret, result.out + result.assert_success() assert re.match( r".*generated\W+xml\W+file.*junit-python\.xml" r".*\W+1\W+passed.*", result.out, re.DOTALL ) assert "sdist-make" not in result.out result = cmd("-epython") - assert not result.ret, result.out + result.assert_success() assert "develop-inst-noop" in result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", @@ -571,7 +570,7 @@ def test_hello(pytestconfig): # see that things work with a different CWD with base.dirpath().as_cwd(): result = cmd("-c", "{}/tox.ini".format(name)) - assert not result.ret, result.out + result.assert_success() assert "develop-inst-noop" in result.out assert re.match( r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*", @@ -584,7 +583,7 @@ def test_hello(pytestconfig): assert testfile.check() testfile.write("def test_fail(): assert 0") result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert "develop-inst-noop" in result.out assert re.match( r".*\W+1\W+failed.*" r"summary.*" r"python:\W+commands\W+failed.*", result.out, re.DOTALL @@ -594,7 +593,7 @@ def test_hello(pytestconfig): setup_py = py.path.local("setup.py") setup_py.write(setup_py.read() + " ") result = cmd() - assert result.ret, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert "develop-inst-nodeps" in result.out @@ -655,7 +654,7 @@ def test_alwayscopy(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret, result.out + result.assert_success() assert "virtualenv --always-copy" in result.out @@ -670,7 +669,7 @@ def test_alwayscopy_default(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret, result.out + result.assert_success() assert "virtualenv --always-copy" not in result.out @@ -687,7 +686,7 @@ def test_empty_activity_ignored(initproj, cmd): }, ) result = cmd() - assert not result.ret, result.out + result.assert_success() assert "installed:" not in result.out @@ -704,7 +703,7 @@ def test_empty_activity_shown_verbose(initproj, cmd): }, ) result = cmd("-v") - assert not result.ret, result.out + result.assert_success() assert "installed:" in result.out @@ -720,7 +719,7 @@ def test_test_piphelp(initproj, cmd): }, ) result = cmd("-vv") - assert not result.ret, result.out, "{}\n{}".format(result.err, result.err) + result.assert_success() def test_notest(initproj, cmd): @@ -735,10 +734,10 @@ def test_notest(initproj, cmd): }, ) result = cmd("-v", "--notest") - assert not result.ret, result.out + result.assert_success() assert re.match(r".*summary.*" r"py26\W+skipped\W+tests.*", result.out, re.DOTALL) result = cmd("-v", "--notest", "-epy26") - assert not result.ret, result.out + result.assert_success() assert re.match(r".*py26\W+reusing.*", result.out, re.DOTALL) @@ -754,7 +753,7 @@ def test_notest_setup_py_error(initproj, cmd): }, ) result = cmd("--notest") - assert result.ret + result.assert_fail() assert re.search("ERROR:.*InvocationError", result.out) @@ -762,7 +761,7 @@ def test_PYC(initproj, cmd, monkeypatch): initproj("example123", filedefs={"tox.ini": ""}) monkeypatch.setenv("PYTHONDOWNWRITEBYTECODE", "1") result = cmd("-v", "--notest") - assert not result.ret, result.out + result.assert_success() assert "create" in result.out @@ -770,7 +769,7 @@ def test_env_VIRTUALENV_PYTHON(initproj, cmd, monkeypatch): initproj("example123", filedefs={"tox.ini": ""}) monkeypatch.setenv("VIRTUALENV_PYTHON", "/FOO") result = cmd("-v", "--notest") - assert not result.ret, result.out, result.outlines + result.assert_success() assert "create" in result.out @@ -786,7 +785,7 @@ def test_envsitepackagesdir(cmd, initproj): }, ) result = cmd() - assert result.ret == 0, "{}\n{}".format(result.err, result.out) + result.assert_success() assert re.match(r".*\nX:.*tox.*site-packages.*", result.out, re.DOTALL) @@ -803,7 +802,7 @@ def test_envsitepackagesdir_skip_missing_issue280(cmd, initproj): }, ) result = cmd("--skip-missing-interpreters") - assert result.ret == 0, "{}\n{}".format(result.err, result.out) + result.assert_success() assert re.match(r".*SKIPPED:.*qwelkj.*", result.out, re.DOTALL) @@ -818,7 +817,7 @@ def test_verbosity(cmd, initproj, verbosity): }, ) result = cmd(verbosity) - assert result.ret == 0, "{}\n{}".format(result.err, result.out) + result.assert_success() needle = "Successfully installed pkgX-0.0.5" if verbosity == "-vv": @@ -870,16 +869,16 @@ def test_envtmpdir(initproj, cmd): ) result = cmd() - assert not result.ret, result.out + result.assert_success() result = cmd() - assert not result.ret, result.out + result.assert_success() def test_missing_env_fails(initproj, cmd): initproj("foo", filedefs={"tox.ini": "[testenv:foo]\ncommands={env:VAR}"}) result = cmd() - assert result.ret == 1, "{}\n{}".format(result.err, result.out) + result.assert_fail() assert result.out.endswith( "foo: unresolvable substitution(s): 'VAR'." " Environment variables are missing or defined recursively.\n" From 969a3776918a6106aa025d1e856d5c0444ba8f5f Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Wed, 27 Mar 2019 10:13:01 +0000 Subject: [PATCH 9/9] check failure does not print congratulations and success does --- .../{1177.bugfix.rst => 1143.feature.rst} | 0 docs/changelog/1159.feature.rst | 1 + src/tox/_pytestplugin.py | 52 +++++++++- src/tox/action.py | 97 +++++++++++-------- src/tox/session/commands/run/sequential.py | 4 +- src/tox/venv.py | 35 ++++--- tests/integration/test_package_int.py | 4 +- tests/integration/test_provision_int.py | 2 +- tests/unit/config/test_config.py | 4 +- tests/unit/package/test_package_parallel.py | 3 + tests/unit/session/test_list_env.py | 3 +- tests/unit/session/test_session.py | 6 +- tests/unit/test_venv.py | 20 ++-- tests/unit/test_z_cmdline.py | 10 +- 14 files changed, 160 insertions(+), 81 deletions(-) rename docs/changelog/{1177.bugfix.rst => 1143.feature.rst} (100%) create mode 100644 docs/changelog/1159.feature.rst diff --git a/docs/changelog/1177.bugfix.rst b/docs/changelog/1143.feature.rst similarity index 100% rename from docs/changelog/1177.bugfix.rst rename to docs/changelog/1143.feature.rst diff --git a/docs/changelog/1159.feature.rst b/docs/changelog/1159.feature.rst new file mode 100644 index 000000000..ea5237d30 --- /dev/null +++ b/docs/changelog/1159.feature.rst @@ -0,0 +1 @@ +Parallel children now are added to ``--result-json`` - by :user:`gaborbernat` diff --git a/src/tox/_pytestplugin.py b/src/tox/_pytestplugin.py index 1c852ac1d..d23f2dc8f 100644 --- a/src/tox/_pytestplugin.py +++ b/src/tox/_pytestplugin.py @@ -15,6 +15,7 @@ import tox from tox import venv from tox.config import parseconfig +from tox.config.parallel import ENV_VAR_KEY as PARALLEL_ENV_VAR_KEY from tox.reporter import update_default_reporter from tox.session import Session, main, setup_reporter from tox.venv import CreationConfig, VirtualEnv, getdigest @@ -58,6 +59,40 @@ def check_cwd_not_changed_by_test(): pytest.fail("test changed cwd: {!r} => {!r}".format(old, new)) +@pytest.fixture(autouse=True) +def check_os_environ_stable(): + old = os.environ.copy() + + to_clean = { + k: os.environ.pop(k, None) + for k in {PARALLEL_ENV_VAR_KEY, str("TOX_WORK_DIR"), str("PYTHONPATH")} + } + + yield + + for key, value in to_clean.items(): + if value is not None: + os.environ[key] = value + + new = os.environ + extra = {k: new[k] for k in set(new) - set(old)} + miss = {k: old[k] for k in set(old) - set(new)} + diff = { + "{} = {} vs {}".format(k, old[k], new[k]) + for k in set(old) & set(new) + if old[k] != new[k] and not k.startswith("PYTEST_") + } + if extra or miss or diff: + msg = "test changed environ" + if extra: + msg += " extra {}".format(extra) + if miss: + msg += " miss {}".format(miss) + if diff: + msg += " diff {}".format(diff) + pytest.fail(msg) + + @pytest.fixture(name="newconfig") def create_new_config_file(tmpdir): def create_new_config_file_(args, source=None, plugins=()): @@ -145,11 +180,20 @@ def __repr__(self): self.ret, " ".join(str(i) for i in self.args), self.out, self.err ) - def assert_success(self): - assert self.ret == 0, "{}\n{}\n{}".format(self.ret, self.err, self.out) + def output(self): + return "{}\n{}\n{}".format(self.ret, self.err, self.out) + + def assert_success(self, is_run_test_env=True): + msg = self.output() + assert self.ret == 0, msg + if is_run_test_env: + assert any(" congratulations :)" == l for l in reversed(self.outlines)), msg - def assert_fail(self): - assert self.ret, "{}\n{}\n{}".format(self.ret, self.err, self.out) + def assert_fail(self, is_run_test_env=True): + msg = self.output() + assert self.ret, msg + if is_run_test_env: + assert not any(" congratulations :)" == l for l in reversed(self.outlines)), msg class ReportExpectMock: diff --git a/src/tox/action.py b/src/tox/action.py index 07a4e265e..7adee59b3 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -14,6 +14,7 @@ from tox import reporter from tox.constants import INFO from tox.exception import InvocationError +from tox.reporter import Verbosity from tox.util.lock import get_unique_file from tox.util.stdlib import is_main_thread @@ -66,19 +67,20 @@ def popen( ignore_ret=False, capture_err=True, callback=None, + report_fail=True, ): """this drives an interaction with a subprocess""" - args = self._rewrite_args(cwd, args) - cmd_args = [str(x) for x in args] + cwd = py.path.local() if cwd is None else cwd + cmd_args = [str(x) for x in self._rewrite_args(cwd, args)] cmd_args_shell = " ".join(pipes.quote(i) for i in cmd_args) stream_getter = self._get_standard_streams( - capture_err, cmd_args_shell, redirect, returnout + capture_err, cmd_args_shell, redirect, returnout, cwd ) - cwd = py.path.local(os.getcwd()) if cwd is None else cwd + exit_code, output = None, None with stream_getter as (fin, out_path, stderr, stdout): try: process = self.via_popen( - args, + cmd_args, stdout=stdout, stderr=stderr, cwd=str(cwd), @@ -92,33 +94,33 @@ def popen( # needed for Windows signal send ability (CTRL+C) ), ) - except OSError as e: - reporter.error( - "invocation failed (errno {:d}), args: {}, cwd: {}".format( - e.errno, cmd_args_shell, cwd - ) - ) - raise - if callback is not None: - callback(process) - reporter.log_popen(cwd, out_path, cmd_args_shell, process.pid) - - output = self.evaluate_cmd(fin, process, redirect) - exit_code = process.returncode - if exit_code and not ignore_ret: - if out_path: - reporter.error( - "invocation failed (exit code {:d}), logfile: {}".format(exit_code, out_path) - ) - output = out_path.read() if out_path.exists() else None - reporter.error(output) - self.command_log.add_command(args, output, exit_code) - raise InvocationError(cmd_args_shell, exit_code, out_path) + except OSError as exception: + exit_code = exception.errno else: - raise InvocationError(cmd_args_shell, exit_code) - if not output and out_path is not None and out_path.exists(): - output = out_path.read() - self.command_log.add_command(args, output, exit_code) + if callback is not None: + callback(process) + reporter.log_popen(cwd, out_path, cmd_args_shell, process.pid) + output = self.evaluate_cmd(fin, process, redirect) + exit_code = process.returncode + finally: + if out_path is not None and out_path.exists(): + output = out_path.read() + try: + if exit_code and not ignore_ret: + if report_fail: + msg = "invocation failed (exit code {:d})".format(exit_code) + if out_path is not None: + msg += ", logfile: {}".format(out_path) + if not out_path.exists(): + msg += " warning log file missing" + reporter.error(msg) + if out_path is not None and out_path.exists(): + reporter.separator("=", "log start", Verbosity.QUIET) + reporter.quiet(output) + reporter.separator("=", "log end", Verbosity.QUIET) + raise InvocationError(cmd_args_shell, exit_code, output) + finally: + self.command_log.add_command(cmd_args, output, exit_code) return output def evaluate_cmd(self, input_file_handler, process, redirect): @@ -201,7 +203,7 @@ def _wait(process, timeout): return process.poll() @contextmanager - def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout): + def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout, cwd): stdout = out_path = input_file_handler = None stderr = subprocess.STDOUT if capture_err else None @@ -209,13 +211,12 @@ def _get_standard_streams(self, capture_err, cmd_args_shell, redirect, returnout out_path = self.get_log_path(self.name) with out_path.open("wt") as stdout, out_path.open("rb") as input_file_handler: stdout.write( - "action: {}\nmsg: {}\ncmd: {!r}\n\n".format( - self.name, self.msg, cmd_args_shell + "action: {}, msg: {}\ncwd: {}\ncmd: {}\n".format( + self.name, self.msg, cwd, cmd_args_shell ) ) stdout.flush() input_file_handler.read() # read the header, so it won't be written to stdout - yield input_file_handler, out_path, stderr, stdout return @@ -229,20 +230,30 @@ def get_log_path(self, actionid): return log_file def _rewrite_args(self, cwd, args): - new_args = [] - cwd = py.path.local(cwd) + + executable = None + if INFO.IS_WIN: + # shebang lines are not adhered on Windows so if it's a python script + # pre-pend the interpreter + ext = os.path.splitext(str(args[0]))[1].lower() + if ext == ".py": + executable = str(self.python) + if executable is None: + executable = args[0] + args = args[1:] + + new_args = [executable] + + # to make the command shorter try to use relative paths for all subsequent arguments + # note the executable cannot be relative as the Windows applies cwd after invocation for arg in args: if arg and os.path.isabs(str(arg)): arg_path = py.path.local(arg) if arg_path.exists() and arg_path.common(cwd) is not None: potential_arg = cwd.bestrelpath(arg_path) if len(potential_arg.split("..")) < 2: - # just one parent directory accepted + # just one parent directory accepted as relative path arg = potential_arg new_args.append(str(arg)) - # subprocess does not always take kindly to .py scripts so adding the interpreter here - if INFO.IS_WIN: - ext = os.path.splitext(str(new_args[0]))[1].lower() - if ext == ".py": - new_args = [str(self.python)] + new_args + return new_args diff --git a/src/tox/session/commands/run/sequential.py b/src/tox/session/commands/run/sequential.py index d2c6625c1..907690985 100644 --- a/src/tox/session/commands/run/sequential.py +++ b/src/tox/session/commands/run/sequential.py @@ -16,8 +16,8 @@ def run_sequential(config, venv_dict): venv.finishvenv() else: installpkg(venv, venv.package) - - runenvreport(venv, config) + if venv.status == 0: + runenvreport(venv, config) if venv.status == 0: runtestenv(venv, config) diff --git a/src/tox/venv.py b/src/tox/venv.py index 68a746533..18a9dd868 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -381,7 +381,8 @@ def expand(val): cmd = list(chain.from_iterable(expand(val) for val in self.envconfig.install_command)) - self.ensure_pip_os_environ_ok() + env = self._get_os_environ() + self.ensure_pip_os_environ_ok(env) old_stdout = sys.stdout sys.stdout = codecs.getwriter("utf8")(sys.stdout) @@ -391,17 +392,18 @@ def expand(val): cwd=self.envconfig.config.toxinidir, action=action, redirect=reporter.verbosity() < reporter.Verbosity.DEBUG, + env=env, ) finally: sys.stdout = old_stdout - def ensure_pip_os_environ_ok(self): + def ensure_pip_os_environ_ok(self, env): for key in ("PIP_RESPECT_VIRTUALENV", "PIP_REQUIRE_VIRTUALENV", "__PYVENV_LAUNCHER__"): - os.environ.pop(key, None) - if "PYTHONPATH" not in self.envconfig.passenv: + env.pop(key, None) + if all("PYTHONPATH" not in i for i in (self.envconfig.passenv, self.envconfig.setenv)): # If PYTHONPATH not explicitly asked for, remove it. - if "PYTHONPATH" in os.environ: - if sys.version_info < (3, 4) or bool(os.environ["PYTHONPATH"]): + if "PYTHONPATH" in env: + if sys.version_info < (3, 4) or bool(env["PYTHONPATH"]): # https://docs.python.org/3/whatsnew/3.4.html#changes-in-python-command-behavior # In a posix shell, setting the PATH environment variable to an empty value is # equivalent to not setting it at all. @@ -409,13 +411,13 @@ def ensure_pip_os_environ_ok(self): "Discarding $PYTHONPATH from environment, to override " "specify PYTHONPATH in 'passenv' in your configuration." ) - os.environ.pop("PYTHONPATH") + env.pop("PYTHONPATH") # installing packages at user level may mean we're not installing inside the venv - os.environ["PIP_USER"] = "0" + env["PIP_USER"] = "0" # installing without dependencies may lead to broken packages - os.environ["PIP_NO_DEPS"] = "0" + env["PIP_NO_DEPS"] = "0" def _install(self, deps, extraopts=None, action=None): if not deps: @@ -533,10 +535,13 @@ def _pcall( redirect=True, ignore_ret=False, returnout=False, + env=None, ): + if env is None: + env = self._get_os_environ(is_test_command=is_test_command) + # construct environment variables - os.environ.pop("VIRTUALENV_PYTHON", None) - env = self._get_os_environ(is_test_command=is_test_command) + env.pop("VIRTUALENV_PYTHON", None) bin_dir = str(self.envconfig.envbindir) env["PATH"] = os.pathsep.join([bin_dir, os.environ["PATH"]]) reporter.verbosity2("setting PATH={}".format(env["PATH"])) @@ -548,7 +553,13 @@ def _pcall( cwd.ensure(dir=1) # ensure the cwd exists return action.popen( - args, cwd=cwd, env=env, redirect=redirect, ignore_ret=ignore_ret, returnout=returnout + args, + cwd=cwd, + env=env, + redirect=redirect, + ignore_ret=ignore_ret, + returnout=returnout, + report_fail=not is_test_command, ) def setupenv(self): diff --git a/tests/integration/test_package_int.py b/tests/integration/test_package_int.py index e676c60c9..98be6d9ba 100644 --- a/tests/integration/test_package_int.py +++ b/tests/integration/test_package_int.py @@ -102,7 +102,7 @@ def test_package_poetry(initproj, cmd): def run(cmd, package): result = cmd("--sdistonly", "-e", "py", "-v", "-v") - result.assert_success() + result.assert_success(is_run_test_env=False) package_venv = (Path() / ".tox" / ".package").resolve() assert ".package create: {}".format(package_venv) in result.outlines, result.out assert "write config to {}".format(package_venv / ".tox-config1") in result.out, result.out @@ -114,7 +114,7 @@ def run(cmd, package): # second call re-uses result2 = cmd("--sdistonly", "-e", "py", "-v", "-v") - assert result2.ret == 0, result2.out + result2.assert_success(is_run_test_env=False) assert ( ".package reusing: {}".format(package_venv) in result2.outlines ), "Second call output:\n{}First call output:\n{}".format(result2.out, result.out) diff --git a/tests/integration/test_provision_int.py b/tests/integration/test_provision_int.py index 7c03920db..2c8a75ea9 100644 --- a/tests/integration/test_provision_int.py +++ b/tests/integration/test_provision_int.py @@ -24,7 +24,7 @@ def test_provision_missing(initproj, cmd): }, ) result = cmd("-q", "-q") - assert result.ret == 1 + result.assert_fail() meta_python = Path(result.out.strip()) assert meta_python.exists() diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 082626b88..22fd6e93a 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -2768,11 +2768,11 @@ def test_showconfig_with_force_dep_version(self, cmd, initproj): }, ) result = cmd("--showconfig") - result.assert_success() + result.assert_success(is_run_test_env=False) assert any(re.match(r".*deps.*dep1==2.3, dep2.*", l) for l in result.outlines) # override dep1 specific version, and force version for dep2 result = cmd("--showconfig", "--force-dep=dep1", "--force-dep=dep2==5.0") - result.assert_success() + result.assert_success(is_run_test_env=False) assert any(re.match(r".*deps.*dep1, dep2==5.0.*", l) for l in result.outlines) diff --git a/tests/unit/package/test_package_parallel.py b/tests/unit/package/test_package_parallel.py index 39ea31f5f..fa376164a 100644 --- a/tests/unit/package/test_package_parallel.py +++ b/tests/unit/package/test_package_parallel.py @@ -1,3 +1,4 @@ +import os import platform import traceback @@ -126,3 +127,5 @@ def build_package(config, session): assert len(dist_after) == 1 sdist = dist_after[0] assert t1_package != sdist + # 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/session/test_list_env.py b/tests/unit/session/test_list_env.py index e48e532a9..364049033 100644 --- a/tests/unit/session/test_list_env.py +++ b/tests/unit/session/test_list_env.py @@ -1,4 +1,5 @@ -def test_listenvs(cmd, initproj): +def test_listenvs(cmd, initproj, monkeypatch): + monkeypatch.delenv(str("TOXENV"), raising=False) initproj( "listenvs", filedefs={ diff --git a/tests/unit/session/test_session.py b/tests/unit/session/test_session.py index e39fcd0c0..eefef4e1b 100644 --- a/tests/unit/session/test_session.py +++ b/tests/unit/session/test_session.py @@ -124,7 +124,7 @@ def func(*args): }, ) result = cmd(*args) - result.assert_success() + result.assert_success(is_run_test_env=False) active = [i.name for i in result.session.existing_venvs.values()] return active, result @@ -279,7 +279,7 @@ def test_tox_env_var_flags_inserted_isolated(popen_env_test): def assert_popen_env(res): - assert res.result.ret == 0, res.result.out + res.result.assert_success() for tox_id, _, env, __, ___ in res.popens: assert env["TOX_WORK_DIR"] == os.path.join(res.cwd, ".tox") if tox_id != "GLOB": @@ -340,7 +340,7 @@ def test_command_prev_fail_command_skip_post_run(cmd, initproj, mock_venv): }, ) result = cmd() - assert result.ret == 1 + result.assert_fail() expected = textwrap.dedent( """ py run-test-pre: commands[0] | python -c 'raise SystemExit(2)' diff --git a/tests/unit/test_venv.py b/tests/unit/test_venv.py index 5a4eaf511..55d884221 100644 --- a/tests/unit/test_venv.py +++ b/tests/unit/test_venv.py @@ -623,13 +623,21 @@ def test_envbindir_path(self, newmocksession, monkeypatch): monkeypatch.setenv("PIP_RESPECT_VIRTUALENV", "1") monkeypatch.setenv("PIP_REQUIRE_VIRTUALENV", "1") monkeypatch.setenv("__PYVENV_LAUNCHER__", "1") + + prev_pcall = venv._pcall + + def collect(*args, **kwargs): + env = kwargs["env"] + assert "PIP_RESPECT_VIRTUALENV" not in env + assert "PIP_REQUIRE_VIRTUALENV" not in env + assert "__PYVENV_LAUNCHER__" not in env + assert env["PIP_USER"] == "0" + assert env["PIP_NO_DEPS"] == "0" + return prev_pcall(*args, **kwargs) + + monkeypatch.setattr(venv, "_pcall", collect) with pytest.raises(ZeroDivisionError): venv.run_install_command(["qwe"], action=action) - assert "PIP_RESPECT_VIRTUALENV" not in os.environ - assert "PIP_REQUIRE_VIRTUALENV" not in os.environ - assert "__PYVENV_LAUNCHER__" not in os.environ - assert os.environ["PIP_USER"] == "0" - assert os.environ["PIP_NO_DEPS"] == "0" def test_pythonpath_remove(self, newmocksession, monkeypatch, caplog): monkeypatch.setenv("PYTHONPATH", "/my/awesome/library") @@ -643,7 +651,6 @@ def test_pythonpath_remove(self, newmocksession, monkeypatch, caplog): venv = mocksession.getvenv("python") with mocksession.newaction(venv.name, "getenv") as action: venv.run_install_command(["qwe"], action=action) - assert "PYTHONPATH" not in os.environ mocksession.report.expect("warning", "*Discarding $PYTHONPATH from environment*") pcalls = mocksession._pcalls @@ -683,7 +690,6 @@ def test_pythonpath_empty(self, newmocksession, monkeypatch, caplog): venv = mocksession.getvenv("python") with mocksession.newaction(venv.name, "getenv") as action: venv.run_install_command(["qwe"], action=action) - assert "PYTHONPATH" not in os.environ if sys.version_info < (3, 4): mocksession.report.expect("warning", "*Discarding $PYTHONPATH from environment*") else: diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py index baf4a9b20..7b523c197 100644 --- a/tests/unit/test_z_cmdline.py +++ b/tests/unit/test_z_cmdline.py @@ -87,14 +87,14 @@ def test_notoxini_help_still_works(initproj, cmd): assert result.err == msg assert result.out.startswith("usage: ") assert any("--help" in l for l in result.outlines), result.outlines - result.assert_success() + result.assert_success(is_run_test_env=False) def test_notoxini_help_ini_still_works(initproj, cmd): initproj("example123-0.5", filedefs={"tests": {"test_hello.py": "def test_hello(): pass"}}) result = cmd("--help-ini") assert any("setenv" in l for l in result.outlines), result.outlines - result.assert_success() + result.assert_success(is_run_test_env=False) def test_envdir_equals_toxini_errors_out(cmd, initproj): @@ -127,8 +127,10 @@ def test_run_custom_install_command_error(cmd, initproj): ) result = cmd() result.assert_fail() - msg = "ERROR: invocation failed (errno 2), args: tox.ini --exists-action w" - assert msg in result.out, result.out + re.match( + r"ERROR: python: InvocationError for command .* \(exited with code \d+\)", + result.outlines[-1], + ), result.out def test_unknown_interpreter_and_env(cmd, initproj):