test/lib: refactoring

2 files changed, 23 insertions, 23 deletions

diff --git a/test/conftest.py b/test/conftest.py
index 80ffef5..65d8a7e 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -9,7 +9,7 @@ import random
 
 from pytest import fixture
 
-from .lib.process import CmdLine, CmdLineBuilder, Runner
+from .lib.process import CmdLine, CmdLineRunner, Runner
 
 
 class Param:
@@ -158,5 +158,5 @@ def server_and_workers(server, workers):
 @fixture
 def client(process_runner, paths, server_port):
     args = ['--port', server_port]
-    cmd_line = CmdLineBuilder(process_runner, paths.client_binary, *args)
+    cmd_line = CmdLineRunner(process_runner, paths.client_binary, *args)
     return cmd_line
diff --git a/test/lib/process.py b/test/lib/process.py
index f13cb56..bdfe3c3 100644
--- a/test/lib/process.py
+++ b/test/lib/process.py
@@ -23,9 +23,13 @@ class LoggingThread(Thread):
     def __init__(self, process):
         self.process = process
         self.ready_event = Event()
+
         target = lambda pipe: self.consume(pipe)
         super().__init__(target=target, args=[process.stdout])
 
+        self.start()
+        self.ready_event.wait()
+
     def consume(self, pipe):
         for line in pipe:
             line = line.removesuffix('\n')
@@ -34,15 +38,6 @@ class LoggingThread(Thread):
                 logging.info('Process %s is ready', self.process.log_id)
                 self.ready_event.set()
 
-    def __enter__(self):
-        self.start()
-        self.ready_event.wait()
-        return self
-
-    def __exit__(self, *args):
-        self.process.shut_down()
-        self.join()
-
 
 class CmdLine:
     @staticmethod
@@ -83,22 +78,25 @@ class CmdLine:
 class Process(subprocess.Popen):
     def __init__(self, cmd_line):
         self.cmd_line = cmd_line
-
-        cmd_line.log_process_start()
-        self.name = cmd_line.process_name
-
         super().__init__(cmd_line.argv, **_COMMON_ARGS)
         logging.info('Process %s has started', self.log_id)
+        self.logger = LoggingThread(self)
 
     @property
     def log_id(self):
-        return f'{self.pid}/{self.name}'
+        return f'{self.pid}/{self.cmd_line.process_name}'
 
     def __exit__(self, *args):
         try:
             self.shut_down()
-        finally:
-            super().__exit__(*args)
+            self.logger.join()
+        except Exception as e:
+            logging.exception(e)
+        # Postpone closing the pipes until after the logging thread is finished
+        # so that it doesn't attempt to read from closed descriptors.
+        super().__exit__(*args)
+
+    SHUT_DOWN_TIMEOUT_SEC = 3
 
     def shut_down(self):
         ec = self.poll()
@@ -106,15 +104,14 @@ class Process(subprocess.Popen):
             return
         logging.info('Terminating process %s', self.log_id)
         self.terminate()
-        timeout = 3
         try:
-            self.wait(timeout=timeout)
+            self.wait(timeout=Process.SHUT_DOWN_TIMEOUT_SEC)
             return
         except subprocess.TimeoutExpired:
             pass
         logging.info('Process %s failed to terminate in time, killing it', self.log_id)
         self.kill()
-        self.wait(timeout=timeout)
+        self.wait(timeout=Process.SHUT_DOWN_TIMEOUT_SEC)
 
 
 class Runner:
@@ -137,17 +134,20 @@ class Runner:
     def run(self, cmd_line):
         cmd_line = self._wrap(cmd_line)
         cmd_line.log_process_start()
+
         result = subprocess.run(cmd_line.argv, **_COMMON_ARGS)
         return result.returncode, result.stdout
 
     @contextmanager
     def run_async(self, cmd_line):
         cmd_line = self._wrap(cmd_line)
-        with Process(cmd_line) as process, LoggingThread(process):
+        cmd_line.log_process_start()
+
+        with Process(cmd_line) as process:
             yield process
 
 
-class CmdLineBuilder:
+class CmdLineRunner:
     def __init__(self, runner, binary, *args):
         self.runner = runner
         self.binary = binary