Get sheriff-o-matic data from auto-sheriff.appspot.com.

Get sheriff-o-matic data from auto-sheriff.appspot.com.

This moves the logic of which tests are failing and their
regression ranges to the server. This makes the tool much
faster to load. Having it on the server makes it so that
multiple tools can use this data.

Also, the data on the server has all the data we need for
non-layout test steps, so extending sheriff-o-matic will
be very easy once we have this in place.

NOTRY=true
R=abarth@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178597

[ojan, 2014-07-18 02:17:37]

This isn't ready. It needs all the TODO(ojan) fixed, tests and probably some
other assorted cleanup. But since some of you asked, I thought I'd put it up so
you could see where I'm heading with this.

There's a lot of improvement we can make once we land this, but I wanted to keep
this change (relatively) simple and not mess with any UI changes or anything.

[ojan, 2014-07-18 21:30:28]

Eric commented that this is a lot of code...that's because I felt like I needed
to do some refactoring to make this change not terrible. I'll split off the
refactoring into smaller patches once it's ready.

[ojan, 2014-07-19 02:18:41]

Rebased to https://codereview.chromium.org/404463003. But, I can't test it at
the moment because auto-sheriff.appspot is down. I'd like to make this change as
is (once I've verified it works) to keep this change as minimal as possible.
Then in a followup patch turn failure-analyzer into a Failures (or something)
model object and write tests for it.

[eseidel, 2014-07-20 05:57:35]

auto-sheriff is back (and updating now), sorry for the outage.

On Fri, Jul 18, 2014 at 7:18 PM,  <ojan@chromium.org> wrote:
> Rebased to https://codereview.chromium.org/404463003. But, I can't test it
> at
> the moment because auto-sheriff.appspot is down. I'd like to make this
> change as
> is (once I've verified it works) to keep this change as minimal as possible.
> Then in a followup patch turn failure-analyzer into a Failures (or
> something)
> model object and write tests for it.
>
> https://codereview.chromium.org/402603007/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[eseidel, 2014-07-20 07:25:57]

Sorry.  Still down.  Will fix in AM.

On Sat, Jul 19, 2014 at 10:57 PM, Eric Seidel <eseidel@chromium.org> wrote:
> auto-sheriff is back (and updating now), sorry for the outage.
>
> On Fri, Jul 18, 2014 at 7:18 PM,  <ojan@chromium.org> wrote:
>> Rebased to https://codereview.chromium.org/404463003. But, I can't test it
>> at
>> the moment because auto-sheriff.appspot is down. I'd like to make this
>> change as
>> is (once I've verified it works) to keep this change as minimal as possible.
>> Then in a followup patch turn failure-analyzer into a Failures (or
>> something)
>> model object and write tests for it.
>>
>> https://codereview.chromium.org/402603007/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[eseidel, 2014-07-20 19:38:47]

Finally actually fixed.  I'll work on getting it into src.chromium.org
this week.

On Sun, Jul 20, 2014 at 12:25 AM, Eric Seidel <eseidel@chromium.org> wrote:
> Sorry.  Still down.  Will fix in AM.
>
> On Sat, Jul 19, 2014 at 10:57 PM, Eric Seidel <eseidel@chromium.org> wrote:
>> auto-sheriff is back (and updating now), sorry for the outage.
>>
>> On Fri, Jul 18, 2014 at 7:18 PM,  <ojan@chromium.org> wrote:
>>> Rebased to https://codereview.chromium.org/404463003. But, I can't test it
>>> at
>>> the moment because auto-sheriff.appspot is down. I'd like to make this
>>> change as
>>> is (once I've verified it works) to keep this change as minimal as
possible.
>>> Then in a followup patch turn failure-analyzer into a Failures (or
>>> something)
>>> model object and write tests for it.
>>>
>>> https://codereview.chromium.org/402603007/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2014-07-21 00:44:57]

This is now ready for review. Still depends on
https://codereview.chromium.org/404463003 though.

This patch looks huge and daunting, but isn't so bad if you collapse away the
deleted files.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
File Tools/GardeningServer/model/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
Tools/GardeningServer/model/ct-commit.html:8: function CTCommit(author, message,
baseUrl) {
I can split these changes out to a different patch if it's too confusing. This
is just moving code from svn-log.js to here. Making this change allowed for
deleting all of svn-log.js.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-builder-grid.html (left):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-builder-grid.html:133: if (!result.is_unexpected)
With the new backend, we only have data for unexpected failures, so there's no
need for this check. All failures are unexpected.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:35: <a href="{{ _url }}">{{
data.revision }}</a> {{ data.summary }} <em>{{ data.author }}</em>
This goes along with the model/ct-commit.html change.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-failure-analyzer.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-analyzer.html:7: <polymer-element
name="ct-failure-analyzer" attributes="failures builderLatestRevisions">
The changes in this file are the only substantive parts of this patch that could
use real review. In a followup patch, we'll turn this whole component into a
model class and write tests for it.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (left):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-unexpected-failures.html:29: <ct-failure-grouper
failures="{{ failures.unexpected }}" groups="{{ failureGroups
}}"></ct-failure-grouper>
The grouping happens on the backend now, so no need for a dedicated grouper.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-unexpected-failures.html:34: <paper-toast id="toast"
text="{{ analyzerStatus }}"></paper-toast>
No more need for a toast anymore since we load so quickly.

[abarth-chromium, 2014-07-21 06:44:18]

This looks like a great start.  There are still a bunch of things to polish, but
we can do that in later CLs.  This is a huge step.  LGTM

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
File Tools/GardeningServer/model/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
Tools/GardeningServer/model/ct-commit.html:16: CTCommit.prototype.url =
function() {
Getter?

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
Tools/GardeningServer/model/ct-commit.html:25: CTCommit._queryParam =
function(params) {
Object.toQueryString from sugar

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:43: },
Lame

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-failure-analyzer.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-analyzer.html:50: var parts =
failure.reason.split(':');
Seems like we should do this work in Python.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (left):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-unexpected-failures.html:29: <ct-failure-grouper
failures="{{ failures.unexpected }}" groups="{{ failureGroups
}}"></ct-failure-grouper>
Neat

[ojan, 2014-07-21 22:46:14]

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
File Tools/GardeningServer/model/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
Tools/GardeningServer/model/ct-commit.html:16: CTCommit.prototype.url =
function() {
I really dislike getters. They make for surprising code because you never expect
a getter to be doing a bunch of work. I'm torn in this case because getters let
you interface better with MDV. I'll switch it over. I don't even know the right
syntax for getters...is Object.defineProperty the right way to set a getter on
the prototype?

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/m...
Tools/GardeningServer/model/ct-commit.html:25: CTCommit._queryParam =
function(params) {
Not sure how I missed that.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:43: },
Now that url is a getter, this goes away. Make this into a noscript component.

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-failure-analyzer.html (right):

https://codereview.chromium.org/402603007/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-analyzer.html:50: var parts =
failure.reason.split(':');
Yeah. Eric didn't want to do that for some reason. We ended up deciding we'd
just do this for now and we can always change it later. Particularly, in the
future I think we'll want more information (e.g. the expected failure types).

[ojan, 2014-07-21 22:46:16]

Committed patchset #7 manually as r178597 (tree was closed).

[eseidel, 2014-07-21 23:04:09]

The reason I added test_name:type instead of making it a dict was that if I made
it a dict I had to solve the name != key problem.  Right now I use test_name
(really "reason") as the key when grouping failures.  I can separate those two. 
But I'd have to answer the qeustion of if type was part of the key or not.  I
embedded it in the name, thus embedding it in the key and punted on that problem
for now.

[ojan, 2014-07-21 23:06:24]

On 2014/07/21 at 23:04:09, eseidel wrote:
> The reason I added test_name:type instead of making it a dict was that if I
made it a dict I had to solve the name != key problem.  Right now I use
test_name (really "reason") as the key when grouping failures.  I can separate
those two.  But I'd have to answer the qeustion of if type was part of the key
or not.  I embedded it in the name, thus embedding it in the key and punted on
that problem for now.

Punting is totally fine for now. FWIW, I think the desired behavior is to use
just the test name as the key when grouping failures.