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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


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


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

https://reviews.llvm.org/D121977