Re: [Lldb-commits] [lldb] r327924 - Modernize a test.

[Jim Ingham via lldb-commits]

So that file you are looking for is 
"packages/Python/lldbsuite/test/README-testsuite.  It would be great to have a 
fresh pair of eyes look this over and add whatever you would have found 
useful...

Jim


> On Mar 19, 2018, at 6:44 PM, Davide Italiano  wrote:
> 
> On Mon, Mar 19, 2018 at 6:38 PM, Jim Ingham  wrote:
>> Yes, I should have written this function much earlier.  I've been trying to 
>> convert over to it every time a tests draws my attention (in this case 
>> 'cause I pointed it out for you to copy...)
>> 
>> It is the way to make a test in the sample_test/TestSampleTest.py.  That 
>> seemed the most effective way to draw it to people's attention.
>> 
>> We could also document it in the README-testsuite as well, after the line 
>> recommending that you use the sample_test, but my experience is that people 
>> don't much read that doc?
>> 
>> Might also be worth a mention in the Projects doc - convert all relevant 
>> tests to use run_to_source_breakpoint...
>> 
>> I can't think of another prominent place to put docs for it.  Maybe if 
>> lldbutil.py had a header that lists the most important routines that module 
>> provides, people might run across it there.
>> 
>> As a new person coming to the code base, where would you have looked to find 
>> this?
>> 
> 
> I (but I can't speak for everybody of course :) generally look at
> other tests so having everything converted to the new style would
> help.
> That said, having maybe a doc for test best practices & how to debug
> now that we're building a pretty decent infrastructure would help
> At least, if we have it, in the code reviews we can just paste a link
> instead of repeating the same set of facts over and over (and
> eventually enough people will see it that they'll learn :)

That file exists, it is:

packages/Python/lldbsuite/test/README-testsuite

but it could certainly use some love.  I wonder if moving it into the www 
directory and linking it on the project page might not make it easier to find?

> .
> I'd mention, among others:
> a) inline tests (IMHO, completely change the way of writing test, much
> more concise and nicer for the "common" case)
> b) Jonas' work on lldb-dotest (for people running CMake build, another
> game changer as you don't have to worry about the location of your
> executables when reproducing issues)
> c) common utilities (like the one you recommended)
> d) how to drop into a python debugger (Adrian taught this to me and I
> need to ask him all the time because I forget)
> 
> I'll add this to my TODO list, unless somebody beats me to the punch
> (as my work queue is an append-only data structure these days).

These all sound great!  

Jim

> 
> Thanks!
> 
> --
> Davide

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r327924 - Modernize a test.

[Davide Italiano via lldb-commits]

On Mon, Mar 19, 2018 at 6:38 PM, Jim Ingham  wrote:
> Yes, I should have written this function much earlier.  I've been trying to 
> convert over to it every time a tests draws my attention (in this case 'cause 
> I pointed it out for you to copy...)
>
> It is the way to make a test in the sample_test/TestSampleTest.py.  That 
> seemed the most effective way to draw it to people's attention.
>
> We could also document it in the README-testsuite as well, after the line 
> recommending that you use the sample_test, but my experience is that people 
> don't much read that doc?
>
> Might also be worth a mention in the Projects doc - convert all relevant 
> tests to use run_to_source_breakpoint...
>
> I can't think of another prominent place to put docs for it.  Maybe if 
> lldbutil.py had a header that lists the most important routines that module 
> provides, people might run across it there.
>
> As a new person coming to the code base, where would you have looked to find 
> this?
>

I (but I can't speak for everybody of course :) generally look at
other tests so having everything converted to the new style would
help.
That said, having maybe a doc for test best practices & how to debug
now that we're building a pretty decent infrastructure would help
At least, if we have it, in the code reviews we can just paste a link
instead of repeating the same set of facts over and over (and
eventually enough people will see it that they'll learn :)
.
I'd mention, among others:
a) inline tests (IMHO, completely change the way of writing test, much
more concise and nicer for the "common" case)
b) Jonas' work on lldb-dotest (for people running CMake build, another
game changer as you don't have to worry about the location of your
executables when reproducing issues)
c) common utilities (like the one you recommended)
d) how to drop into a python debugger (Adrian taught this to me and I
need to ask him all the time because I forget)

I'll add this to my TODO list, unless somebody beats me to the punch
(as my work queue is an append-only data structure these days).

Thanks!

--
Davide
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r327924 - Modernize a test.

[Jim Ingham via lldb-commits]

Yes, I should have written this function much earlier.  I've been trying to 
convert over to it every time a tests draws my attention (in this case 'cause I 
pointed it out for you to copy...)

It is the way to make a test in the sample_test/TestSampleTest.py.  That seemed 
the most effective way to draw it to people's attention.

We could also document it in the README-testsuite as well, after the line 
recommending that you use the sample_test, but my experience is that people 
don't much read that doc?  

Might also be worth a mention in the Projects doc - convert all relevant tests 
to use run_to_source_breakpoint...

