Page MenuHomePhabricator

Modernize TestThreadStepOut.py
ClosedPublic

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

Details

Summary

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.Fri, Oct 25, 3:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Oct 25, 3:46 PM

Thanks! Nitpicking inside.

lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
135

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)

lldb/packages/Python/lldbsuite/test/lldbutil.py
781–786

this isn't used in the next condition?

783

assertEquals! :-)

785

assertGreater (see .../unittest2/case.py)

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

Addressed review comments.

jingham marked 4 inline comments as done.Mon, Oct 28, 3:52 PM
jingham added inline comments.
lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
135

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.Mon, Oct 28, 4:01 PM
This revision is now accepted and ready to land.Mon, Oct 28, 4:01 PM
This revision was automatically updated to reflect the committed changes.