[Lldb-commits] [PATCH] Fix TestCreateAfterAttach on Windows

[Chaoren Lin]

================
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/