[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

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

1640

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

[JDevlieghere]

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

1624

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

@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

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

19 ↗

(On Diff #416586)

Just events maybe?

22 ↗

(On Diff #416586)

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).

23 ↗

(On Diff #416586)

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

6 ↗

(On Diff #416581)

EventListenerTest

[jingham]

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

1624

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

23 ↗

(On Diff #416586)

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 ↗

(On Diff #416631)

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 ↗

(On Diff #416631)

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 ↗

(On Diff #416631)

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 ↗

(On Diff #416631)

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/lldbutil.py

with open(log, "r") as logfile:
                 line = line[1:]
                 # Unescape '}'.
                 dylib_info = json.loads(line.replace('}]','}')[:-4])
                 expect_dylib_info_response = False
             if 'send packet: $jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}' in line:
                 expect_dylib_info_response = True
 
     return dylib_info
+
+# ==================================================
+# Utility functions to setup a progress event listener
+# ==================================================
+
+import threading
+
+class ProgressEventListener:
+
+    eBroadcastBitStopProgressThread = (1 << 0)
+
+    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.broadcaster):
+                    if event_mask & self.eBroadcastBitStopProgressThread:
+                        done = True;
+                elif event.BroadcasterMatchesRef(self.src_broadcaster):
+                    ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
+                    if not len(ret_args):
+                        continue
+
+                    message = ret_args[0]
+                    if message:
+                        self.progress_events.append((message, event))
+
+    def start(self, src_broadcaster, event_mask, callback=self.__fetch_events__):
+        self.src_broadcaster = src_broadcaster
+        self.broadcaster = lldb.SBBroadcaster('lldb.progress.broadcaster')
+        self.listener = lldb.SBListener("lldb.progress.listener")
+        self.listener.StartListeningForEvents(self.test_broadcaster,
+                                              self.eBroadcastBitStopProgressThread)
+
+        src_broadcaster.AddListener(self.listener, event_mask)
+
+        self.listener_thread = threading.Thread(target=callback)
+        self.listener_thread.start()
+
+    def stop(self):
+        self.broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
+        self.listener_thread.join()
+        return self.progress_events

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):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    eBroadcastBitStopProgressThread = (1 << 0)
-
     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))
 
     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()
+        progress_events_listener = lldbutil.ProgressEventListener()
+        progress_events_listener.start(self.dbg.GetBroadcaster(),
+                                       lldb.SBDebugger.eBroadcastBitProgress)
 
         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)
+        progress_events = progress_events_listener.stop()
+        self.assertGreater(len(progress_events), 0)