Skip to content

Commit 4f77883

Browse files
gopiotrgchwier
authored andcommitted
scripts: twister: add timeout for pytest process
Add protection timeout for pytest subprocess, to avoid situation of suspending whole Twister in case of internal pytest test problem. Co-authored-by: Grzegorz Chwierut <[email protected]> Signed-off-by: Piotr Golyzniak <[email protected]>
1 parent ab5b48b commit 4f77883

File tree

3 files changed

+59
-38
lines changed

3 files changed

+59
-38
lines changed

scripts/pylib/twister/twisterlib/handlers.py

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,26 @@
4242
SUPPORTED_SIMS = ["mdb-nsim", "nsim", "renode", "qemu", "tsim", "armfvp", "xt-sim", "native"]
4343

4444

45+
def terminate_process(proc):
46+
"""
47+
encapsulate terminate functionality so we do it consistently where ever
48+
we might want to terminate the proc. We need try_kill_process_by_pid
49+
because of both how newer ninja (1.6.0 or greater) and .NET / renode
50+
work. Newer ninja's don't seem to pass SIGTERM down to the children
51+
so we need to use try_kill_process_by_pid.
52+
"""
53+
54+
for child in psutil.Process(proc.pid).children(recursive=True):
55+
try:
56+
os.kill(child.pid, signal.SIGTERM)
57+
except ProcessLookupError:
58+
pass
59+
proc.terminate()
60+
# sleep for a while before attempting to kill
61+
time.sleep(0.5)
62+
proc.kill()
63+
64+
4565
class Handler:
4666
def __init__(self, instance, type_str="build"):
4767
"""Constructor
@@ -82,20 +102,7 @@ def record(self, harness):
82102
cw.writerow(instance)
83103

84104
def terminate(self, proc):
85-
# encapsulate terminate functionality so we do it consistently where ever
86-
# we might want to terminate the proc. We need try_kill_process_by_pid
87-
# because of both how newer ninja (1.6.0 or greater) and .NET / renode
88-
# work. Newer ninja's don't seem to pass SIGTERM down to the children
89-
# so we need to use try_kill_process_by_pid.
90-
for child in psutil.Process(proc.pid).children(recursive=True):
91-
try:
92-
os.kill(child.pid, signal.SIGTERM)
93-
except ProcessLookupError:
94-
pass
95-
proc.terminate()
96-
# sleep for a while before attempting to kill
97-
time.sleep(0.5)
98-
proc.kill()
105+
terminate_process(proc)
99106
self.terminated = True
100107

101108
def _verify_ztest_suite_name(self, harness_state, detected_suite_names, handler_time):

scripts/pylib/twister/twisterlib/harness.py

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
from collections import OrderedDict
1010
import xml.etree.ElementTree as ET
1111
import logging
12+
import threading
1213
import time
1314

1415
from twisterlib.environment import ZEPHYR_BASE, PYTEST_PLUGIN_INSTALLED
16+
from twisterlib.handlers import terminate_process
1517

1618

1719
logger = logging.getLogger('twister')
@@ -229,13 +231,13 @@ def configure(self, instance):
229231
self.report_file = os.path.join(self.running_dir, 'report.xml')
230232
self.reserved_serial = None
231233

232-
def pytest_run(self):
234+
def pytest_run(self, timeout):
233235
try:
234236
cmd = self.generate_command()
235237
if not cmd:
236238
logger.error('Pytest command not generated, check logs')
237239
return
238-
self.run_command(cmd)
240+
self.run_command(cmd, timeout)
239241
except PytestHarnessException as pytest_exception:
240242
logger.error(str(pytest_exception))
241243
finally:
@@ -314,33 +316,36 @@ def _generate_parameters_for_hardware(self, handler):
314316

315317
return command
316318

317-
def run_command(self, cmd):
319+
def run_command(self, cmd, timeout):
318320
cmd, env = self._update_command_with_env_dependencies(cmd)
319-
320321
with subprocess.Popen(cmd,
321322
stdout=subprocess.PIPE,
322323
stderr=subprocess.STDOUT,
323324
env=env) as proc:
324325
try:
325-
while proc.stdout.readable() and proc.poll() is None:
326-
line = proc.stdout.readline().decode().strip()
327-
if not line:
328-
continue
329-
logger.debug("PYTEST: %s", line)
330-
proc.communicate()
331-
tree = ET.parse(self.report_file)
332-
root = tree.getroot()
333-
for child in root:
334-
if child.tag == 'testsuite':
335-
if child.attrib['failures'] != '0':
336-
self.state = "failed"
337-
elif child.attrib['skipped'] != '0':
338-
self.state = "skipped"
339-
elif child.attrib['errors'] != '0':
340-
self.state = "error"
341-
else:
342-
self.state = "passed"
343-
self.instance.execution_time = float(child.attrib['time'])
326+
reader_t = threading.Thread(target=self._output_reader, args=(proc,), daemon=True)
327+
reader_t.start()
328+
reader_t.join(timeout)
329+
if reader_t.is_alive():
330+
terminate_process(proc)
331+
logger.warning('Timeout has occurred.')
332+
self.state = 'failed'
333+
proc.wait(timeout)
334+
335+
if self.state != 'failed':
336+
tree = ET.parse(self.report_file)
337+
root = tree.getroot()
338+
for child in root:
339+
if child.tag == 'testsuite':
340+
if child.attrib['failures'] != '0':
341+
self.state = "failed"
342+
elif child.attrib['skipped'] != '0':
343+
self.state = "skipped"
344+
elif child.attrib['errors'] != '0':
345+
self.state = "error"
346+
else:
347+
self.state = "passed"
348+
self.instance.execution_time = float(child.attrib['time'])
344349
except subprocess.TimeoutExpired:
345350
proc.kill()
346351
self.state = "failed"
@@ -383,6 +388,15 @@ def _update_command_with_env_dependencies(cmd):
383388

384389
return cmd, env
385390

391+
@staticmethod
392+
def _output_reader(proc):
393+
while proc.stdout.readable() and proc.poll() is None:
394+
line = proc.stdout.readline().decode().strip()
395+
if not line:
396+
continue
397+
logger.debug('PYTEST: %s', line)
398+
proc.communicate()
399+
386400
def _apply_instance_status(self):
387401
if self.state:
388402
self.instance.status = self.state

scripts/pylib/twister/twisterlib/runner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1031,7 +1031,7 @@ def run(self):
10311031
harness = HarnessImporter.get_harness(instance.testsuite.harness.capitalize())
10321032
harness.configure(instance)
10331033
if isinstance(harness, Pytest):
1034-
harness.pytest_run()
1034+
harness.pytest_run(instance.handler.timeout)
10351035
else:
10361036
instance.handler.handle(harness)
10371037

0 commit comments

Comments
 (0)