merge run and check-rc steps

merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

The N5 should pass and the N10 and N7 should fail, showing us both sides.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003

Committed: https://skia.googlesource.com/skia/+/7de2d40dccdd7db09fcd3460a9bde8cc9082874b

[mtklein_C, 2016-09-14 16:04:27]

Description was changed from

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

BUG=skia:
==========

to

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003
==========

[mtklein_C, 2016-09-14 16:45:04]

Description was changed from

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003
==========

to

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

The N5 should pass and the N10 and N7 should fail, showing us both sides.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003
==========

[mtklein_C, 2016-09-14 16:45:10]

mtklein@chromium.org changed reviewers:
+ borenet@google.com

[borenet, 2016-09-14 16:52:24]

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
File infra/bots/recipe_modules/flavor/gn_android_flavor.py (right):

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/flavor/gn_android_flavor.py:81: self._adb('dump log ',
'logcat', '-d')
Nit: trailing space?

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/flavor/gn_android_flavor.py:105: bin_dir + 'rc'])))
Please break this into a few lines and add a helpful error message, eg:

try:
  output = subprocess.check_output(['adb', 'shell', 'cat', bin_dir + 'rc'])
except:
  print 'Failed to obtain return code from file'
  sys.exit(1)
try:
  rc = int(output)
except:
  print 'Got non-integer return code: "%s"' % output
  sys.exit(1)
sys.exit(rc)

[mtklein_C, 2016-09-14 17:30:31]

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
File infra/bots/recipe_modules/flavor/gn_android_flavor.py (right):

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/flavor/gn_android_flavor.py:81: self._adb('dump log ',
'logcat', '-d')
On 2016/09/14 at 16:52:24, borenet wrote:
> Nit: trailing space?

Done.

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/flavor/gn_android_flavor.py:105: bin_dir + 'rc'])))
On 2016/09/14 at 16:52:24, borenet wrote:
> Please break this into a few lines and add a helpful error message, eg:
> 
> try:
>   output = subprocess.check_output(['adb', 'shell', 'cat', bin_dir + 'rc'])
> except:
>   print 'Failed to obtain return code from file'
>   sys.exit(1)
> try:
>   rc = int(output)
> except:
>   print 'Got non-integer return code: "%s"' % output
>   sys.exit(1)
> sys.exit(rc)

Done, mostly.  We don't expect this 'adb shell ...' to fail any more or less
than the others, so I didn't check for CalledProcessError, but I did add a check
here for ValueError.

[borenet, 2016-09-14 17:44:35]

LGTM

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
File infra/bots/recipe_modules/flavor/gn_android_flavor.py (right):

https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/flavor/gn_android_flavor.py:105: bin_dir + 'rc'])))
On 2016/09/14 17:30:31, mtklein_C wrote:
> On 2016/09/14 at 16:52:24, borenet wrote:
> > Please break this into a few lines and add a helpful error message, eg:
> > 
> > try:
> >   output = subprocess.check_output(['adb', 'shell', 'cat', bin_dir + 'rc'])
> > except:
> >   print 'Failed to obtain return code from file'
> >   sys.exit(1)
> > try:
> >   rc = int(output)
> > except:
> >   print 'Got non-integer return code: "%s"' % output
> >   sys.exit(1)
> > sys.exit(rc)
> 
> Done, mostly.  We don't expect this 'adb shell ...' to fail any more or less
> than the others, so I didn't check for CalledProcessError, but I did add a
check
> here for ValueError.

Thanks. Mainly just want to distinguish between "thing failed" and "couldn't
read return code", plus having the python stacktrace at the end of the DM log is
kind of unintelligible.

[mtklein_C, 2016-09-14 17:48:40]

On 2016/09/14 at 17:44:35, borenet wrote:
> LGTM
> 
>
https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
> File infra/bots/recipe_modules/flavor/gn_android_flavor.py (right):
> 
>
https://codereview.chromium.org/2339713003/diff/20001/infra/bots/recipe_modul...
> infra/bots/recipe_modules/flavor/gn_android_flavor.py:105: bin_dir + 'rc'])))
> On 2016/09/14 17:30:31, mtklein_C wrote:
> > On 2016/09/14 at 16:52:24, borenet wrote:
> > > Please break this into a few lines and add a helpful error message, eg:
> > > 
> > > try:
> > >   output = subprocess.check_output(['adb', 'shell', 'cat', bin_dir +
'rc'])
> > > except:
> > >   print 'Failed to obtain return code from file'
> > >   sys.exit(1)
> > > try:
> > >   rc = int(output)
> > > except:
> > >   print 'Got non-integer return code: "%s"' % output
> > >   sys.exit(1)
> > > sys.exit(rc)
> > 
> > Done, mostly.  We don't expect this 'adb shell ...' to fail any more or less
> > than the others, so I didn't check for CalledProcessError, but I did add a
check
> > here for ValueError.
> 
> Thanks. Mainly just want to distinguish between "thing failed" and "couldn't
read return code", plus having the python stacktrace at the end of the DM log is
kind of unintelligible.

Agreed.  The N7 just dumped this:

14.1m elapsed, 1 active, 11 queued, 780MB RAM, 780MB peak
	unit test  GLPrograms
Couldn't read the return code.  Probably killed for OOM.
step returned non-zero exit code: 1

[mtklein_C, 2016-09-14 17:50:28]

The CQ bit was checked by mtklein@chromium.org

[commit-bot: I haz the power, 2016-09-14 17:50:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 18:15:57]

Description was changed from

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

The N5 should pass and the N10 and N7 should fail, showing us both sides.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003
==========

to

==========
merge run and check-rc steps

This'll make the 'dm' or 'nanobench' step go red like you'd hope.

The N5 should pass and the N10 and N7 should fail, showing us both sides.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339713003

Committed:
https://skia.googlesource.com/skia/+/7de2d40dccdd7db09fcd3460a9bde8cc9082874b
==========

[commit-bot: I haz the power, 2016-09-14 18:15:58]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/7de2d40dccdd7db09fcd3460a9bde8cc9082874b