on CompareGMs failure, show link to most recent results on this builder

on CompareGMs failure, show link to most recent results on this builder

BUG=skia:2639
Committed: https://skia.googlesource.com/buildbot/+/b24fb55b5de58c2b34ad762bf59b56befaff350c

[epoger, 2014-06-04 21:55:10]

PTAL...

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
File master/skia_master_scripts/factory.py (right):

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
master/skia_master_scripts/factory.py:537: get_props_from_stdout={
Unfortunately, this doesn't work.

https://github.com/google/skia-buildbot/blob/master/master/skia_master_script...
only processes get_props_from_stdout if the build step succeeded.  In this case,
the URL is only useful when the test fails. :-(

See
http://c128.i.corp.google.com:10115/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-...
: CompareGMs fails, and its stdio contains the link, but the build does not
display the property.

ALSO- I would rather have the URL be visible under the CompareGMs step on the
build page, rather than being a property of the build as a whole.  It's more
detail that developers want to look at in the context of a single step's
results...

[borenet, 2014-06-05 14:36:10]

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
File master/skia_master_scripts/factory.py (right):

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
master/skia_master_scripts/factory.py:46: # EPOGER: how can I share this
definition with compare_gms.py?
global_variables.json.

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
master/skia_master_scripts/factory.py:537: get_props_from_stdout={
On 2014/06/04 21:55:11, epoger wrote:
> Unfortunately, this doesn't work.
> 
>
https://github.com/google/skia-buildbot/blob/master/master/skia_master_script...
> only processes get_props_from_stdout if the build step succeeded.  In this
case,
> the URL is only useful when the test fails. :-(
> 
> See
>
http://c128.i.corp.google.com:10115/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-...
> : CompareGMs fails, and its stdio contains the link, but the build does not
> display the property.
> 
> ALSO- I would rather have the URL be visible under the CompareGMs step on the
> build page, rather than being a property of the build as a whole.  It's more
> detail that developers want to look at in the context of a single step's
> results...

We can probably change the logic in
master/skia_master_scripts/skia_build_step.py: SkiaBuildStep.commandComplete,
which currently doesn't bother checking for get_props_from_stdout when the
command failed.  Instead, we could check anyway but avoid raising an Exception
when we don't find a match IF the command failed.

I'm a little confused about what you're asking for with where the link is
displayed.  It should show up under CompareGMs in the list of steps (just like
got_revision shows up under Update) and it should show up in the table of build
properties.  Is that not sufficient?  I don't know if it'll actually be a
clickable link (my guess is no).

[epoger, 2014-06-05 15:08:42]

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
File master/skia_master_scripts/factory.py (right):

https://codereview.chromium.org/313203003/diff/1/master/skia_master_scripts/f...
master/skia_master_scripts/factory.py:537: get_props_from_stdout={
On 2014/06/05 14:36:10, borenet wrote:
> On 2014/06/04 21:55:11, epoger wrote:
> > Unfortunately, this doesn't work.
> > 
> >
>
https://github.com/google/skia-buildbot/blob/master/master/skia_master_script...
> > only processes get_props_from_stdout if the build step succeeded.  In this
> case,
> > the URL is only useful when the test fails. :-(
> > 
> > See
> >
>
http://c128.i.corp.google.com:10115/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-...
> > : CompareGMs fails, and its stdio contains the link, but the build does not
> > display the property.
> > 
> > ALSO- I would rather have the URL be visible under the CompareGMs step on
the
> > build page, rather than being a property of the build as a whole.  It's more
> > detail that developers want to look at in the context of a single step's
> > results...
> 
> We can probably change the logic in
> master/skia_master_scripts/skia_build_step.py: SkiaBuildStep.commandComplete,
> which currently doesn't bother checking for get_props_from_stdout when the
> command failed.  Instead, we could check anyway but avoid raising an Exception
> when we don't find a match IF the command failed.
> 
> I'm a little confused about what you're asking for with where the link is
> displayed.  It should show up under CompareGMs in the list of steps (just like
> got_revision shows up under Update) and it should show up in the table of
build
> properties.  Is that not sufficient?  I don't know if it'll actually be a
> clickable link (my guess is no).

Ultimately, my vision is of something like
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/16786
, where the webkit_tests step goes red to show you which TYPE of test failed,
and then you can click on a link right there to see what the specific failures
were.

How can we get there?  Or, is there some other model that's even better?

[epoger, 2014-06-05 15:27:57]

On 2014/06/05 15:08:42, epoger wrote:
> Ultimately, my vision is of something like
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/16786
> , where the webkit_tests step goes red to show you which TYPE of test failed,
> and then you can click on a link right there to see what the specific failures
> were.
> 
> How can we get there?  Or, is there some other model that's even better?

Live discussion with Eric:

- The model in
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/16786
would be ideal, but we don't know (offhand) how we would do that in our branch
of the buildbot code.
- Rather than trying to figure out how to shoehorn that type of display into our
branch of the buildbot code, make simple modifications to our existing
get_props_from_stdout logic to get the link displayed with minimal work.
- After we have merged back to the HEAD buildbot code, we can explore niftier
ways of displaying a clickable URL.

[epoger, 2014-06-05 20:17:34]

PTAL.  You can see its output at
http://c128.i.corp.google.com:10115/builders/Test-Ubuntu13.10-ShuttleA-NoGPU-...

[borenet, 2014-06-05 20:27:48]

LGTM

[epoger, 2014-06-05 20:28:34]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-06-05 20:28:54]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/epoger@google.com/313203003/20001

[commit-bot: I haz the power, 2014-06-05 20:29:48]

Change committed as b24fb55b5de58c2b34ad762bf59b56befaff350c