Skip to content

Commit

Permalink
Test that child processes are killed as well.
Browse files Browse the repository at this point in the history
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
  • Loading branch information
stuhood committed Nov 19, 2021
1 parent 69429a1 commit e6048e4
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_ctrl_c(pantsd: bool) -> None:
dest = os.path.join(workdir, "dest.log")

# Start a pantsd run that will wait forever, then kill the pantsd client.
client_handle, _, _ = launch_waiter(
client_handle, _, _, _ = launch_waiter(
workdir=workdir, config=workunit_logger_config(dest, pantsd=pantsd)
)
client_pid = client_handle.process.pid
Expand Down
32 changes: 26 additions & 6 deletions testprojects/src/python/coordinated_runs/waiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,35 @@
import os
import sys
import time
from multiprocessing import Process

waiting_for_file = sys.argv[1]
pid_file = sys.argv[2]
child_pid_file = sys.argv[3]
attempts = 60


def run_child():
while True:
print("Child running...")
time.sleep(1)


child = Process(target=run_child, daemon=True)
child.start()

with open(child_pid_file, "w") as pf:
pf.write(str(child.pid))

with open(pid_file, "w") as pf:
pf.write(str(os.getpid()))
while not os.path.isfile(waiting_for_file):
if attempts <= 0:
raise Exception("File was never written.")
attempts -= 1
sys.stderr.write("Waiting for file {}\n".format(waiting_for_file))
time.sleep(1)

try:
while not os.path.isfile(waiting_for_file):
if attempts <= 0:
raise Exception("File was never written.")
attempts -= 1
sys.stderr.write("Waiting for file {}\n".format(waiting_for_file))
time.sleep(1)
finally:
child.terminate()
34 changes: 9 additions & 25 deletions tests/python/pants_test/pantsd/pantsd_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,26 +498,6 @@ def test_pantsd_invalidation_stale_sources(self):
finally:
rm_rf(test_path)

@unittest.skip("TODO https://github.com/pantsbuild/pants/issues/7654")
def test_pantsd_parse_exception_success(self):
# This test covers the case described in #6426, where a run that is failing fast due to an
# exception can race other completing work. We expect all runs to fail due to the error
# that has been introduced, but none of them should hang.
test_path = "testprojects/3rdparty/this_is_definitely_not_a_valid_directory"
test_build_file = os.path.join(test_path, "BUILD")
invalid_symbol = "this_is_definitely_not_a_valid_symbol"

try:
safe_mkdir(test_path, clean=True)
safe_file_dump(test_build_file, f"{invalid_symbol}()")
for _ in range(3):
with self.pantsd_run_context(success=False) as ctx:
result = ctx.runner(["list", "testprojects::"])
ctx.checker.assert_started()
self.assertIn(invalid_symbol, result.stderr)
finally:
rm_rf(test_path)

def _assert_pantsd_keyboardinterrupt_signal(
self, signum: int, regexps: list[str] | None = None
):
Expand All @@ -527,25 +507,29 @@ def _assert_pantsd_keyboardinterrupt_signal(
:param regexps: Assert that all of these regexps match somewhere in stderr.
"""
with self.pantsd_test_context() as (workdir, config, checker):
client_handle, waiter_process_pid, _ = launch_waiter(workdir=workdir, config=config)
client_handle, waiter_pid, child_pid, _ = launch_waiter(workdir=workdir, config=config)
client_pid = client_handle.process.pid
waiter_process = psutil.Process(waiter_process_pid)
waiter_process = psutil.Process(waiter_pid)
child_process = psutil.Process(waiter_pid)

assert waiter_process.is_running()
assert child_process.is_running()
checker.assert_started()

# This should kill the client, which will cancel the run on the server, which will
# kill the waiting process.
# kill the waiting process and its child.
os.kill(client_pid, signum)
client_run = client_handle.join()
client_run.assert_failure()

for regexp in regexps or []:
self.assertRegex(client_run.stderr, regexp)

# pantsd should still be running, but the waiter process should have been killed.
# pantsd should still be running, but the waiter process and child should have been
# killed.
time.sleep(5)
assert not waiter_process.is_running()
assert not child_process.is_running()
checker.assert_running()

def test_pantsd_sigint(self):
Expand All @@ -560,7 +544,7 @@ def test_sigint_kills_request_waiting_for_lock(self):
config = {"GLOBAL": {"pantsd_timeout_when_multiple_invocations": -1, "level": "debug"}}
with self.pantsd_test_context(extra_config=config) as (workdir, config, checker):
# Run a process that will wait forever.
first_run_handle, _, file_to_create = launch_waiter(workdir=workdir, config=config)
first_run_handle, _, _, file_to_create = launch_waiter(workdir=workdir, config=config)

checker.assert_started()
checker.assert_running()
Expand Down
14 changes: 9 additions & 5 deletions tests/python/pants_test/pantsd/pantsd_integration_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,34 @@ def attempts(

def launch_waiter(
*, workdir: str, config: Mapping | None = None
) -> tuple[PantsJoinHandle, int, str]:
) -> tuple[PantsJoinHandle, int, int, str]:
"""Launch a process that will wait forever for a file to be created.
Returns the pid of the pants client, the pid of the waiting child process, and the file to
create to cause the waiting child to exit.
Returns the pants client handle, the pid of the waiting process, the pid of a child of the
waiting process, and the file to create to cause the waiting child to exit.
"""
file_to_make = os.path.join(workdir, "some_magic_file")
waiter_pid_file = os.path.join(workdir, "pid_file")
child_pid_file = os.path.join(workdir, "child_pid_file")

argv = [
"run",
"testprojects/src/python/coordinated_runs:waiter",
"--",
file_to_make,
waiter_pid_file,
child_pid_file,
]
client_handle = run_pants_with_workdir_without_waiting(argv, workdir=workdir, config=config)
waiter_pid = -1
for _ in attempts("The waiter process should have written its pid."):
waiter_pid_str = maybe_read_file(waiter_pid_file)
if waiter_pid_str:
child_pid_str = maybe_read_file(child_pid_file)
if waiter_pid_str and child_pid_str:
waiter_pid = int(waiter_pid_str)
child_pid = int(child_pid_str)
break
return client_handle, waiter_pid, file_to_make
return client_handle, waiter_pid, child_pid, file_to_make


class PantsDaemonMonitor(ProcessManager):
Expand Down

0 comments on commit e6048e4

Please sign in to comment.