Bug 36474 – Add support for revision -> build lookup in buildbot.py and layout test result parsing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Add support for revision -> build lookup in buildbot.py and layout test result parsing

[Eric Seidel (no email)]

Created attachment 51376 [details]
Patch

[Eric Seidel (no email)]

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)]

Created attachment 51401 [details]
Patch

[Adam Barth]

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)]

(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)]

Committed r56421: <http://trac.webkit.org/changeset/56421>