Bug 36474

Summary: Add support for revision -> build lookup in buildbot.py and layout test result parsing
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch abarth: review+, abarth: commit-queue-

Eric Seidel (no email)
Reported 2010-03-22 18:26:28 PDT
Add support for revision -> build lookup in buildbot.py and layout test result parsing
Attachments
Patch (11.64 KB, patch)
2010-03-22 18:26 PDT, Eric Seidel (no email)
no flags
Patch (17.37 KB, patch)
2010-03-23 01:19 PDT, Eric Seidel (no email)
abarth: review+
abarth: commit-queue-
Eric Seidel (no email)
Comment 1 2010-03-22 18:26:52 PDT
Eric Seidel (no email)
Comment 2 2010-03-23 01:19:41 PDT
Ojan: With this added code it is now trivial to convert existing results.html files from the buildbot into the results.json files which the layout test dashboard supports.
Eric Seidel (no email)
Comment 3 2010-03-23 01:19:59 PDT
Adam Barth
Comment 4 2010-03-23 13:43:31 PDT
We need to abstract out the calls to urllib2.urlopen. We keep using the same idiom to surround them. We could include the 404 handling and caching as optional parameters. + _fetch_layout_test_results I'd prefer if all control paths had explicit returns. + _parse_twisted_file_row This twisted stuff should be in a separate file. It's not specific to buildbot. - _longest_builder_name Why did we get rid of this function? It seemed helpful to document what that complex line is doing.
Eric Seidel (no email)
Comment 5 2010-03-23 14:22:07 PDT
(In reply to comment #4) > + _fetch_layout_test_results > > I'd prefer if all control paths had explicit returns. Ludite! :) I think it's actually more readable to not add the "return None" since that's implied in python. > + _parse_twisted_file_row > > This twisted stuff should be in a separate file. It's not specific to > buildbot. Agreed. The functions started out as specific to this task, but I realized they could be made general. i'm not sure where we'd put any other "twisted" stuff, but I named them so that they would be easy to recognize as general. > - _longest_builder_name > > Why did we get rid of this function? It seemed helpful to document what that > complex line is doing. I had to copy the code. I realized it was a realatively simple one-liner and didn't have any good place to put that funciton, so I just inlined it in both places. I think I'm just gonna land as is, but I'm happy to make further changes upon further discussion with you.
Eric Seidel (no email)
Comment 6 2010-03-23 15:24:52 PDT
Note You need to log in before you can comment on or make changes to this bug.