[Lldb-commits] [PATCH] D119167: [lldb/test] Remove sleeps from some lldb-server tests

[Pavel Labath via Phabricator via lldb-commits]

labath planned changes to this revision.
labath marked an inline comment as done.
labath added a comment.

Speaking of windows, I just realized that this method will (obviously) not work there. I should have occurred to me there was a reason I didn't do this earlier. Let me try a different synchronization method instead.



================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py:699
+        threads = self.parse_threadinfo_packets(context)
+        self.assertGreaterEqual(len(threads), thread_count)
+        return context, threads
----------------
DavidSpickett wrote:
> I don't think this is an issue but what would cause you to have > the number of threads?
> 
> Are there automatically created threads sometimes that we don't explicitly ask for.
Windows `std::thread`s maintain a thread pool of spun-up threads, presumably to speed up their creation. It is somewhat annoying because, for some of these lower-level tests, we would really want to know the exact number of threads and their properties. One day, I'm going to create some some thread creation wrappers to avoid this...


================
Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:22
+        key_vals_text = context.get("stop_reply_kv")
+        self.assertIsNotNone(key_vals_text)
 
----------------
DavidSpickett wrote:
> If there's a very slim chance this text is `""` you could `assertTrue` and catch both situations.
technically, "" might be correct, if we decide to return a bare `Txx` packet for some reason.


================
Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:34
         kv_dict = self.parse_key_val_dict(key_vals_text)
         self.assertIsNotNone(kv_dict)
 
----------------
DavidSpickett wrote:
> Afaict `parse_key_val_dict` will never return None. At worst an empty dictionary.
Yeah, this code is being overly defensive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119167