WIP: Add auto-sheriff.appspot.com code to Blink

WIP: Add auto-sheriff.appspot.com code to Blink

auto-sheriff was written as an experiment and its
code kept on github:
https://github.com/eseidel/cycletimes/tree/master/nannybot

It's now about to be used in production as the new
back-end for garden-o-matic and should live in one
of chromium's repositories.

Unclear if this belongs here or infra or src or whatever
but posting this to Blink for now since this is where
GOM is.

This change includes copies of three files from build/
gatekeeper.json
gatekeeper_ng_config.py
gtest_utils.py

We should find other ways to use those without
copying them to Blink.

BUG=

[ojan, 2014-07-22 02:01:28]

<darth-vader>I find your lack of tests disturbing.</darth-vader> :)

The most important thing is landing this so that we can iterate on it. It's
really hard to review such a huge chunk of code though. I mostly just gave a
bunch of style nits. Feel free to make anything FIXMEs in the name of getting
this landed sooner rather than later and then doing the FIXMEs asynchronously.

At a high level, I think we should integrate auto-sheriff and sheriff-o-matic
into one server. May as well do that in the first commit? Although that'd
require figuring out how the files should fit in to the GardeningServer
heirarchy, so maybe we should land this and move in that direction
incrementally.

Also, I probably would scale back the UI code considerably. I'd make it just a
simple visualization of the JSON + a few tweaks to make it easier to make sense
of. Otherwise, this UI code is just going to bitrot over time since the real UI
people will be using will be sheriff-o-matic.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/README.md
File Tools/AutoSheriff/README.md (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/README.md#...
Tools/AutoSheriff/README.md:9: Directories with third_party in their name may
have their own (compatible) licensing.
Can we get these things with bower instead? Chromium has all sort of policies
around checking in third party code.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/README.md#...
Tools/AutoSheriff/README.md:16: TODO:
I wouldn't put this and the below here.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis.py
File Tools/AutoSheriff/analysis.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis.p...
Tools/AutoSheriff/analysis.py:10: # Git ready, but only implemented for SVN atm.
I think we should only operate on monotonically increasing integer values. My
understanding is that even after the git migration, we'll have such a value for
each commit. We should make it the job of the infrastructure that this is built
upon to expose those values in addition to or instead of the git hash.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis.p...
Tools/AutoSheriff/analysis.py:123: def longestSubstringFinder(string1, string2):
s/camelCase/camel_case

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis.p...
Tools/AutoSheriff/analysis.py:149: # exact matching revisions when failing.
I'm not sure it's wrong to group these together in this case. They have the same
regression range. I guess you eventually want to be able to associate a "note"
with each group for when the sheriff wants to annotate, in which case, not
grouping makes sense.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis_u...
File Tools/AutoSheriff/analysis_unittest.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/analysis_u...
Tools/AutoSheriff/analysis_unittest.py:80: passing = { 'v8': '1', 'chromium':
'4'}
Inconsistent spacing here and in a few places below.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/buildbot.py
File Tools/AutoSheriff/buildbot.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/buildbot.p...
Tools/AutoSheriff/buildbot.py:15: # Python logging is stupidly verbose to
configure.
Not really a useful comment.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/buildbot.p...
Tools/AutoSheriff/buildbot.py:160: # Don't even think about using 'revision'
that's wrong too.
Lol!

This should perhaps be a FIXME? Theoretically, we should fix the stupidity
eventually.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.html
File Tools/AutoSheriff/closers.html (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.ht...
Tools/AutoSheriff/closers.html:1: <!doctype html>
Nit: We usually use <!DOCTYPE html> to match the official html doctype.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.ht...
Tools/AutoSheriff/closers.html:8: <script
src="scripts/third_party/js_humanized_time_span/humanized_time_span.js"></script>
We've been standardizing on polymer+sugarjs for all our code. I think sugarjs
gives you human readable dates.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.ht...
Tools/AutoSheriff/closers.html:10: html,body {
Nit: need space after the comma

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.ht...
Tools/AutoSheriff/closers.html:18: <script src="ui/filters.js"></script>
If you use script tags, then your load order isn't guaranteed. The thing we're
standardizing on is to *always* use html imports and just have an HTML file that
only contains an inline script. Also, that way you can include-what-you-use. No
need to include filters.js way up here where it's not used. The components that
use it can include it directly.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/closers.ht...
Tools/AutoSheriff/closers.html:55: this.$.loader.go();
We've been moving away from using core-ajax. This everything is a DOM node thing
seems to cause more pain than gain. For example, if you were to use
GardeningServer/scripts.net.js, then this chunk of code would just be the
following:
net.json(function(response) {
  this.failures = response.alerts;
  this.filteredFailures = this.failuresForTree(this.selectedTree);
}.bind(this));

It's more straightforward code and doesn't involve a ton of complex machinery.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py
File Tools/AutoSheriff/feeder.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:34: def setup_logging():
Move this to a shared file instead of copy-pasting it?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:56: '''Returns last_pass_build, first_fail_build,
fail_count'''
Meh. This comment doesn't tell me anything more than the last line of the
function.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:155: last_pass_build, first_fail_build, fail_count =
\
Nit: tc tells me that idiomatic python is to use parens here.
last_pass_build, first_fail_build, fail_count = (
    compute_transition_and_failure_count(failure, build, recent_builds))

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:183: recent_build_ids = recent_build_ids[:100]
Move this into a constant at the top?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:224: # Reduce
Did you mean to leave these comments in? It's not clear that this is telling me.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:234: # Unclear if this should be set or not?
FIXME?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:241: alert['would_close_tree'] = \
Ditto re: parens

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/feeder.py#...
Tools/AutoSheriff/feeder.py:300: data = { 'content': json.dumps({
Nit: I'd make this more verbose just to make it easier to read.
{
  'content: json.dumps({
    'alerts': alerts,
    ...
  }),
}

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
File Tools/AutoSheriff/gatekeeper.json (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
Tools/AutoSheriff/gatekeeper.json:1: {
Can we put these three files into a copypaste subdirectory with a README or
something to make it clear that they need to be gotten from elsewhere?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
File Tools/AutoSheriff/gatekeeper_extras.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
Tools/AutoSheriff/gatekeeper_extras.py:30: # http://crbug.com/394961
We should change the flakiness dashboard concept of groups to match this list,
and eventually, we should get this list from one place of course.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
File Tools/AutoSheriff/gatekeeper_ng_config.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/gatekeeper...
Tools/AutoSheriff/gatekeeper_ng_config.py:5: 
This should have a FIXME to get the file from elsewhere, right?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/main.py
File Tools/AutoSheriff/main.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/main.py#ne...
Tools/AutoSheriff/main.py:1: import webapp2
Copyright

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/main.py#ne...
Tools/AutoSheriff/main.py:24: pattern = ndb.StringProperty(indexed=False)
Will StringProperty be long enough?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/nannybot.html
File Tools/AutoSheriff/nannybot.html (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/nannybot.h...
Tools/AutoSheriff/nannybot.html:1: <!doctype html>
Ditto all the comments on the previous html file.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/nannybot.h...
Tools/AutoSheriff/nannybot.html:7: <script
src="scripts/repositories.js"></script>
Ditto: re using html imports for scripts.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/nannybot.h...
Tools/AutoSheriff/nannybot.html:33: <div>This is a debugging view for building a
global-failures datastream (<a href='/data'>/data</a>) off of which we can build
tools to automatically maintain Chromium bots.  <a
href='https://github.com/eseidel/cycletimes/tree/master/nannybot'>Source<a></div>
Nit: please indent all this stuff to show nesting.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py
File Tools/AutoSheriff/reasons.py (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:68: KNOWN_STEPS = [
I'd leave this out for now. Can always add it back in when we decide we need it.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:141: flakes[test] = actual_results[0]
Can we just store result['actual'] here? That way the consuming tool can decide
that it wants to do with the result. Also, some test suites retry multiple
times.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:145: failures[test] = actual_results[0]
Ditto

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:182: # print json.dumps(build['steps'], indent=1)
Delete dead code?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:195: # print json.dumps(archive_step, indent=1)
Ditto

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:198: # !@?#!$^&$% WTF HOW DO URLS HAVE \r in
them!?!
Wat

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:202: # FIXME: Silly that this is still JSONP.
This is because failing_results.json is used by results.html to show you the
failures when you run locally and chrome doesn't let you XHR files from file
urls. Open to suggestions to how to do it better. We could output two files from
run-webkit-tests I guess. FWIW, full_results.json is pure JSON. No P.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:227: # Compile example:
Even better than examples in comments is examples in tests. :)

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:238: if not stdio:
Should we log an error or something here?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/reasons.py...
Tools/AutoSheriff/reasons.py:287: # GenericRunTests(),
Should we delete this if we're going to comment it out?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/scripts/re...
File Tools/AutoSheriff/scripts/repositories.js (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/scripts/re...
Tools/AutoSheriff/scripts/repositories.js:2: 
Add a FIXME to move this somewhere shared? We'll need this in a bunch of places.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/scripts/re...
Tools/AutoSheriff/scripts/repositories.js:42: for (var i = 0; i <
registry.length; ++i) {
If you had sugarjs you could use registery.find here. :)

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters.js
File Tools/AutoSheriff/ui/filters.js (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:1: // Needed to make filters visible inside
sortable-table.
Copyright

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:2: PolymerExpressions.prototype.since_string =
function(value) {
Why do you need to do this? Can't you define a since_string function on your
PolymerElement and then use that as your filter? That's the pattern we've been
using in sheriff-o-matic. It avoids monkey-patching polymer and depending on
global functions.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:8: return value ? value : '';
Nit: Idiomatic JS for this is:
return value || '';

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:12: return 'foo';
Log a warning and return something better?

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:35: var MASTERS = [
This is fine for now, but add a FIXME to use test-results.appspot.com/builders
or otherwise get rid of this code.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/filters...
Tools/AutoSheriff/ui/filters.js:72: + "&group=" + groupForMaster(master_url);
This also needs to be encoded. FWIW, with sugarjs this would be:
return "http://test-results.appspot.com/dashboards/flakiness_dashboard.html#" +
    Object.toQueryString({
      testType: step_name,
      tests: test_name,
      group: groupForMaster(master_url),
    });

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
File Tools/AutoSheriff/ui/nb-alert-list.html (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:1: <polymer-element name="nb-alert-list"
attributes='failures'>
Copyright

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:1: <polymer-element name="nb-alert-list"
attributes='failures'>
Copyright. Here and in all the files below

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:6: .fail_link { background-color: #FCC;}
Nit: we've been using fail-link and pass-link style for classnames and IDs.
Although, I suppose that's not awesome since you can't put a - in a variable
name. :(

Also, inconsistent spacing.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:9: <td>{{record.row.master_url |
master_name }}</td>
Inconsistent spacing. FWIW, we've been standardizing on {{ thingy }}.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:62: '$.table.sortColumn':
'sortColumnChanged'
Indentation is off.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-aler...
Tools/AutoSheriff/ui/nb-alert-list.html:64: sortColumnChanged:
function(oldValue, newValue) {
Delete this code. Can always add it later.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-chan...
File Tools/AutoSheriff/ui/nb-changelogs.html (right):

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-chan...
Tools/AutoSheriff/ui/nb-changelogs.html:5: <template if="{{name |
commits_differ(passing, failing) }}">
Spacing.

https://codereview.chromium.org/398823008/diff/1/Tools/AutoSheriff/ui/nb-chan...
Tools/AutoSheriff/ui/nb-changelogs.html:11: before
Indentation.

[szager1, 2016-06-13 21:28:47]

Description was changed from

==========
WIP: Add auto-sheriff.appspot.com code to Blink

auto-sheriff was written as an experiment and its
code kept on github:
https://github.com/eseidel/cycletimes/tree/master/nannybot

It's now about to be used in production as the new
back-end for garden-o-matic and should live in one
of chromium's repositories.

Unclear if this belongs here or infra or src or whatever
but posting this to Blink for now since this is where
GOM is.

This change includes copies of three files from build/
gatekeeper.json
gatekeeper_ng_config.py
gtest_utils.py

We should find other ways to use those without
copying them to Blink.

BUG=
==========

to

==========
WIP: Add auto-sheriff.appspot.com code to Blink

auto-sheriff was written as an experiment and its
code kept on github:
https://github.com/eseidel/cycletimes/tree/master/nannybot

It's now about to be used in production as the new
back-end for garden-o-matic and should live in one
of chromium's repositories.

Unclear if this belongs here or infra or src or whatever
but posting this to Blink for now since this is where
GOM is.

This change includes copies of three files from build/
gatekeeper.json
gatekeeper_ng_config.py
gtest_utils.py

We should find other ways to use those without
copying them to Blink.

BUG=
==========