Polymer-based Garden-O-Matic should have a basic results panel

Polymer-based Garden-O-Matic should have a basic results panel

This CL makes it possible to examine failures on Sheriff-O-Matic (the Polymer
version of Garden-O-Matic). Currently, the results panel just lets you select
which test and builder you're interested in. It's not able to display the
actual test results yet.

NOTRY=true

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

[abarth-chromium, 2014-06-29 09:07:23]

This CL has a bunch more of the component structure, but it doesn't have any
tests yet.  Mostly I'm uploading this CL to checkpoint my work for tonight.

[michaelpg, 2014-06-29 23:16:05]

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
Where does this data come from? I'm looking at ct-commit-list.html:

<ct-commit-data first="{{first}}" last="{{last}}"
data="{{commitData}}"></ct-commit-data>

but don't see when the commitData property is set on the element.

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:38:
last="{{failures[0].oldestFailingRevision}}"></ct-commits>
</ct-commit-list>

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-test-list.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-test-list.html:7: <link rel="import"
href="ct-commit-list.html">
Do you need this?

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-test-list.html:12: a {
nit: indent 2 spaces

[michaelpg, 2014-06-29 23:49:19]

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
On 2014/06/29 23:16:05, Michael Giuffrida wrote:
> Where does this data come from? I'm looking at ct-commit-list.html:
> 
> <ct-commit-data first="{{first}}" last="{{last}}"
> data="{{commitData}}"></ct-commit-data>
> 
> but don't see when the commitData property is set on the element.

ok, answering my own question: commitData is set (by polymer) when "this.data"
is updated in ct-commit-data.html because binding is two-way.

[abarth-chromium, 2014-06-29 23:52:26]

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-commit.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
Yep

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-test-list.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-test-list.html:7: <link rel="import"
href="ct-commit-list.html">
Nope!  Copy/pasta

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-test-list.html:12: a {
Done

[abarth-chromium, 2014-06-29 23:53:42]

Ok, I added some tests.  This should be ready to review now.  Thanks!!

[abarth-chromium, 2014-06-29 23:54:19]

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-results-detail.html (right):

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-results-detail.html:9: <polymer-element
name="ct-results-detail" attributes="test builder">
No tests for this element yet because this version is just a placeholder.  :)

[michaelpg, 2014-06-29 23:58:32]

Sorry for the email spamming, I've just been catching things as I try to get
this up and running on my machine.

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:38:
last="{{failures[0].oldestFailingRevision}}"></ct-commits>
first and last are reversed (or, they need to be reversed in
ct-commit-data.html::update_()) - this passes an invalid revision range to the
model which is why no CLs are showing up, at least for me (Do the tests pass
with this?)

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:42: failures: [],
The Polymer docs emphasize that object and array members shouldn't be
initialized on the prototype, but in the create method. But throughout this
project we've been initializing them on the prototype. Does that matter here? It
seems like we mostly re-set the objects/arrays on each element in an "update"
method so it works.

[abarth-chromium, 2014-06-30 00:43:29]

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:38:
last="{{failures[0].oldestFailingRevision}}"></ct-commits>
Why are they reversed?  The first (earliest) commit we want to show is the one
just after the last known passing revision and the last (latest) commit we want
to see is the oldest known failing revision:

ppppffffffffffff
 ^    ^

Where "p" means pass, "f" means fail, and the ^ indicate revisions that we have
observations about.  The commits we want to display start with the one to the
right of the first ^ and stop at the second ^.  The revision that caused the
failure is included in that range.

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/80001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:42: failures: [],
It doesn't actually matter because we don't mutate them.  Python as the same
footgun.

[michaelpg, 2014-06-30 02:32:38]

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:38:
last="{{failures[0].oldestFailingRevision}}"></ct-commits>
Sorry, you're totally right. What I was actually seeing was just
newestPassingRevision == oldestFailingRevision so that's why no CLs were
grabbed.

[eseidel, 2014-06-30 04:30:15]

rslgtm.  forward ho!

[ojan, 2014-06-30 04:48:14]

lgtm as well. just a few nits.

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/r...
File Tools/GardeningServer/run-unittests.html (right):

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/r...
Tools/GardeningServer/run-unittests.html:83: <link rel="import"
href="ui/ct-test-list-tests.html">
Yay tests!

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:14: padding: 5px 10px 5px 10px;
nit: padding: 5px 10px;

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
These filters are so great!

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-card.html:18: min-height: 30px;
Nit: Why the min-height?

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-stream-tests.html:29: start();
This testing framework is so silly. Hopefully we can switch to mocha soon.

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-results-detail.html:19: <script>
You don't need this, right? You can give the polymer-element a noscript
attribute instead.

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-test-list.html:15: <a
href="{{testName|flakinessDashboardURL}}">{{testName}}</a>
Do you really want clicking anywhere on the block to navigate? I think you want
to do:
<div>
  <a ...>...</a>
</div>

And remove the a { display: block }.

[abarth-chromium, 2014-06-30 04:55:06]

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:14: padding: 5px 10px 5px 10px;
Done

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
Good frameworks are good.

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-card.html:18: min-height: 30px;
Otherwise the Examine button overhangs the next entry.  It's floating left, so
another solution is to use |clear|.

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-stream-tests.html:29: start();
Go for it

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-results-detail.html:19: <script>
Yes.  I had more stuff in here, but I decided to stop at this point.  Fixed.

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-test-list.html:15: <a
href="{{testName|flakinessDashboardURL}}">{{testName}}</a>
Ok

[ojan, 2014-06-30 04:57:22]

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

https://codereview.chromium.org/349783003/diff/100001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-card.html:18: min-height: 30px;
On 2014/06/30 04:55:05, abarth wrote:
> Otherwise the Examine button overhangs the next entry.  It's floating left, so
> another solution is to use |clear|.

Clear is better. More explicit what it's there for and less hard-coded. This
won't work right if you, e.g., zoom the page.

That said, don't use floats for this. Use flexbox.

[abarth-chromium, 2014-06-30 05:06:51]

On 2014/06/30 at 04:57:22, ojan wrote:
> That said, don't use floats for this. Use flexbox.

Done.

I also got rid of the silly hover effect and used papayawhip instead.  Yay for
material design embracing lots of crazy colors.

[abarth-chromium, 2014-06-30 05:07:01]

The CQ bit was checked by abarth@chromium.org

[commit-bot: I haz the power, 2014-06-30 05:07:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/abarth@chromium.org/349783003/140001

[commit-bot: I haz the power, 2014-06-30 05:11:52]

Change committed as 177189

[esprehn, 2014-07-01 19:47:45]

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

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-commit.html:35: <a
href="{{data.revision|changesetURL}}">{{data.revision}}</a> {{data.summary}}
<em>{{data.author}}</em>
It's usually nice to put spaces around the pipe since it's easier to read.

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-card.html:24: <div style="flex: 1">
I wouldn't use inline styles, if you want to go all polymer like there's a
"flex" attribute you can add that does this.

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-failure-card.html:26: <ct-commit-list
first="{{failures[0].newestPassingRevision + 1}}"
This is weird, why do you only care about the first entry?

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-results-by-builder.html (right):

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-results-by-builder.html:33: this.builders =
Object.getOwnPropertyNames(this.failure.resultNodesByBuilder);
Object.keys()

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-results-detail.html (right):

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-results-detail.html:16: Test: {{test}}<br>
Ick, can we use a <div> instead of a <br>?

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
File Tools/GardeningServer/ui/ct-sheriff-o-matic.html (right):

https://codereview.chromium.org/349783003/diff/140001/Tools/GardeningServer/u...
Tools/GardeningServer/ui/ct-sheriff-o-matic.html:49: this.currentFailures =
event.detail;
It's nicer to pass an object as the detail so you can expand this later without
changing callers.

event.detail.failures or something