Skip to content

Commit

Permalink
feat: allow configurable child shutdown timeouts (#565)
Browse files Browse the repository at this point in the history
* feat: allow configurable child shutdown timeouts

* fix: mypy

* updated code from review comments

* added new test

* updated from review comments

* added docs

* whitespace

* more style matching

* typo
  • Loading branch information
raddessi committed Feb 22, 2022
1 parent d15dde9 commit bcee2dc
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 6 deletions.
15 changes: 10 additions & 5 deletions nox/popen.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,19 @@
from typing import IO, Mapping, Sequence


def shutdown_process(proc: subprocess.Popen) -> tuple[bytes, bytes]:
def shutdown_process(
proc: subprocess.Popen,
interrupt_timeout: float | None,
terminate_timeout: float | None,
) -> tuple[bytes, bytes]:
"""Gracefully shutdown a child process."""

with contextlib.suppress(subprocess.TimeoutExpired):
return proc.communicate(timeout=0.3)
return proc.communicate(timeout=interrupt_timeout)

proc.terminate()

with contextlib.suppress(subprocess.TimeoutExpired):
return proc.communicate(timeout=0.2)
return proc.communicate(timeout=terminate_timeout)

proc.kill()

Expand Down Expand Up @@ -60,6 +63,8 @@ def popen(
silent: bool = False,
stdout: int | IO | None = None,
stderr: int | IO = subprocess.STDOUT,
interrupt_timeout: float | None = 0.3,
terminate_timeout: float | None = 0.2,
) -> tuple[int, str]:
if silent and stdout is not None:
raise ValueError(
Expand All @@ -77,7 +82,7 @@ def popen(
sys.stdout.flush()

except KeyboardInterrupt:
out, err = shutdown_process(proc)
out, err = shutdown_process(proc, interrupt_timeout, terminate_timeout)
if proc.returncode != 0:
raise

Expand Down
31 changes: 31 additions & 0 deletions nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,17 @@ def run(
'bash', '-c', 'echo $SOME_ENV',
env={'SOME_ENV': 'Hello'})
You can extend the shutdown timeout to allow long-running cleanup tasks to
complete before being terminated. For example, if you wanted to allow ``pytest``
extra time to clean up large projects in the case that nox receives an
interrupt signal from your build system and needs to terminate its child
processes::
session.run(
'pytest', '-k', 'long_cleanup',
interrupt_timeout=10.0,
terminate_timeout=2.0)
You can also tell nox to treat non-zero exit codes as success using
``success_codes``. For example, if you wanted to treat the ``pytest``
"tests discovered, but none selected" error as success::
Expand Down Expand Up @@ -308,6 +319,16 @@ def run(
``--error-on-external-run``. This has no effect for sessions that
do not have a virtualenv.
:type external: bool
:param interrupt_timeout: The timeout (in seconds) that nox should wait after it
and its children receive an interrupt signal before sending a terminate
signal to its children. Set to ``None`` to never send a terminate signal.
Default: ``0.3``
:type interrupt_timeout: float or None
:param terminate_timeout: The timeout (in seconds) that nox should wait after it
sends a terminate signal to its children before sending a kill signal to
them. Set to ``None`` to never send a kill signal.
Default: ``0.2``
:type terminate_timeout: float or None
"""
if not args:
raise ValueError("At least one argument required to run().")
Expand Down Expand Up @@ -348,6 +369,16 @@ def run_always(
``--error-on-external-run``. This has no effect for sessions that
do not have a virtualenv.
:type external: bool
:param interrupt_timeout: The timeout (in seconds) that nox should wait after it
and its children receive an interrupt signal before sending a terminate
signal to its children. Set to ``None`` to never send a terminate signal.
Default: ``0.3``
:type interrupt_timeout: float or None
:param terminate_timeout: The timeout (in seconds) that nox should wait after it
sends a terminate signal to its children before sending a kill signal to
them. Set to ``None`` to never send a kill signal.
Default: ``0.2``
:type terminate_timeout: float or None
"""
if (
self._runner.global_config.no_install
Expand Down
55 changes: 54 additions & 1 deletion tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,10 @@ def test_run_external_not_a_virtualenv(self):
session.run(sys.executable, "--version")

run.assert_called_once_with(
(sys.executable, "--version"), external=True, env=mock.ANY, paths=None
(sys.executable, "--version"),
external=True,
env=mock.ANY,
paths=None,
)

def test_run_external_condaenv(self):
Expand Down Expand Up @@ -344,6 +347,56 @@ def test_run_always_install_only(self, caplog):

assert session.run_always(operator.add, 23, 19) == 42

@pytest.mark.parametrize(
(
"interrupt_timeout_setting",
"terminate_timeout_setting",
"interrupt_timeout_expected",
"terminate_timeout_expected",
),
[
("default", "default", 0.3, 0.2),
(None, None, None, None),
(0, 0, 0, 0),
(1, 2, 1, 2),
],
)
def test_run_shutdown_process_timeouts(
self,
interrupt_timeout_setting,
terminate_timeout_setting,
interrupt_timeout_expected,
terminate_timeout_expected,
):
session, runner = self.make_session_and_runner()

runner.venv = nox.virtualenv.ProcessEnv()

subp_popen_instance = mock.Mock()
subp_popen_instance.communicate.side_effect = KeyboardInterrupt()
with mock.patch(
"nox.popen.shutdown_process", autospec=True
) as shutdown_process, mock.patch(
"nox.popen.subprocess.Popen",
new=mock.Mock(return_value=subp_popen_instance),
):
shutdown_process.return_value = ("", "")

timeout_kwargs = {}
if interrupt_timeout_setting != "default":
timeout_kwargs["interrupt_timeout"] = interrupt_timeout_setting
if terminate_timeout_setting != "default":
timeout_kwargs["terminate_timeout"] = terminate_timeout_setting

with pytest.raises(KeyboardInterrupt):
session.run(sys.executable, "--version", **timeout_kwargs)

shutdown_process.assert_called_once_with(
proc=mock.ANY,
interrupt_timeout=interrupt_timeout_expected,
terminate_timeout=terminate_timeout_expected,
)

@pytest.mark.parametrize(
("no_install", "reused", "run_called"),
[
Expand Down

0 comments on commit bcee2dc

Please sign in to comment.