Link to stdio output for compile failures.

Link to stdio output for compile failures.

Some compile failures on Sheriff-O-Matic link to the wrong page today, causing
"No results to display" to be shown under the information for a host, instead
of stdio for that compile failure.

This change checks if the step is "compile" and always displays stdio in that
case.

I haven't added a test case for this; I'd like to clean up the routines in this
class to make it easier to test. I tested this manually while waiting for a
compile failure to happen that didn't display well.

NOTRY=true
BUG=404927
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181841

[teravest, 2014-09-10 20:23:23]

teravest@chromium.org changed reviewers:
+ ojan@chromium.org

[ojan, 2014-09-11 01:58:42]

lgtm

FYI, there's crbug.com/400391 for cleaning up this stuff. Most of the code in
this component should go into a model class, along with the stuff in results.js
that this code uses. Would love it if you wanted to take on cleaning this stuff
up. It's the last major remaining wart from this codebase's garden-o-matic
origins.

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

https://codereview.chromium.org/549313005/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-results-detail.html:98: var use_stdio =
result.actual == 'UNKNOWN' || this.failure.step == 'compile';
Lets make a helper function to avoid code duplication.

function isStdioStep(result, step) {
  return result.actual == 'UNKNOWN' || step == 'compile'
}

Then we could use it here and in line 59.

[teravest, 2014-09-11 15:12:32]

On 2014/09/11 01:58:42, ojan-only-code-yellow-reviews wrote:
> lgtm
> 
> FYI, there's crbug.com/400391 for cleaning up this stuff. Most of the code in
> this component should go into a model class, along with the stuff in
results.js
> that this code uses. Would love it if you wanted to take on cleaning this
stuff
> up. It's the last major remaining wart from this codebase's garden-o-matic
> origins.
> 
>
https://codereview.chromium.org/549313005/diff/1/Tools/GardeningServer/ui/ct-...
> File Tools/GardeningServer/ui/ct-results-detail.html (right):
> 
>
https://codereview.chromium.org/549313005/diff/1/Tools/GardeningServer/ui/ct-...
> Tools/GardeningServer/ui/ct-results-detail.html:98: var use_stdio =
> result.actual == 'UNKNOWN' || this.failure.step == 'compile';
> Lets make a helper function to avoid code duplication.
> 
> function isStdioStep(result, step) {
>   return result.actual == 'UNKNOWN' || step == 'compile'
> }
> 
> Then we could use it here and in line 59.

Done.

I wasn't sure how to decare a function to not be a member of that object given
how things are declared in polymer, so I defined a _isStdioStep() function.

[ojan, 2014-09-11 18:09:08]

lgtm

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

https://codereview.chromium.org/549313005/diff/20001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-results-detail.html:49: _isStdioStep:
function(result, step) {
This is fine, especially since this code needs to move into a model class
anyways. The other thing you could do is to use an anonymous function scope
around the whole Polymer call. Either way is fine.

<script>
(function() {
  function isStdioStep(...) {
    ...
  }

  Polymer({...});
})();

[teravest, 2014-09-11 19:31:51]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-09-11 19:33:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549313005/20001

[commit-bot: I haz the power, 2014-09-11 19:33:32]

Committed patchset #2 (id:20001) as 181841