Fix bug in test executor where manual aborts were not letting the test clean up properly.

openhtf/core/test_descriptor.py

@@ -258,6 +258,8 @@ def handle_sig_int(cls, signalnum: Optional[int], handler: Any) -> None:
       test.abort_from_sig_int()
     if not cls.HANDLED_SIGINT_ONCE:
       cls.HANDLED_SIGINT_ONCE = True
+      # Re-raise the KeyboardInterrupt in the main thread so that any other
+      # handlers aren't oblivious to this having happened.
       raise KeyboardInterrupt
     # Otherwise, does not raise KeyboardInterrupt to ensure that the tests are
     # cleaned up.
@@ -340,7 +342,15 @@ def trigger_phase(test):
     except KeyboardInterrupt:
       # The SIGINT handler only raises the KeyboardInterrupt once, so only retry
       # that once.
+      _LOG.info(
+          'Waiting for clean interrupted exit from test: %s',
+          self.descriptor.code_info.name,
+      )
       self._executor.wait()
+      _LOG.info(
+          'Clean interrupted exit from test: %s',
+          self.descriptor.code_info.name,
+      )
       raise
     finally:
       try:

openhtf/core/test_executor.py

@@ -110,6 +110,7 @@ def __init__(self, test_descriptor: 'test_descriptor.TestDescriptor',
     self._last_execution_unit: str = None
     self._abort = threading.Event()
     self._full_abort = threading.Event()
+    self._execution_finished = threading.Event()
     # This is a reentrant lock so that the teardown logic that prevents aborts
     # affects nested sequences.
     self._teardown_phases_lock = threading.RLock()
@@ -165,15 +166,13 @@ def abort(self) -> None:
   def finalize(self) -> test_state.TestState:
     """Finalize test execution and output resulting record to callbacks.
 
-    Should only be called once at the conclusion of a test run, and will raise
-    an exception if end_time_millis is already set.
+    Should only be called once at the conclusion of a test run.
 
     Returns:
       Finalized TestState.  It must not be modified after this call.
 
     Raises:
-      TestStopError: test
-      TestAlreadyFinalized if end_time_millis already set.
+      TestStopError: If the test is already stopped or never ran.
     """
     if not self.test_state:
       raise TestStopError('Test Stopped.')
@@ -193,10 +192,16 @@ def wait(self) -> None:
         threading.TIMEOUT_MAX,
         31557600,  # Seconds in a year.
     )
-    self.join(timeout)
+    # This function is expected to be called twice in the case of a SIGINT,
+    # and in Python 3.12 the second call would always return immediately,
+    # preventing a clean exit (see `execute` in test_descriptor.py). Instead,
+    # we wait on an Event that we control.
+    self._execution_finished.wait(timeout)
+    self.join()
 
   def _thread_proc(self) -> None:
     """Handles one whole test from start to finish."""
+    self._execution_finished.clear()
     try:
       # Top level steps required to run a single iteration of the Test.
       self.test_state = test_state.TestState(self._test_descriptor, self.uid,
@@ -228,6 +233,7 @@ def _thread_proc(self) -> None:
       raise
     finally:
       self._execute_test_teardown()
+      self._execution_finished.set()
 
   def _initialize_plugs(
       self,