[Lldb-commits] [PATCH] D121977: [lldb/test] Add events listener helper class to lldbtest

Fri Mar 18 13:40:33 PDT 2022

jingham added inline comments.

================
Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1624
+# ==================================================
+# Utility class to setup a progress event listener
+# ==================================================
----------------
JDevlieghere wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > 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. 
> > @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) ?
> 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`.
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.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121977/new/

https://reviews.llvm.org/D121977