[lldb-dev] New test summary results formatter

Zachary Turner via lldb-dev lldb-dev at lists.llvm.org
Wed Dec 2 14:18:14 PST 2015 

 think I got something working, I'll upload a patch in a few.

On Wed, Dec 2, 2015 at 2:10 PM Zachary Turner <zturner at google.com> wrote:

> BTW, this is what I do in the swig service, and I think it's the idiomatic
> way of sending variable length data over a socket.
>
> On Wed, Dec 2, 2015 at 2:08 PM Zachary Turner <zturner at google.com> wrote:
>
>> I think the best solution is going to be to use struct.pack.  Like this
>> on the writing side:
>>
>>         import struct
>>         msg = cPickle.dumps(test_event)
>>         packet = struct.pack("!I%ds" % len(msg), len(msg), msg)
>>         self.out_file.send(packet)
>>
>> and like this on the reading side:
>>
>>                 self.packet_bytes_remaining = struct.unpack("!I",
>> self.header_contents)[0]
>>                 self.header_contents = b""
>>                 self.reading_header = False
>>                 return data[(index+1):]
>>
>> The current solution still doesn't work because it's calling
>> socket.send() with a str instead of a bytes.  So it has to be bytes all the
>> way through.
>>
>> On Wed, Dec 2, 2015 at 1:48 PM Todd Fiala <todd.fiala at gmail.com> wrote:
>>
>>> Those fixes are up here:
>>> r254550
>>>
>>> Let me know what you see after that, Zachary.
>>>
>>> On Wed, Dec 2, 2015 at 1:45 PM, Todd Fiala <todd.fiala at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Dec 2, 2015 at 1:42 PM, Todd Fiala <todd.fiala at gmail.com>
>>>> wrote:
>>>>
>>>>> Can you try making those changes in the other spots?  There's a
>>>>> handful of code you have probably not ever run if you haven't selected
>>>>> running with a test results formatter.
>>>>>
>>>>>
>>>> I'm actually going to make the ibuffer ones now since it's easier for
>>>> me to verify that it doesn't break the non-Windows side since I'm right in
>>>> there now.  If that doesn't do it, we'll need to see what else doesn't work.
>>>>
>>>>
>>>>> If not, I can try to address them tonight or tomorrow night when I can
>>>>> get to some kind of python 3 setup.
>>>>>
>>>>> On Wed, Dec 2, 2015 at 1:36 PM, Zachary Turner <zturner at google.com>
>>>>> wrote:
>>>>>
>>>>>> Yea I was messing around with it too.  I don't have a fix yet but I
>>>>>> think you will need to either encode the pickled data as utf8, or better
>>>>>> yet, don't write this:
>>>>>>
>>>>>> "{}#{}".format(...)
>>>>>>
>>>>>> because pickled data is supposed to be binary data anyway.  So use
>>>>>> bytes.append() instead.
>>>>>>
>>>>>> Then on the other side in dotest_channels, there's a couple places
>>>>>> where you do something like:
>>>>>>
>>>>>> self.ibuffer = ""
>>>>>>
>>>>>> which would need to change to
>>>>>>
>>>>>> self.ibuffer = b""
>>>>>>
>>>>>> and any other similar operations on self.ibuffer which assume string
>>>>>> data.
>>>>>>
>>>>>> On Wed, Dec 2, 2015 at 1:33 PM Todd Fiala <todd.fiala at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I think I know how to fix.  Trying now.
>>>>>>>
>>>>>>> On Wed, Dec 2, 2015 at 1:17 PM, Todd Fiala <todd.fiala at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I think I can fix the issue without you debugging.
>>>>>>>>
>>>>>>>> Getting the single pass test runner to use it isn't impossible but
>>>>>>>> will take some work.  Can you direct-send me the backtrace from the point
>>>>>>>> of failure from your system?  Thanks!
>>>>>>>>
>>>>>>>> -Todd
>>>>>>>>
>>>>>>>> On Wed, Dec 2, 2015 at 12:34 PM, Zachary Turner <zturner at google.com
>>>>>>>> > wrote:
>>>>>>>>
>>>>>>>>> Is there any way to force the single process test runner to use
>>>>>>>>> this same system?  I'm trying to debug the problem, but this codepath
>>>>>>>>> doesn't execute in the single process test runner, and it executes in the
>>>>>>>>> child process in the multiprocess test runner.  Basically I need the
>>>>>>>>> following callstack to execute in the single process test runner:
>>>>>>>>>
>>>>>>>>> Command invoked: C:\Python35\python_d.exe
>>>>>>>>> D:\src\llvm\tools\lldb\test\dotest.py -q --arch=i686 --executable
>>>>>>>>> D:/src/llvmbuild/ninja_py35/bin/lldb.exe -s
>>>>>>>>> D:/src/llvmbuild/ninja_py35/lldb-test-traces -u CXXFLAGS -u CFLAGS
>>>>>>>>> --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe
>>>>>>>>> --results-port 60794 --inferior -p TestIntegerTypesExpr.py
>>>>>>>>> D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test --event-add-entries
>>>>>>>>> worker_index=7:int
>>>>>>>>> 411 out of 412 test suites processed - TestIntegerTypesExpr.py
>>>>>>>>> Traceback (most recent call last):
>>>>>>>>>   File "D:\src\llvm\tools\lldb\test\dotest.py", line 7, in <module>
>>>>>>>>>     lldbsuite.test.run_suite()
>>>>>>>>>   File
>>>>>>>>> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\dotest.py", line
>>>>>>>>> 1476, in run_suite
>>>>>>>>>     setupTestResults()
>>>>>>>>>   File
>>>>>>>>> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\dotest.py", line
>>>>>>>>> 982, in setupTestResults
>>>>>>>>>     results_formatter_object.handle_event(initialize_event)
>>>>>>>>>   File
>>>>>>>>> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\test_results.py",
>>>>>>>>> line 1033, in handle_event
>>>>>>>>>     "{}#{}".format(len(pickled_message), pickled_message))
>>>>>>>>> TypeError: a bytes-like object is required, not 'str'
>>>>>>>>>
>>>>>>>>> On Wed, Dec 2, 2015 at 11:40 AM Zachary Turner <zturner at google.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> When I run this under Python 3 I get "A bytes object is used like
>>>>>>>>>> a string" on Line 1033 of test_results.py.  I'm going to dig into it a
>>>>>>>>>> little bit, but maybe you know off the top of your head the right way to
>>>>>>>>>> fix it.
>>>>>>>>>>
>>>>>>>>>> On Wed, Dec 2, 2015 at 11:32 AM Zachary Turner <
>>>>>>>>>> zturner at google.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Oh yea, I made up that decorator idea because I didn't know all
>>>>>>>>>>> the formatters were derived from a common base.  But your idea is better if
>>>>>>>>>>> everything is derived from a common base.  To be honest you could even just
>>>>>>>>>>> generate an error if there are two ResultsFormatter derived classes in the
>>>>>>>>>>> same module.  We should be encouraging more smaller files with single
>>>>>>>>>>> responsibility.  One of the things I plan to do as part of some cleanup in
>>>>>>>>>>> a week or two is to split up dotest, dosep, and lldbtest.py into a couple
>>>>>>>>>>> different files by breaking out things like TestBase, etc into separate
>>>>>>>>>>> files.  So that it's easier to keep a mental map of where different code is.
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:26 AM Todd Fiala <todd.fiala at gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:20 AM, Todd Fiala <
>>>>>>>>>>>> todd.fiala at gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Yeah I'd be good with that.  I can change that as well.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Todd
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:10 AM, Zachary Turner <
>>>>>>>>>>>>> zturner at google.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also another stylistic suggestion.  I've been thinking about
>>>>>>>>>>>>>> how to more logically organize all the source files now that we have a
>>>>>>>>>>>>>> package.  So it makes sense conceptually to group all of the different
>>>>>>>>>>>>>> result formatters under a subpackage called formatters.  So right now
>>>>>>>>>>>>>> you've got lldbsuite.test.basic_results_formatter.
>>>>>>>>>>>>>> BasicResultsFormatter but it might make sense for this to be
>>>>>>>>>>>>>> lldbsuite.test.formatters.basic.BasicResultsFormatter.  If you do things
>>>>>>>>>>>>>> this way, it can actually result in a substantially shorter command line,
>>>>>>>>>>>>>> because the --results-formatter option can use lldbsuite.test.formatters as
>>>>>>>>>>>>>> a starting point.  So you could instead write:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> test/dotest.py --results-formatter basic
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> dotest then looks for a `basic.py` module in the
>>>>>>>>>>>>>> `lldbsuite.test.formatters` package, looks for a class inside with a
>>>>>>>>>>>>>> @result_formatter decorator, and instantiates that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This has the advantage of making the command line shorter
>>>>>>>>>>>>>> *and* a more logical source file organization.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>> The other thing that could allow me to do is possibly
>>>>>>>>>>>> short-circuit the results formatter specifier so that, if just the module
>>>>>>>>>>>> is specified, and if the module only has one ResultsFormatter-derived
>>>>>>>>>>>> class, I can probably rig up code that figures out the right results
>>>>>>>>>>>> formatter, shortening the required discriminator to something even shorter
>>>>>>>>>>>> (i.e. module.classname becomes just module.)
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:04 AM Zachary Turner <
>>>>>>>>>>>>>> zturner at google.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Can --results-file=stdout be the default so that we don't
>>>>>>>>>>>>>>> have to specify that?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:02 AM Todd Fiala via lldb-dev <
>>>>>>>>>>>>>>> lldb-dev at lists.llvm.org> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Also, all the text in the summary is fixed-width lined up
>>>>>>>>>>>>>>>> nicely, which may not show in the commit message description if you're
>>>>>>>>>>>>>>>> using a variable-width font.  On a terminal it looks nice.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Wed, Dec 2, 2015 at 11:01 AM, Todd Fiala <
>>>>>>>>>>>>>>>> todd.fiala at gmail.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Wed, Dec 2, 2015 at 10:57 AM, Todd Fiala <
>>>>>>>>>>>>>>>>> todd.fiala at gmail.com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I just put up an optional test results formatter that is
>>>>>>>>>>>>>>>>>> a prototype of what we may move towards for our default test summary
>>>>>>>>>>>>>>>>>> results.  It went in here:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> r254530
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> and you can try it out with something like:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> time test/dotest.py --executable `pwd`/build/Debug/lldb
>>>>>>>>>>>>>>>>>> --results-formatter
>>>>>>>>>>>>>>>>>> lldbsuite.test.basic_results_formatter.BasicResultsFormatter --results-file
>>>>>>>>>>>>>>>>>> st
>>>>>>>>>>>>>>>>>> out
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I cut and paste my line, but more than likely for most
>>>>>>>>>>>>>>>>> people you'd just want this:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> test/dotest.py --results-formatter
>>>>>>>>>>>>>>>>> lldbsuite.test.basic_results_formatter.BasicResultsFormatter --results-file
>>>>>>>>>>>>>>>>> stdout
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> The other stuff was specific to my setup.  That line
>>>>>>>>>>>>>>>>> assumes you run from the lldb source dir root.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Let me know if this satisfies the basic needs of counts
>>>>>>>>>>>>>>>>>> and whatnot.  It counts test method runs rather than all the oddball "file,
>>>>>>>>>>>>>>>>>> class, etc." counts we had before.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> It prints out the Details section when there are details,
>>>>>>>>>>>>>>>>>> and keeps it nice and clean when there are none.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> It also mentions a bit about test reruns up top, but that
>>>>>>>>>>>>>>>>>> won't come into play until I get the multi-test-pass,
>>>>>>>>>>>>>>>>>> single-worker/low-load mechanism in place, which will depend on newer rerun
>>>>>>>>>>>>>>>>>> count awareness support.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> The change also cleans up places where the test event
>>>>>>>>>>>>>>>>>> framework was using string codes and replaces them with symbolic constants.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Let me know what you think.  I can tweak it as needed to
>>>>>>>>>>>>>>>>>> address testbot and other needs.  Once it looks reasonable, I'd like to
>>>>>>>>>>>>>>>>>> move over to using it by default in the parallel test runner rather than
>>>>>>>>>>>>>>>>>> the legacy support.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>> -Todd
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>> -Todd
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> -Todd
>>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>>> lldb-dev mailing list
>>>>>>>>>>>>>>>> lldb-dev at lists.llvm.org
>>>>>>>>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> -Todd
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> -Todd
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> -Todd
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> -Todd
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> -Todd
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> -Todd
>>>>
>>>
>>>
>>>
>>> --
>>> -Todd
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151202/f255dc50/attachment-0001.html>

More information about the lldb-dev mailing list