Fix partytime.

Fix partytime.

My last change to it made it not work if there were no failures
for a tree because failures[tree] would be undefined. Make it
less brittle, add tests for this code and while here add an
explanatory message that this page means there are no failures.

BUG=407496
NOTRY=true
R=jyasskin@chromium.org

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

[ojan, 2014-09-01 00:29:31]

ojan@chromium.org changed reviewers:
+ jyasskin@chromium.org, leviw@chromium.org, michaelpg@chromium.org

[ojan, 2014-09-01 00:31:15]

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/test/ct-failures-tests.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/test/ct-failures-tests.html:9: <link rel="import"
href="../ct-commit-list.html">
This import was missing. Didn't break anything, but include-what-you-use.

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:46: <ct-failure-stream
category="snoozed" groups="{{ failures && failures.failures[tree] }}"
commitLog="{{ commitLog }}" tree="{{ tree }}"></ct-failure-stream>
These changes are to make createElement(ct-unexpected-faulures) not throw.

[Jeffrey Yasskin, 2014-09-02 18:34:53]

lgtm

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:44: <ct-party-time
partytime="{{ !failures || !failures.failures[tree] ||
failures.failures[tree].length == 0 }}"></ct-party-time>
I keep assuming that Polymer's templates parse the paths they use and handle
'undefined' anywhere along it. :(

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-results-panel-tests.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-results-panel-tests.html:102: // also means
that we actually fetch requests. This results in some console
"fetch requests"? Maybe "fetch the alerts page"?

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html:37:
failures.failures.failures = {
Hmmm. Can you avoid having the same name 3 deep?

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html:43:
assert.ok(noFailures.shadowRoot.querySelector('ct-party-time').partytime);
Style-wise, I'd probably split this into 4 different tests. I certainly won't
insist, and I see that it's more syntactic overhead to do that, but I think
it'll be a bit easier to read.

[michaelpg, 2014-09-02 21:57:34]

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:44: <ct-party-time
partytime="{{ !failures || !failures.failures[tree] ||
failures.failures[tree].length == 0 }}"></ct-party-time>
I feel like this would be better as a conditional template instead of giving
ct-party-time the partytime parameter...

[ojan, 2014-09-09 03:26:40]

Committed patchset #4 (id:60001) manually as 181599 (presubmit successful).

[ojan, 2014-09-11 03:04:48]

Hmmm...my drafts never sent. :(

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:44: <ct-party-time
partytime="{{ !failures || !failures.failures[tree] ||
failures.failures[tree].length == 0 }}"></ct-party-time>
On 2014/09/02 21:57:33, michaelpg wrote:
> I feel like this would be better as a conditional template instead of giving
> ct-party-time the partytime parameter...

Yeah, that's much better. Done.

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-results-panel-tests.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-results-panel-tests.html:102: // also means
that we actually fetch requests. This results in some console
On 2014/09/02 18:34:53, Jeffrey Yasskin wrote:
> "fetch requests"? Maybe "fetch the alerts page"?

Clarified which request we actually fetch.

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html (right):

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html:37:
failures.failures.failures = {
On 2014/09/02 18:34:53, Jeffrey Yasskin wrote:
> Hmmm. Can you avoid having the same name 3 deep?

Changed the first one to hasFailures. I find I keep doing this sort of thing
with our model classes. Need to come up with a better naming scheme.

https://codereview.chromium.org/530613002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-unexpected-failures-tests.html:43:
assert.ok(noFailures.shadowRoot.querySelector('ct-party-time').partytime);
On 2014/09/02 18:34:53, Jeffrey Yasskin wrote:
> Style-wise, I'd probably split this into 4 different tests. I certainly won't
> insist, and I see that it's more syntactic overhead to do that, but I think
> it'll be a bit easier to read.

Done.