I can't think of another prominent place to put docs for it.  Maybe if 
lldbutil.py had a header that lists the most important routines that module 
provides, people might run across it there.

As a new person coming to the code base, where would you have looked to find 
this?

Jim

> On Mar 19, 2018, at 6:31 PM, Davide Italiano  wrote:
> 
> On Mon, Mar 19, 2018 at 4:15 PM, Jim Ingham via lldb-commits
>  wrote:
>> Author: jingham
>> Date: Mon Mar 19 16:15:06 2018
>> New Revision: 327924
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=327924=rev
>> Log:
>> Modernize a test.
>> 
>> Modified:
>>
>> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>> 
>> Modified: 
>> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py?rev=327924=327923=327924=diff
>> ==
>> --- 
>> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>>  (original)
>> +++ 
>> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>>  Mon Mar 19 16:15:06 2018
>> @@ -48,35 +48,8 @@ class TestCppIncompleteTypes(TestBase):
>> # Get main source file
>> src_file = "main.cpp"
>> src_file_spec = lldb.SBFileSpec(src_file)
>> -self.assertTrue(src_file_spec.IsValid(), "Main source file")
>> -
>> -# Get the path of the executable
>> -exe_path = self.getBuildArtifact(exe)
>> -
>> -# Load the executable
>> -target = self.dbg.CreateTarget(exe_path)
>> -self.assertTrue(target.IsValid(), VALID_TARGET)
>> -
>> -# Break on main function
>> -main_breakpoint = target.BreakpointCreateBySourceRegex(
>> -"break here", src_file_spec)
>> -self.assertTrue(
>> -main_breakpoint.IsValid() and main_breakpoint.GetNumLocations() 
>> >= 1,
>> -VALID_BREAKPOINT)
>> -
>> -# Launch the process
>> -args = None
>> -env = None
>> -process = target.LaunchSimple(
>> -args, env, self.get_process_working_directory())
>> -self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
>> -
>> -# Get the thread of the process
>> -self.assertTrue(
>> -process.GetState() == lldb.eStateStopped,
>> -PROCESS_STOPPED)
>> -thread = lldbutil.get_stopped_thread(
>> -process, lldb.eStopReasonBreakpoint)
>> 
>> +(target, process, thread, main_breakpoint) = 
>> lldbutil.run_to_source_breakpoint(self,
>> +"break here", src_file_spec, exe_name = exe)
>> # Get frame for current thread
>> return thread.GetSelectedFrame()
>> 
> 
> Thanks. I was actually adding a new test and complaining to myself
> that there was too much boilerplate.
> This helper clearly makes things much better. It's probably worth
> documenting it somewhere? What do you think?
> 
> Thanks!
> 
> --
> Davide

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r327924 - Modernize a test.

[Davide Italiano via lldb-commits]

On Mon, Mar 19, 2018 at 4:15 PM, Jim Ingham via lldb-commits
 wrote:
> Author: jingham
> Date: Mon Mar 19 16:15:06 2018
> New Revision: 327924
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327924=rev
> Log:
> Modernize a test.
>
> Modified:
> 
> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py?rev=327924=327923=327924=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
>  Mon Mar 19 16:15:06 2018
> @@ -48,35 +48,8 @@ class TestCppIncompleteTypes(TestBase):
>  # Get main source file
>  src_file = "main.cpp"
>  src_file_spec = lldb.SBFileSpec(src_file)
> -self.assertTrue(src_file_spec.IsValid(), "Main source file")
> -
> -# Get the path of the executable
> -exe_path = self.getBuildArtifact(exe)
> -
> -# Load the executable
> -target = self.dbg.CreateTarget(exe_path)
> -self.assertTrue(target.IsValid(), VALID_TARGET)
> -
> -# Break on main function
> -main_breakpoint = target.BreakpointCreateBySourceRegex(
> -"break here", src_file_spec)
> -self.assertTrue(
> -main_breakpoint.IsValid() and main_breakpoint.GetNumLocations() 
> >= 1,
> -VALID_BREAKPOINT)
> -
> -# Launch the process
> -args = None
> -env = None
> -process = target.LaunchSimple(
> -args, env, self.get_process_working_directory())
> -self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
> -
> -# Get the thread of the process
> -self.assertTrue(
> -process.GetState() == lldb.eStateStopped,
> -PROCESS_STOPPED)
> -thread = lldbutil.get_stopped_thread(
> -process, lldb.eStopReasonBreakpoint)
>
> +(target, process, thread, main_breakpoint) = 
> lldbutil.run_to_source_breakpoint(self,
> +"break here", src_file_spec, exe_name = exe)
>  # Get frame for current thread
>  return thread.GetSelectedFrame()
>

Thanks. I was actually adding a new test and complaining to myself
that there was too much boilerplate.
This helper clearly makes things much better. It's probably worth
documenting it somewhere? What do you think?

Thanks!

--
Davide
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits