<div dir="ltr">Thanks, Zachary.<br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 3:59 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Now that it's working in Python 3 and I've had a chance to see the output, I think I like it much better. These are the things I noticed (good and bad):<div><br></div><div>1) Output of each test is printed on the fly as the test is run. Much better than staring at a black screen for 5 minutes and then seeing a bunch of junk fly by.</div><div>2) Summary output is organized by test function, not by file. This gives better counts.</div><div>3) Behavior seems different somehow. My successes / failures / unexpected successes seem different when using this formatter. I'll have to look into this to figure out if it's just my imagination or something's actually wrong.</div></div></blockquote><div><br></div><div>Let me know if you see anything wrong. We've been using the Xunit formatter with our CI for 3-ish months at this point (which runs on the test event architecture), and have been happy with the results. Our CI cleanup still gets timeouts and exceptional exits from the legacy summary, but I have bugzilla bugs on routing those through the test event system as well. That means ResultFormatter implementations can handle them, which is ultimately what we want.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Will this abstraction of a result formatter provide some way to make a formatter intended for machine consumption? </div></div></blockquote><div><br></div><div>Sure. What did you have in mind? Some kind of structured output? (Right now the Xunit formatter is exactly that - our CI uses the Xunit output and ingests it into Jenkins, which has an Xunit-based plugin that can interpret and display the results quite nicely).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Right now lots of people scrape logs for the purposes of build bots, and it would be better if we could just dump the results as json or something.</div><div><br></div><div>Only suggestion I have at this point is that in the summary printout <span style="line-height:1.5">the lines are too long. </span></div></div></blockquote><div><br></div><div>Agreed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="line-height:1.5">Long test name (module.classname.function) and long filename. Perhaps the filename could be converted to a relative path</span></div></div></blockquote><div><br></div><div>This was something I was planning on doing. I like the idea.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="line-height:1.5"> and the classname could be dropped (there's only one class per file anyway, so the classname is just wasted space)</span></div></div></blockquote><div><br></div><div>Part of the reason I included that is I've hit several times where copy and paste errors lead to the same class name, method name or even file name being used for a test. I think, though, that most of those are addressed by having the path (relative is fine) to the python test file. I think we can probably get by with classname.methodname (relative test path). (From your other email, I think you nuke the classname and keep the module name, but I'd probably do the reverse, keeping the class name and getting rid of the module name since it can be derived from the filename).</div><div><br></div><div>Definitely let me know if you find any issues aside from lack of timeout and exceptional (POSIX-land crashed process) exits. As you noted and I described, the counts are all now test method based.</div><div><br></div><div>Feel free to play with the Xunit one as well if you're looking for something machine readable. And if you're running a test run on Linux, the curses-based one gives nice output with some user control over what test results show in the lower portion of the display. The upper portion shows you what each of the workers is doing. --curses is the shortcut to get to it.</div><div><br></div><div>-Todd</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 3:06 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">All good.<div><br></div><div>Yeah Python 2/3 string/byte handling is one of the trickier areas of cross-Python version compatibility.</div></div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 2:18 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> think I got something working, I'll upload a patch in a few.</div><div><div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 2:10 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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. </div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 2:08 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think the best solution is going to be to use struct.pack. Like this on the writing side:<div><br></div><div><div> import struct</div><div> msg = cPickle.dumps(test_event)</div><div> packet = struct.pack("!I%ds" % len(msg), len(msg), msg)</div><div> self.out_file.send(packet)</div></div><div><br></div><div>and like this on the reading side:</div><div><br></div><div><div><div> self.packet_bytes_remaining = struct.unpack("!I", self.header_contents)[0]</div><div> self.header_contents = b""</div><div> self.reading_header = False</div><div> return data[(index+1):]</div></div></div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 1:48 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Those fixes are up here:<div>r254550</div><div><br></div><div>Let me know what you see after that, Zachary.</div></div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 1:45 PM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Wed, Dec 2, 2015 at 1:42 PM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div></div></blockquote><div><br></div></span><div>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.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>If not, I can try to address them tonight or tomorrow night when I can get to some kind of python 3 setup.</div></div><div class="gmail_extra"><div><div><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 1:36 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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:<div><br></div><div>"{}#{}".format(...)</div><div><br></div><div>because pickled data is supposed to be binary data anyway. So use bytes.append() instead.</div><div><br></div><div>Then on the other side in dotest_channels, there's a couple places where you do something like:</div><div><br></div><div>self.ibuffer = ""</div><div><br></div><div>which would need to change to</div><div><br></div><div>self.ibuffer = b""</div><div><br></div><div>and any other similar operations on self.ibuffer which assume string data.</div></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 1:33 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think I know how to fix. Trying now.</div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 1:17 PM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think I can fix the issue without you debugging.<div><br></div><div>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!</div><div><br></div><div>-Todd</div></div><div class="gmail_extra"><div><div><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 12:34 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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:<div><br></div><div><div>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</div><div>411 out of 412 test suites processed - TestIntegerTypesExpr.py</div><div>Traceback (most recent call last):</div><div> File "D:\src\llvm\tools\lldb\test\dotest.py", line 7, in <module></div><div> lldbsuite.test.run_suite()</div><div> File "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\dotest.py", line 1476, in run_suite</div><div> setupTestResults()</div><div> File "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\dotest.py", line 982, in setupTestResults</div><div> results_formatter_object.handle_event(initialize_event)</div><div> File "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\test_results.py", line 1033, in handle_event</div><div> "{}#{}".format(len(pickled_message), pickled_message))</div><div>TypeError: a bytes-like object is required, not 'str'</div></div></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 11:40 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 11:32 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 11:26 AM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 2, 2015 at 11:20 AM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Yeah I'd be good with that. I can change that as well.<div><br></div><div>-Todd</div></div><div class="gmail_extra"><div><div><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 11:10 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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 <span style="font-family:monospace,monospace">lldbsuite.test.basic_results_</span><span style="font-family:monospace,monospace">formatter.</span><span style="font-family:monospace,monospace">BasicResultsFormatter </span>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:<div><br></div><div><span style="font-family:monospace,monospace">test/dotest.py --results-formatter basic</span><br></div><div><span style="font-family:monospace,monospace"><br></span></div><div>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.<span style="font-family:monospace,monospace"><br></span></div><div><br></div><div>This has the advantage of making the command line shorter *and* a more logical source file organization.</div></div></blockquote></div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.) </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 11:04 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Can --results-file=stdout be the default so that we don't have to specify that?</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 2, 2015 at 11:02 AM Todd Fiala via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 11:01 AM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Wed, Dec 2, 2015 at 10:57 AM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi all,<div><br></div><div>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:</div><div><br></div><div>r254530<br clear="all"><div><br></div><div>and you can try it out with something like:</div><div><br></div><div><div>time test/dotest.py --executable `pwd`/build/Debug/lldb --results-formatter lldbsuite.test.basic_results_formatter.BasicResultsFormatter --results-file st</div><div>out</div></div><div><br></div></div></div></blockquote><div><br></div>
</span><p><span>I cut and paste my line, but more than likely for most people you'd just want this:</span></p>
<p><font face="monospace, monospace">test/dotest.py --results-formatter lldbsuite.test.basic_results_formatter.BasicResultsFormatter --results-file stdout <br><span></span></font></p><p><font face="monospace, monospace">The other stuff was specific to my setup. That line assumes you run from the lldb source dir root.</font></p><span><p><font face="monospace, monospace"><br></font></p><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div></div><div>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.</div><div><br></div><div>It prints out the Details section when there are details, and keeps it nice and clean when there are none.</div><div><br></div><div>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.</div><div><br></div><div>The change also cleans up places where the test event framework was using string codes and replaces them with symbolic constants.</div><div><br></div><div>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.</div><div><br></div><div>Thanks!</div><span><font color="#888888">-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div></div>
</blockquote></span></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</div>
_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
</blockquote></div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div>
</blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</div></div></blockquote></div></blockquote></div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div>
</blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div><div dir="ltr">-Todd</div></div>
</div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div>
</blockquote></div></div></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div><div dir="ltr">-Todd</div></div>
</div></blockquote></div></blockquote></div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div><div dir="ltr">-Todd</div></div>
</div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">-Todd</div></div>
</div></div>