| 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 Bugs | Assignee: | 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
Eric Seidel (no email)
2010-03-22 18:26:28 PDT
Created attachment 51376 [details]
Patch
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. Created attachment 51401 [details]
Patch
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. (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. Committed r56421: <http://trac.webkit.org/changeset/56421> |