[lldb/test] Add events listener helper class to lldbtest

[mib]

Summary

This patch introduces a generic helper class that will listen for
event in a background thread and match it against a source broadcaster.

If the event received matches the source broadcaster, the event is
queued up in a list that the user can access later on.

The motivation behind this is to easily test new kinds of events
(i.e. Swift type-system progress events). However, this patch also
updates TestProgressReporting.py and TestDiagnosticReporting.py
to make use of this new helper class.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[mib]

Reformat

[labath]

lldb/packages/Python/lldbsuite/test/lldbutil.py

1635 ↗

(On Diff #416396)

This contains way too many underscores. An appropriate name for a private function would be _fetch_events.

1640 ↗

(On Diff #416396)

If you're doing an infinite loop, you might as well use a blocking call.

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/lldbutil.py

1624 ↗

(On Diff #416396)

We should make this more generic so that the warning and error events can also use this. The way I imagined this was that the thread would be generic and you would pass it in the broadcaster, the listener and the event type and finally a function callback to have the test do whatever it wants with the event.

[mib]

lldb/packages/Python/lldbsuite/test/lldbutil.py

1624 ↗

(On Diff #416396)

@JDevlieghere I think this could be achieved very easily by assuming the _fetch_events method will always be called and replace the callback argument by a "pointer" to the event_data_extractor function. wdyk ?'

Do you think it's a fair assumption (that we should always call _fetch_events and match the event against the src_broadcaster argument) ?

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/lldbutil.py

1624 ↗

(On Diff #416396)

Yes, that's one way to do it. I'm happy as long as we can use it for both TestDiagnosticReporting.py and TestProgressReporting.py. I was on the fence about the broadcaster and listener being part of the utility, but I think it can work. The diagnostics are slightly different because they listen for two event types, but I guess we can abstract over that by ORing the types.

If we do go that route, I think we might want to make this a TestBase class instead that starts and stops the event thread at the beginning and end of the test. Something like GDBRemoteTestBase and PExpectTest.

[mib]

Address @JDevlieghere comments:

• Move the EventListener class to a separate module and make it inherit from TestBase to start/stop the background listener thread in setUp/tearDown.

[mib]

Update EventListener.tearDown comment

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/eventlistener.py

7

EventListenerTest

20

Just events maybe?

23

Could we avoid the arguments by making these two things properties of the test? I'm trying to avoid the:

TestBase.setUp(self) 
EventListener.setUp(self)

So that tests would have to set:

class TestDiagnosticReporting(EventListenerTest):
  event_mask = lldb.SBDebugger.eBroadcastBitWarning | lldb.SBDebugger.eBroadcastBitError
  event_extractor = lldb.SBDebugger.GetDiagnosticFromEvent

Presumable those two things don't need the debugger and I think we can avoid the self.src_broadcaster too (see below).

24

Would this always be self.dbg.GetBroadcaster()?

[jingham]

lldb/packages/Python/lldbsuite/test/lldbutil.py

1624 ↗

(On Diff #416396)

It might be good to put an error in there if some one tries to do this with process events. That's pretty unlikely to work, since it would fight with the debugger for those events.

[mib]

lldb/packages/Python/lldbsuite/test/eventlistener.py

24

For now, it's always the debugger broadcaster so we can assume that I think.

[mib]

Address @JDevlieghere comments

[mib]

@jingham It's a bit restrictive, but as I stated before, this class will expect to receive events from the debugger broadcaster. If you can think of other broadcasters that could be tested with this class, please let me know so we can think of another solution.

[JDevlieghere]

Nice, thanks for bearing with me. LGTM.

[mib]

Make event_mask and event_data_extractor class attributes to remove EventListenerTestBase.setUp overloading in the derived class tests.

[labath]

I've reverted this patch because it introduces races in the event processing. See inline comment for details.

lldb/packages/Python/lldbsuite/test/eventlistener.py

38–41

This cannot be done in the tearDown method (without extra synchronisation) as this is what guarantees that the events have been processed and can be read from the event array. This is what caused TestDiagnosticReporting.py to flake. If you want to do the shutdown in the tearDown method, then you'll need to introduce some other means of synchronising and ensuring that the event array can be accessed safely and contains all the things it is supposed to contain (e.g. through a new kind of a message).

lldb/test/API/functionalities/diagnostic_reporting/TestDiagnosticReporting.py

15

Why not just queue the events themselves and let the user do whatever it wants with them later?

[mib]

lldb/packages/Python/lldbsuite/test/eventlistener.py

38–41

I knew this was going to bite us eventually :/ Should we get back to the start/stop approach ?

lldb/test/API/functionalities/diagnostic_reporting/TestDiagnosticReporting.py

15

We still need to setup the event_mask to filter out unwanted events from the source broadcaster.

We would have to set that up in the derived class setUp override.

@JDevlieghere do you have any other suggestion ?

[mib]

Came up with a new approach in D122193

lldb/packages/Python/lldbsuite/test/eventlistener.py

+import threading
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class EventListenerTestBase(TestBase):
+
+    """
+    Base class for lldb event listener tests.
+
+    This class will setup and start an event listener for the test to use.
+
+    If the event received matches the source broadcaster, the event is
+    queued up in a list that the user can access later on.
+    """
+    NO_DEBUG_INFO_TESTCASE = True
+
+    eBroadcastBitStopListenerThread = (1 << 0)
+    events = []
+    event_mask = None
+    event_data_extractor = None
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+        self.src_broadcaster = self.dbg.GetBroadcaster()
+        self.broadcaster = lldb.SBBroadcaster('lldb.test.broadcaster')
+        self.listener = lldb.SBListener("lldb.test.listener")
+        self.listener.StartListeningForEvents(self.broadcaster,
+                                              self.eBroadcastBitStopListenerThread)
+
+        self.src_broadcaster.AddListener(self.listener, self.event_mask)
+
+        self.listener_thread = threading.Thread(target=self._fetch_events)
+        self.listener_thread.start()
+
+    def tearDown(self):
+        # Broadcast a eBroadcastBitStopListenerThread` event so the background
+        # thread stops listening to events, then join the background thread.
+        self.broadcaster.BroadcastEventByType(self.eBroadcastBitStopListenerThread)
+        self.listener_thread.join()
+        TestBase.tearDown(self)
+
+    def _fetch_events(self):
+        event = lldb.SBEvent()
+
+        done = False
+        while not done:
+            if self.listener.GetNextEvent(event):
+                event_mask = event.GetType();
+                if event.BroadcasterMatchesRef(self.broadcaster):
+                    if event_mask & self.eBroadcastBitStopListenerThread:
+                        done = True;
+                elif event.BroadcasterMatchesRef(self.src_broadcaster):
+                    # NOTE: https://wiki.python.org/moin/FromFunctionToMethod
+                    #
+                    # When assigning the `event_data_extractor` function pointer
+                    # to the `EventListenerTestBase` instance, it turns the
+                    # function object into an instance method which subsequently
+                    # passes `self` as an extra argument.
+
+                    # However, because most of the event data extractor
+                    # functions are static, passing the `self` argument makes
+                    # the number of passed arguments exceeds the function definition
+
+                    # This is why we unwrap the function from the instance
+                    # method object calling `__func__` instead.
+                    ret_args = self.event_data_extractor.__func__(event);
+                    if not ret_args:
+                        continue
+
+                    self.events.append(ret_args)

lldb/test/API/functionalities/diagnostic_reporting/TestDiagnosticReporting.py

 """
 Test that we are able to broadcast and receive diagnostic events from lldb
 """
 import lldb
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test.decorators import *
+
 import lldbsuite.test.lldbutil as lldbutil
-import threading
 
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.eventlistener import EventListenerTestBase
 
-class TestDiagnosticReporting(TestBase):
+class TestDiagnosticReporting(EventListenerTestBase):
 
     mydir = TestBase.compute_mydir(__file__)
-
-    eBroadcastBitStopDiagnosticThread = (1 << 0)
-
-    def setUp(self):
-        TestBase.setUp(self)
-        self.diagnostic_events = []
-
-    def fetch_events(self):
-        event = lldb.SBEvent()
-
-        done = False
-        while not done:
-            if self.listener.WaitForEvent(1, event):
-                event_mask = event.GetType()
-                if event.BroadcasterMatchesRef(self.test_broadcaster):
-                    if event_mask & self.eBroadcastBitStopDiagnosticThread:
-                        done = True
-                elif event.BroadcasterMatchesRef(self.diagnostic_broadcaster):
-                    self.diagnostic_events.append(
-                        lldb.SBDebugger.GetDiagnosticFromEvent(event))
+    event_mask = lldb.SBDebugger.eBroadcastBitWarning | lldb.SBDebugger.eBroadcastBitError
+    event_data_extractor = lldb.SBDebugger.GetDiagnosticFromEvent
 
     def test_dwarf_symbol_loading_diagnostic_report(self):
         """Test that we are able to fetch diagnostic events"""
-        self.listener = lldb.SBListener("lldb.diagnostic.listener")
-        self.test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-        self.listener.StartListeningForEvents(
-            self.test_broadcaster, self.eBroadcastBitStopDiagnosticThread)
-
-        self.diagnostic_broadcaster = self.dbg.GetBroadcaster()
-        self.diagnostic_broadcaster.AddListener(
-            self.listener, lldb.SBDebugger.eBroadcastBitWarning)
-        self.diagnostic_broadcaster.AddListener(
-            self.listener, lldb.SBDebugger.eBroadcastBitError)
-
-        listener_thread = threading.Thread(target=self.fetch_events)
-        listener_thread.start()
 
         self.yaml2obj("minidump.yaml", self.getBuildArtifact("minidump.core"))
 
         self.dbg.CreateTarget(None)
         self.target = self.dbg.GetSelectedTarget()
         self.process = self.target.LoadCore(
             self.getBuildArtifact("minidump.core"))
 
-        self.test_broadcaster.BroadcastEventByType(
-            self.eBroadcastBitStopDiagnosticThread)
-        listener_thread.join()
-
-        self.assertEquals(len(self.diagnostic_events), 1)
+        self.assertEquals(len(self.events), 1)
 
-        diagnostic_event = self.diagnostic_events[0]
+        diagnostic_event = self.events[0]
         self.assertEquals(
             diagnostic_event.GetValueForKey("type").GetStringValue(100),
             "warning")
         self.assertEquals(
             diagnostic_event.GetValueForKey("message").GetStringValue(100),
             "unable to retrieve process ID from minidump file, setting process ID to 1"
         )

lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

 """
 Test that we are able to broadcast and receive progress events from lldb
 """
 import lldb
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test.decorators import *
+
 import lldbsuite.test.lldbutil as lldbutil
-import threading
 
-class TestProgressReporting(TestBase):
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.eventlistener import EventListenerTestBase
 
-    mydir = TestBase.compute_mydir(__file__)
 
-    eBroadcastBitStopProgressThread = (1 << 0)
+class TestProgressReporting(EventListenerTestBase):
 
-    def setUp(self):
-        TestBase.setUp(self)
-        self.progress_events = []
-
-    def fetch_events(self):
-        event = lldb.SBEvent()
-
-        done = False
-        while not done:
-            if self.listener.WaitForEvent(1, event):
-                event_mask = event.GetType();
-                if event.BroadcasterMatchesRef(self.test_broadcaster):
-                    if event_mask & self.eBroadcastBitStopProgressThread:
-                        done = True;
-                elif event.BroadcasterMatchesRef(self.progress_broadcaster):
-                    ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
-                    self.assertGreater(len(ret_args), 1)
-
-                    message = ret_args[0]
-                    if message:
-                        self.progress_events.append((message, event))
+    mydir = TestBase.compute_mydir(__file__)
+    event_mask = lldb.SBDebugger.eBroadcastBitProgress
+    event_data_extractor = lldb.SBDebugger.GetProgressFromEvent
 
     def test_dwarf_symbol_loading_progress_report(self):
         """Test that we are able to fetch dwarf symbol loading progress events"""
         self.build()
 
-        self.listener = lldb.SBListener("lldb.progress.listener")
-        self.test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-        self.listener.StartListeningForEvents(self.test_broadcaster,
-                                              self.eBroadcastBitStopProgressThread)
-
-        self.progress_broadcaster = self.dbg.GetBroadcaster()
-        self.progress_broadcaster.AddListener(self.listener, lldb.SBDebugger.eBroadcastBitProgress)
-
-        listener_thread = threading.Thread(target=self.fetch_events)
-        listener_thread.start()
-
         lldbutil.run_to_source_breakpoint(self, 'break here', lldb.SBFileSpec('main.c'))
-
-        self.test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
-        listener_thread.join()
-
-        self.assertGreater(len(self.progress_events), 0)
+        self.assertGreater(len(self.events), 0)