[Lldb-commits] [PATCH] D16247: Don't assume that thread 0 is always the main thread

[Jim Ingham via lldb-commits]

jingham added a comment.

Yes, that is a cleaner way to do this.

In about half the cases you use get_threads_stopped_at_breakpoint and half get_stopped_thread.  It looks like you were just mirroring what the test did, so that's fine, but if I know the breakpoint I'm expecting, I prefer get_threads_stopped_at_breakpoint as it is a more complete test.  As noted in the individual comments, I don't think you are using get_threads_stopped_at_breakpoint right, since it returns an array of threads, not a single thread.

BTW, I'm not averse to the notion of an API to get the "main thread" but none of these tests should be changed to use it (or should have used it if it already existed.)  After all, you wanted to know that you stopped on a thread at some breakpoint you set.  The fact that that is the "main" thread is incidental and gating ALL these tests on having "the main thread" even be a sensible notion for your platform seems to make them fragile for no benefit.


================
Comment at: packages/Python/lldbsuite/test/expression_command/test/TestExprs.py:133-134
@@ -132,8 +132,4 @@
 
-        # The stop reason of the thread should be breakpoint.
-        thread = process.GetThreadAtIndex(0)
-        if thread.GetStopReason() != lldb.eStopReasonBreakpoint:
-            from lldbsuite.test.lldbutil import stop_reason_to_str
-            self.fail(STOPPED_DUE_TO_BREAKPOINT_WITH_STOP_REASON_AS %
-                      stop_reason_to_str(thread.GetStopReason()))
+        thread = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint)
+        self.assertIsNotNone(thread, "Could not find thread stopped at breakpoint")
 
----------------
Is this one right? get_threads_stopped_at_breakpoint returns an array. You need to check that the return has only one element and then set thread to that element. Unless there's some Python magic that I'm missing...

================
Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoins/TestConsecutiveBreakpoints.py:40
@@ -39,4 +39,3 @@
         # We should be stopped at the first breakpoint
-        thread = process.GetThreadAtIndex(0)
-        self.assertEqual(thread.GetStopReason(), lldb.eStopReasonBreakpoint)
+        thread = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint)
 
----------------
Is this one right? get_threads_stopped_at_breakpoint returns an array. You need to check that the return has only one element and then set thread to that element. Unless there's some Python magic that I'm missing...

================
Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoins/TestConsecutiveBreakpoints.py:54-55
@@ -54,4 +53,4 @@
         # We should be stopped at the second breakpoint
-        thread = process.GetThreadAtIndex(0)
-        self.assertEqual(thread.GetStopReason(), lldb.eStopReasonBreakpoint)
+        thread = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint2)
+        self.assertIsNotNone(thread)
 
----------------
Is this one right? get_threads_stopped_at_breakpoint returns an array. You need to check that the return has only one element and then set thread to that element. Unless there's some Python magic that I'm missing...

================
Comment at: packages/Python/lldbsuite/test/functionalities/conditional_break/TestConditionalBreak.py:67-68
@@ -66,3 +66,4 @@
                 print("j is: ", j)
-            thread = process.GetThreadAtIndex(0)
+            thread = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint)
+            self.assertIsNotNone(thread)
             
----------------
Is this one right?  get_threads_stopped_at_breakpoint returns an array.  You need to check that the return has only one element and then set thread to that element.  Unless there's some Python magic that I'm missing...


http://reviews.llvm.org/D16247