Page MenuHomePhabricator


Authored by jingham on Oct 25 2019, 3:46 PM.



This test was timing out on the bots occasionally, so I was poking around at it. I didn't see any obvious reason why it would stall, but it had a slightly odd way of rendezvousing the two test threads, and it also finished by running the inferior to completion, even though that wasn't relevant to the test at hand.

So I took the opportunity to modernize the test, and clean up these aspects of it. In order to do that I had to make run_to_source_breakpoint accept the case where the breakpoint it was setting had more than one thread hit the created breakpoint. So this diff also has the modifications to pass an "only_one_thread" parameter if you want to allow more than one thread stopping at the candidate breakpoint. The function still returns only one thread, since the vast majority of uses will require only one thread, and it's easy to find the threads stopped at a breakpoint if you need that.

Diff Detail


Event Timeline

jingham created this revision.Oct 25 2019, 3:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2019, 3:46 PM

Thanks! Nitpicking inside.


If self.bkpt_string and self.main_file are only used here, I would find this easier to read:

ldbutil.run_to_source_breakpoint(self, 'break here', lldb.SBFileSpec('main.cpp'), only_one_thread = False)


this isn't used in the next condition?


assertEquals! :-)


assertGreater (see .../unittest2/

jingham updated this revision to Diff 226772.Oct 28 2019, 3:50 PM

Addressed review comments.

jingham marked 4 inline comments as done.Oct 28 2019, 3:52 PM
jingham added inline comments.

self.main_file is only used here, I moved it.

self.bkpt_string is used to calculate the line number self.breakpoint, and then the source breakpoint we set in run_to_source_breakpoint is supposed to match that line. So I think it is clearer to use the variable than to repeat the pattern twice.

aprantl accepted this revision.Oct 28 2019, 4:01 PM
This revision is now accepted and ready to land.Oct 28 2019, 4:01 PM
This revision was automatically updated to reflect the committed changes.