Skip to content

Commit

Permalink
tests: Fix broken SIGINT test
Browse files Browse the repository at this point in the history
The old test would send the interrupt signal before the script had
a chance to enter the integration loop, and thus the custom signal
handler was never tested. The new version waits for the subprocess
to write it's ready for the interrupt signal, and the traceback is
checked to confirm the signal was re-raised by the script interface.
  • Loading branch information
jngrad committed Apr 24, 2023
1 parent 62069c7 commit 752cb97
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 13 deletions.
20 changes: 15 additions & 5 deletions testsuite/python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
Configure a python test case.
Flag arguments: none.
Flag arguments:
* ``NO_MPI``: run test case without MPI, i.e. in singleton mode. Only use
this option when relevant to the test, e.g. to check signal propagation
(MPI vendors introduce their own implementation-defined signal handlers).
Single-value arguments:
Expand All @@ -50,7 +54,7 @@
#]=======================================================================]
function(python_test)
cmake_parse_arguments(TEST "" "FILE;MAX_NUM_PROC;GPU_SLOTS;SUFFIX"
cmake_parse_arguments(TEST "NO_MPI" "FILE;MAX_NUM_PROC;GPU_SLOTS;SUFFIX"
"DEPENDS;DEPENDENCIES;LABELS;ARGUMENTS" ${ARGN})
get_filename_component(TEST_NAME ${TEST_FILE} NAME_WE)
set(TEST_FILE_CONFIGURED "${CMAKE_CURRENT_BINARY_DIR}/${TEST_NAME}.py")
Expand All @@ -65,7 +69,13 @@ function(python_test)
set(TEST_FILE ${TEST_FILE_CONFIGURED})

if(NOT DEFINED TEST_MAX_NUM_PROC)
set(TEST_MAX_NUM_PROC 4)
if(${TEST_NO_MPI})
set(TEST_MAX_NUM_PROC 1)
else()
set(TEST_MAX_NUM_PROC 4)
endif()
elseif(${TEST_NO_MPI} AND NOT ${TEST_MAX_NUM_PROC} EQUAL 1)
message(FATAL_ERROR "NO_MPI and MAX_NUM_PROC are mutually exclusive")
endif()
if(NOT DEFINED TEST_GPU_SLOTS)
set(TEST_GPU_SLOTS 0)
Expand All @@ -77,7 +87,7 @@ function(python_test)
set(TEST_NUM_PROC ${TEST_MAX_NUM_PROC})
endif()

if(EXISTS ${MPIEXEC})
if(EXISTS ${MPIEXEC} AND NOT ${TEST_NO_MPI})
espresso_set_mpiexec_tmpdir(${TEST_NAME})
add_test(
NAME ${TEST_NAME}
Expand Down Expand Up @@ -352,7 +362,7 @@ python_test(FILE lb_momentum_conservation.py MAX_NUM_PROC 1 GPU_SLOTS 1 SUFFIX
1_core)
python_test(FILE p3m_electrostatic_pressure.py MAX_NUM_PROC 2 GPU_SLOTS 1)
python_test(FILE p3m_madelung.py MAX_NUM_PROC 2 GPU_SLOTS 2 LABELS long)
python_test(FILE sigint.py DEPENDENCIES sigint_child.py MAX_NUM_PROC 1)
python_test(FILE sigint.py DEPENDENCIES sigint_child.py NO_MPI)
python_test(FILE lb_density.py MAX_NUM_PROC 1)
python_test(FILE observable_chain.py MAX_NUM_PROC 4)
python_test(FILE mpiio.py MAX_NUM_PROC 4)
Expand Down
50 changes: 43 additions & 7 deletions testsuite/python/sigint.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,55 @@
import time
import sys
import pathlib
import os


class SigintTest(ut.TestCase):

def setUp(self):
script = str(pathlib.Path(__file__).parent / 'sigint_child.py')
self.process = subprocess.Popen([sys.executable, script])
script = str(pathlib.Path(__file__).parent / 'sigint_child.py')

def check_signal_handling(self, process, sig):
# send signal
process.send_signal(sig)
# capture stderr and return code (negative of signum)
stdout, stderr = process.communicate(input=None, timeout=6.)
assert stdout is None
traceback = stderr.decode()
return_code = process.poll()
signum = -return_code
self.assertEqual(signum, sig.value)
if sig == signal.Signals.SIGTERM:
self.assertEqual(traceback, "")
elif sig == signal.Signals.SIGINT:
self.assertIn(" self.integrator.run(", traceback)
self.assertTrue(traceback.endswith(
" in handle_sigint\n signal.raise_signal(signal.Signals.SIGINT)\nKeyboardInterrupt\n"))

def test_signal_handling(self):
self.process.send_signal(signal.Signals.SIGINT)
# Wait for the signal to arrive and one integration step to be finished
time.sleep(1)
self.assertIsNotNone(self.process.poll())
signals = [signal.Signals.SIGINT, signal.Signals.SIGTERM]
processes = []
# open asynchronous processes with non-blocking read access on stderr
for _ in range(len(signals)):
process = subprocess.Popen([sys.executable, self.script],
stderr=subprocess.PIPE)
os.set_blocking(process.stderr.fileno(), False)
processes.append(process)

# wait for the script to reach the integration loop
time.sleep(0.5)
for process, sig in zip(processes, signals):
tick = time.time()
while True:
message = process.stderr.readline().decode()
if message == "start of integration loop\n":
# wait for the script to enter the integrator run method
time.sleep(0.1)
# send signal and check process behavior
self.check_signal_handling(process, sig)
break
tock = time.time()
assert tock - tick < 8., "subprocess timed out"
time.sleep(0.1)


if __name__ == '__main__':
Expand Down
4 changes: 3 additions & 1 deletion testsuite/python/sigint_child.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
import sys
import numpy as np
import espressomd

Expand All @@ -26,5 +27,6 @@
for i in range(100):
system.part.add(pos=np.random.random() * system.box_l)

print("start of integration loop", file=sys.stderr)
while True:
system.integrator.run(1000)
system.integrator.run(100000)

0 comments on commit 752cb97

Please sign in to comment.