Bug 76011 – last-green-revision should give us per-bot information

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ryosuke Niwa]

last-green-revision has been returning None for a while and isn't really useful because there are always some failing bots. It's much more useful if it provided such information per-bot.

[Adam Barth]

Do you want is to tell you about all the bots, or do you want to be able to query it per bot?

[Ryosuke Niwa]

(In reply to comment #1)
> Do you want is to tell you about all the bots, or do you want to be able to query it per bot?

Query per bot. e.g. if I ask last-green-revision "Chromium Win" then it should tell me when "Chromium Win Release" was green and also when "Chromium Win Release Tests" was green.

[Ryosuke Niwa]

Created attachment 121957 [details]
Patch

[Ryosuke Niwa]

I guess we can extend it so that it tries to find the revision for which all bots succeeded first and then fallbacks to this per-bot version when that fails.

[Adam Barth]

Comment on attachment 121957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121957&action=review

> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:436
> +        builder_page_url = "%s/builders/%s?numbuilds=100" % (self.buildbot_url, builder.name().replace(' ', '%20'))

replace(' ', '%20') => does we need to use some more general URL encoding function?

> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:443
> +        for status_row in soup.find('table').findAll('tr'):

Should we be using the JSON API instead?

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:60
> +        return "%s: %s" % (nick, urls.view_revision_url(tool.buildbot.last_green_revision(args[0])))

view_revision_url still right?  last_green_revision seems to return a string now, not a revision number.

[Ryosuke Niwa]

Comment on attachment 121957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121957&action=review

Thanks for the review.

>> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:436
>> +        builder_page_url = "%s/builders/%s?numbuilds=100" % (self.buildbot_url, builder.name().replace(' ', '%20'))
> 
> replace(' ', '%20') => does we need to use some more general URL encoding function?

urllib2.quote?

>> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:443
>> +        for status_row in soup.find('table').findAll('tr'):
> 
> Should we be using the JSON API instead?

I don't think build.webkit.org has JSON API. We could add those if we're so inclined.

>> Tools/Scripts/webkitpy/tool/bot/irc_command.py:60
>> +        return "%s: %s" % (nick, urls.view_revision_url(tool.buildbot.last_green_revision(args[0])))
> 
> view_revision_url still right?  last_green_revision seems to return a string now, not a revision number.

Oh oops, yeah, I shouldn't be calling urls.view_revision_url here.

[Adam Barth]

(In reply to comment #6)
> (From update of attachment 121957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121957&action=review
> 
> >> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:443
> >> +        for status_row in soup.find('table').findAll('tr'):
> > 
> > Should we be using the JSON API instead?
> 
> I don't think build.webkit.org has JSON API. We could add those if we're so inclined.

Oh, it totally does:

http://build.webkit.org/json/builders/

I'm not sure what all is there, but I bet this information is there somewhere.

[Ryosuke Niwa]

Comment on attachment 121957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121957&action=review

>>>> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:443
>>>> +        for status_row in soup.find('table').findAll('tr'):
>>> 
>>> Should we be using the JSON API instead?
>> 
>> I don't think build.webkit.org has JSON API. We could add those if we're so inclined.
> 
> Oh, it totally does:
> 
> http://build.webkit.org/json/builders/
> 
> I'm not sure what all is there, but I bet this information is there somewhere.

Unfortunately it doesn't provide the information I need in an efficient manner. I'll have to make 100 http requests to get the status on 100 runs :(

[Adam Barth]

Ok

[Ryosuke Niwa]

Committed r104729: <http://trac.webkit.org/changeset/104729>

[Emil A Eklund]

This is a good start but limiting it to the last 100 runs is unfortunate. Anyway, thanks for doing this.