fix(all): refactor killing processes and waiting

* Use `psutils.pid_exists()` instead of iterating processes for no reason.
* Drop use of recursion which is completely unnecessarily complex. Use loop.
* Remove checking of `returncode` right away after process starts, it's too soon.
* Lower `PROCESS_TIMEOUT_SEC` to 5 seconds to avoid needless delays.

Signed-off-by: Jakub Sokołowski <jakub@status.im>

test/e2e/configs/timeouts.py

@@ -2,6 +2,6 @@
 
 UI_LOAD_TIMEOUT_SEC = 5
 UI_LOAD_TIMEOUT_MSEC = UI_LOAD_TIMEOUT_SEC * 1000
-PROCESS_TIMEOUT_SEC = 10
+PROCESS_TIMEOUT_SEC = 5
 APP_LOAD_TIMEOUT_MSEC = 60000
 MESSAGING_TIMEOUT_SEC = 60

test/e2e/driver/aut.py

@@ -81,8 +81,9 @@ class AUT:
 
     def _kill_process(self):
         if self.pid is None:
-            raise Exception('No process to kill, no PID available.')
-        local_system.kill_process(self.pid, verify=True)
+            LOG.warning('No PID availale for AUT.')
+            return
+        local_system.kill_process_with_retries(self.pid)
         self.pid = None
 
     @allure.step('Close application')

test/e2e/driver/server.py

@@ -36,9 +36,10 @@ class SquishServer:
 
     @classmethod
     def stop(cls):
-        if cls.pid is not None:
-            local_system.kill_process(cls.pid, verify=True)
-            cls.pid = None
+        if cls.pid is None:
+            return
+        local_system.kill_process(cls.pid)
+        cls.pid = None
         cls.port = None
 
     @classmethod

test/e2e/fixtures/aut.py

@@ -10,6 +10,7 @@ from scripts.utils import system_path
 from scripts.utils.system_path import SystemPath
 
 
+
 @pytest.fixture
 def options(request):
     if hasattr(request, 'param'):

test/e2e/scripts/utils/local_system.py

@@ -11,7 +11,7 @@ import psutil
 import configs
 from configs.system import IS_WIN
 
-_logger = logging.getLogger(__name__)
+LOG = logging.getLogger(__name__)
 
 
 def find_process_by_port(port: int) -> typing.List[int]:
@@ -31,79 +31,63 @@ def find_free_port(start: int, step: int):
         start+=step
     return start
 
-
-def wait_for_close(pid: int, timeout_sec: int = configs.timeouts.PROCESS_TIMEOUT_SEC):
-    started_at = time.monotonic()
-    while True:
-        for proc in psutil.process_iter():
-            try:
-                if proc.pid == pid:
-                    return True
-            except (psutil.NoSuchProcess, psutil.AccessDenied, psutil.ZombieProcess):
-                pass
-        time.sleep(1)
-        if time.monotonic() - started_at > timeout_sec:
-            raise RuntimeError(f'Process with PID: {pid} not closed')
-        else:
-            break
-
-
 @allure.step('Kill process')
-def kill_process(pid, verify: bool = False, timeout_sec: int = configs.timeouts.PROCESS_TIMEOUT_SEC, attempt: int = 2):
+def kill_process(pid, sig: signal.Signals = signal.SIGKILL):
+    LOG.debug('Sending %s to %d process', sig.name, pid)
     try:
-        os.kill(pid, signal.SIGILL if IS_WIN else signal.SIGKILL)
+        os.kill(pid, sig)
     except ProcessLookupError as err:
-        _logger.debug(err)
-    if verify:
-        try:
-            wait_for_close(pid, timeout_sec)
-        except RuntimeError as err:
-            if attempt:
-                kill_process(pid, verify, timeout_sec, attempt - 1)
-            else:
-                raise err
+        LOG.error('Failed to find process %d: %s', pid, err)
+        raise err
 
+@allure.step('Kill process with retries')
+def kill_process_with_retries(pid, sig: signal.Signals = signal.SIGTERM, attempts: int = 3):
+    LOG.debug('Killing process: %d', pid)
+    try:
+        p = psutil.Process(pid)
+    except psutil.NoSuchProcess:
+        LOG.warning('Process %d already gone.', pid)
+        return
+
+    p.terminate()
+
+    while attempts > 0:
+        attempts -= 1
+        try:
+            LOG.warning('Waiting for process to exit: %d', pid)
+            p.wait()
+        except TimeoutError as err:
+            p.kill()
+        else:
+            return
+
+    raise RuntimeError('Failed to kill proicess: %d' % pid)
 
 @allure.step('System execute command')
 def execute(
         command: list,
-        shell=False if IS_WIN else True,
-        stderr=subprocess.PIPE,
-        stdout=subprocess.PIPE,
-        check=False
+        stderr=subprocess.STDOUT,
+        stdout=subprocess.STDOUT,
+        shell=False,
 ):
-    def _is_process_exists(_process) -> bool:
-        return _process.poll() is None
-
-    def _wait_for_execution(_process):
-        while _is_process_exists(_process):
-            time.sleep(1)
-
-    def _get_output(_process):
-        _wait_for_execution(_process)
-        return _process.communicate()
-
-    command = " ".join(str(atr) for atr in command)
-    _logger.info(f'Execute: {command}')
+    LOG.info('Executing: %s', command)
     process = subprocess.Popen(command, shell=shell, stderr=stderr, stdout=stdout)
-    if check and process.returncode != 0:
-        stdout, stderr = _get_output(process)
-        raise RuntimeError(stderr)
     return process.pid
 
-
 @allure.step('System run command')
 def run(
         command: list,
-        shell=False if IS_WIN else True,
-        stderr=subprocess.PIPE,
-        stdout=subprocess.PIPE,
-        timeout_sec=configs.timeouts.PROCESS_TIMEOUT_SEC,
-        check=True
+        stderr=subprocess.STDOUT,
+        stdout=subprocess.STDOUT,
+        shell=False,
+        timeout_sec=configs.timeouts.PROCESS_TIMEOUT_SEC
 ):
-    command = " ".join(str(atr) for atr in command)
-    _logger.info(f'Execute: {command}')
-    process = subprocess.run(command, shell=shell, stderr=stderr, stdout=stdout, timeout=timeout_sec)
-    if check and process.returncode != 0:
-        raise subprocess.CalledProcessError(process.returncode, command, process.stdout, process.stderr)
-    _logger.debug(f'stdout: {process.stdout}')
+    LOG.info('Running: %s', command)
+    process = subprocess.run(
+        command,
+        shell=shell,
+        stderr=stderr,
+        stdout=stdout,
+        timeout=timeout_sec,
+        check=True
+    )

test/e2e/tests/settings/test_settings_sign_out_and_quit.py

@@ -1,9 +1,8 @@
 import allure
 import pytest
+import psutil
 from allure_commons._allure import step
-
 from gui.main_window import MainWindow
-from scripts.utils.local_system import wait_for_close
 
 
 @allure.testcase('https://ethstatus.testrail.net/index.php?/cases/view/703010', 'Settings - Sign out & Quit')
@@ -17,4 +16,4 @@ def test_sign_out_and_quit(aut, main_screen: MainWindow):
         sign_out_screen.sign_out_and_quit()
 
     with step('Check that app was closed'):
-        wait_for_close(aut.pid)
+        psutil.Process(aut.pid).wait()