================ Comment at: test/functionalities/thread/create_after_attach/TestCreateAfterAttach.py:93 @@ -90,1 +92,3 @@ + 'main', + 'stop reason = breakpoint']) ---------------- amccarth wrote: > chaoren wrote: > > amccarth wrote: > > > labath wrote: > > > > I think this would be a good time to switch from substring matching to > > > > checking things properly using the SBAPI. Right now, it's getting quite > > > > hard to tell what this tests exactly, and if the all edge cases are > > > > treated properly (e.g. what would happen if one thread stops in "main", > > > > but some other thread has "stop reason = breakpoint"). lldbutil has a > > > > lot of wrappers to make it easy to use the python api. For example, > > > > this fragment could be written way more robustly like this: > > > > ``` > > > > threads = lldbutil.continue_to_breakpoint(process, 1) # run to first > > > > breakpoint > > > > self.assertEquals(len(threads), 1) > > > > self.assertEquals(threads[0].GetFrameAtIndex(0).GetFunctionName(), > > > > "main") > > > > ``` > > > > (disclaimer: I did not test this code, but I believe you get the idea) > > > I agree that should be done, but I would prefer to do it later, in a > > > separate patch. This patch has been blocked for two weeks while I > > > tracked down the problem with the confusion between the thread resume > > > state and the thread temporary resume state in the Windows > > > implementation. Further modification of the test increases the risk of > > > breaking it for other platforms than the one I'm focused on. > > > > > > There are higher priority things now. For example, because of some > > > crashing tests, simply running ninja check-lldb hangs, leaving dozens of > > > orphaned processes. > > ``` > > self.assertEquals(len(threads), 1) > > ``` > > > > I don't think you can make any assumptions on the number of threads because > > of the thread pool. But it is an important assertion to make on other > > platforms. > At this point, even on other platforms, there are at least two threads at > this point: the main thread, and the one spawned before this breakpoint. > > I could do self.assertGreaterEqual(len(threads), 2), if you think that's > useful. I think it's useful to assert to exact number of threads, and the creation order of those threads, and we'd still have a problem even if using the SB api. Perhaps you could create a Windows version of this test case where thread numbers/order don't matter and disable this one?
http://reviews.llvm.org/D10850 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
