Rework ui_dispatch tests to avoid need for Qt (#1792)

Since #1788, we have only one test module that makes use of the Qt event
loop. That test module contains tests for the behaviour of handlers that
use `dispatch='ui'` mechanism to redispatch off-thread notifications to
the ui thread.

This PR reworks that test module, with some significant collateral
damage along the way.

In detail:

- reworks that test module (`test_ui_notifiers`) to avoid the need for
the Qt event loop; instead, it tests against a `ui_handler` based on
asyncio, which redispatches to the running asyncio event loop
- adds a `get_ui_handler` counterpart to `set_ui_handler`, and exposes
both functions in `traits.api`
- adds type hints for `get_ui_handler` and `set_ui_handler`
- removes two public module globals from `trait_notifiers`: `ui_handler`
has been made private, while `ui_thread` is removed altogether
- fixes a bug where ui dispatch didn't do the right thing (PR #1740 was
incomplete; this bug should have been caught at review time on that PR)
- makes another couple of drive-by cleanups, removing a very old check
for `threading.local()` being a dict (which it hasn't been in living
memory), and tidying up some uses of thread identity.

traits/api.py

@@ -34,6 +34,8 @@
 )
 
 from .trait_notifiers import (
+    get_ui_handler,
+    set_ui_handler,
     push_exception_handler,
     pop_exception_handler,
     TraitChangeNotifyWrapper,

traits/api.pyi

@@ -178,3 +178,8 @@ from .trait_numeric import (
     ArrayOrNone as ArrayOrNone,
     CArray as CArray,
 )
+
+from .trait_notifiers import (
+    get_ui_handler as get_ui_handler,
+    set_ui_handler as set_ui_handler,
+)

traits/tests/test_new_notifiers.py

@@ -74,8 +74,8 @@ def test_notification_on_separate_thread(self):
         receiver = Receiver()
 
         def on_foo_notifications(obj, name, old, new):
-            thread_id = threading.current_thread().ident
-            event = (thread_id, obj, name, old, new)
+            thread = threading.current_thread()
+            event = (thread, obj, name, old, new)
             receiver.notifications.append(event)
 
         obj = Foo()
@@ -97,5 +97,5 @@ def on_foo_notifications(obj, name, old, new):
         self.assertEqual(len(notifications), 1)
         self.assertEqual(notifications[0][1:], (obj, "foo", 0, 3))
 
-        this_thread_id = threading.current_thread().ident
-        self.assertNotEqual(this_thread_id, notifications[0][0])
+        this_thread = threading.current_thread()
+        self.assertNotEqual(this_thread, notifications[0][0])

traits/tests/test_ui_notifiers.py

@@ -18,33 +18,22 @@
 notification really occurs on the UI thread.
 
 At present, `dispatch='ui'` and `dispatch='fast_ui'` have the same effect.
-
 """
 
+import asyncio
+import contextlib
 import threading
-import time
 import unittest
 
-# Preamble: Try importing Qt, and set QT_FOUND to True on success.
-try:
-    from pyface.util.guisupport import get_app_qt4
-
-    # This import is necessary to set the `ui_handler` global variable in
-    # `traits.trait_notifiers`, which is responsible for dispatching the events
-    # to the UI thread.
-    from traitsui.qt import toolkit  # noqa: F401
-
-    qt4_app = get_app_qt4()
-
-except Exception:
-    QT_FOUND = False
-
-else:
-    QT_FOUND = True
-
-
+from traits.api import (
+    Callable,
+    Float,
+    get_ui_handler,
+    HasTraits,
+    on_trait_change,
+    set_ui_handler,
+)
 from traits import trait_notifiers
-from traits.api import Callable, Float, HasTraits, on_trait_change
 
 
 class CalledAsMethod(HasTraits):
@@ -61,88 +50,163 @@ def on_foo_change(self, obj, name, old, new):
         self.callback(obj, name, old, new)
 
 
-class BaseTestUINotifiers(object):
-    """ Tests for dynamic notifiers with `dispatch='ui'`.
+def asyncio_ui_handler(event_loop):
+    """
+    Create a UI handler that dispatches to the asyncio event loop.
     """
 
+    def ui_handler(handler, *args, **kwargs):
+        """
+        UI handler that dispatches to the asyncio event loop.
+        """
+        event_loop.call_soon_threadsafe(lambda: handler(*args, **kwargs))
+
+    return ui_handler
+
+
+@contextlib.contextmanager
+def use_asyncio_ui_handler(event_loop):
+    """
+    Context manager that temporarily sets the UI handler to an asyncio handler.
+    """
+    old_handler = get_ui_handler()
+    set_ui_handler(asyncio_ui_handler(event_loop))
+    try:
+        yield
+    finally:
+        set_ui_handler(old_handler)
+
+
+@contextlib.contextmanager
+def clear_ui_handler():
+    """
+    Context manager that temporarily clears the UI handler.
+    """
+    old_handler = get_ui_handler()
+    set_ui_handler(None)
+    try:
+        yield
+    finally:
+        set_ui_handler(old_handler)
+
+
+class BaseTestUINotifiers(object):
+    """Tests for dynamic notifiers with `dispatch='ui'`."""
+
     #### 'TestCase' protocol ##################################################
 
     def setUp(self):
         self.notifications = []
+        self.exceptions = []
+        self.done = asyncio.Event()
+        self.obj = self.obj_factory()
+
+    def enterContext(self, cm):
+        # Backport of Python 3.11's TestCase.enterContext method.
+        result = type(cm).__enter__(cm)
+        self.addCleanup(type(cm).__exit__, cm, None, None, None)
+        return result
 
     #### 'TestUINotifiers' protocol ###########################################
 
-    def flush_event_loop(self):
-        """ Post and process the Qt events. """
-        qt4_app.sendPostedEvents()
-        qt4_app.processEvents()
+    def modify_obj(self):
+        trait_notifiers.push_exception_handler(
+            lambda *args: None, reraise_exceptions=True
+        )
+        try:
+            self.obj.foo = 3
+        except Exception as e:
+            self.exceptions.append(e)
+        finally:
+            trait_notifiers.pop_exception_handler()
 
     def on_foo_notifications(self, obj, name, old, new):
-        thread_id = threading.current_thread().ident
-        event = (thread_id, (obj, name, old, new))
+        event = (threading.current_thread(), (obj, name, old, new))
         self.notifications.append(event)
+        self.done.set()
 
     #### Tests ################################################################
 
-    @unittest.skipIf(
-        not QT_FOUND, "Qt event loop not found, UI dispatch not possible."
-    )
-    def test_notification_from_main_thread(self):
-
-        obj = self.obj_factory()
+    def test_notification_from_main_thread_with_no_ui_handler(self):
+        # Given
+        self.enterContext(clear_ui_handler())
 
-        obj.foo = 3
-        self.flush_event_loop()
+        # When we set obj.foo to 3 on the main thread.
+        self.modify_obj()
 
-        notifications = self.notifications
-        self.assertEqual(len(notifications), 1)
+        # Then the notification is processed synchronously on the main thread.
+        self.assertEqual(
+            self.notifications,
+            [(threading.main_thread(), (self.obj, "foo", 0, 3))],
+        )
 
-        thread_id, event = notifications[0]
-        self.assertEqual(event, (obj, "foo", 0, 3))
+    def test_notification_from_main_thread_with_registered_ui_handler(self):
+        # Given
+        self.enterContext(use_asyncio_ui_handler(asyncio.get_event_loop()))
 
-        ui_thread = trait_notifiers.ui_thread
-        self.assertEqual(thread_id, ui_thread)
+        # When we set obj.foo to 3 on the main thread.
+        self.modify_obj()
 
-    @unittest.skipIf(
-        not QT_FOUND, "Qt event loop not found, UI dispatch not possible."
-    )
-    def test_notification_from_separate_thread(self):
+        # Then the notification is processed synchronously on the main thread.
+        self.assertEqual(
+            self.notifications,
+            [(threading.main_thread(), (self.obj, "foo", 0, 3))],
+        )
 
-        obj = self.obj_factory()
+    def test_notification_from_separate_thread_failure_case(self):
+        # Given no registered ui handler
+        self.enterContext(clear_ui_handler())
 
-        # Set obj.foo to 3 on a separate thread.
-        def set_foo_to_3(obj):
-            obj.foo = 3
+        # When we set obj.foo to 3 on a separate thread.
+        background_thread = threading.Thread(target=self.modify_obj)
+        background_thread.start()
+        self.addCleanup(background_thread.join)
 
-        threading.Thread(target=set_foo_to_3, args=(obj,)).start()
+        # Then no notification is processed ...
+        self.assertEqual(self.notifications, [])
 
-        # Wait for a while to make sure the function has finished.
-        time.sleep(0.1)
+        # ... and an error was raised
+        self.assertEqual(len(self.exceptions), 1)
+        self.assertIsInstance(self.exceptions[0], RuntimeError)
+        self.assertIn("no UI handler registered", str(self.exceptions[0]))
 
-        self.flush_event_loop()
+        # ... but the attribute change was still applied.
+        self.assertEqual(self.obj.foo, 3)
 
-        notifications = self.notifications
-        self.assertEqual(len(notifications), 1)
+    async def test_notification_from_separate_thread(self):
+        # Given an asyncio ui handler
+        self.enterContext(use_asyncio_ui_handler(asyncio.get_event_loop()))
 
-        thread_id, event = notifications[0]
-        self.assertEqual(event, (obj, "foo", 0, 3))
+        # When we set obj.foo to 3 on a separate thread.
+        background_thread = threading.Thread(target=self.modify_obj)
+        background_thread.start()
+        self.addCleanup(background_thread.join)
 
-        ui_thread = trait_notifiers.ui_thread
-        self.assertEqual(thread_id, ui_thread)
+        # Then the notification will eventually be processed on the main
+        # thread.
+        await asyncio.wait_for(self.done.wait(), timeout=5.0)
+        self.assertEqual(
+            self.notifications,
+            [(threading.main_thread(), (self.obj, "foo", 0, 3))],
+        )
+        self.assertEqual(self.obj.foo, 3)
 
 
-class TestMethodUINotifiers(BaseTestUINotifiers, unittest.TestCase):
-    """ Tests for dynamic notifiers with `dispatch='ui'` set by method call.
-    """
+class TestMethodUINotifiers(
+    BaseTestUINotifiers, unittest.IsolatedAsyncioTestCase
+):
+    """Tests for dynamic notifiers with `dispatch='ui'` set by method call."""
 
     def obj_factory(self):
         obj = CalledAsMethod()
         obj.on_trait_change(self.on_foo_notifications, "foo", dispatch="ui")
         return obj
 
 
-class TestDecoratorUINotifiers(BaseTestUINotifiers, unittest.TestCase):
-    """ Tests for dynamic notifiers with `dispatch='ui'` set by decorator. """
+class TestDecoratorUINotifiers(
+    BaseTestUINotifiers, unittest.IsolatedAsyncioTestCase
+):
+    """Tests for dynamic notifiers with `dispatch='ui'` set by decorator."""
 
     def obj_factory(self):
         return CalledAsDecorator(callback=self.on_foo_notifications)

traits/trait_notifiers.py

@@ -27,27 +27,33 @@
 
 # Global Data
 
-# The thread ID for the user interface thread
-ui_thread = -1
+# The currently active handler for notifications that must be run on the UI
+# thread, or None if no handler has been set.
+_ui_handler = None
 
-# The handler for notifications that must be run on the UI thread
-ui_handler = None
+
+def get_ui_handler():
+    """
+    Return the current user interface thread handler.
+    """
+    return _ui_handler
 
 
 def set_ui_handler(handler):
     """ Sets up the user interface thread handler.
     """
-    global ui_handler, ui_thread
+    global _ui_handler
 
-    ui_handler = handler
-    ui_thread = threading.current_thread().ident
+    _ui_handler = handler
 
 
 def ui_dispatch(handler, *args, **kw):
     if threading.current_thread() == threading.main_thread():
         handler(*args, **kw)
+    elif _ui_handler is None:
+        raise RuntimeError("no UI handler registered for dispatch='ui'")
     else:
-        ui_handler(handler, *args, **kw)
+        _ui_handler(handler, *args, **kw)
 
 
 class NotificationExceptionHandlerState(object):
@@ -153,11 +159,7 @@ def _get_handlers(self):
             thread.
         """
         thread_local = self.thread_local
-        if isinstance(thread_local, dict):
-            id = threading.current_thread().ident
-            handlers = thread_local.get(id)
-        else:
-            handlers = getattr(thread_local, "handlers", None)
+        handlers = getattr(thread_local, "handlers", None)
 
         if handlers is None:
             if self.main_thread is not None:
@@ -167,10 +169,7 @@ def _get_handlers(self):
                     self._log_exception, False, False
                 )
             handlers = [handler]
-            if isinstance(thread_local, dict):
-                thread_local[id] = handlers
-            else:
-                thread_local.handlers = handlers
+            thread_local.handlers = handlers
 
         return handlers
 
@@ -615,10 +614,12 @@ class FastUITraitChangeNotifyWrapper(TraitChangeNotifyWrapper):
     """
 
     def dispatch(self, handler, *args):
-        if threading.current_thread().ident == ui_thread:
+        if threading.current_thread() == threading.main_thread():
             handler(*args)
+        elif _ui_handler is None:
+            raise RuntimeError("no UI handler registered for dispatch='ui'")
         else:
-            ui_handler(handler, *args)
+            _ui_handler(handler, *args)
 
 
 class NewTraitChangeNotifyWrapper(TraitChangeNotifyWrapper):

traits/traits_notifiers.pyi

@@ -0,0 +1,8 @@
+from typing import Callable
+
+_UI_Handler = Callable[..., None] | None
+
+
+def get_ui_handler() -> _UI_Handler: ...
+
+def set_ui_handler(handler: _UI_Handler) -> None: ...