Add resource_sizes to chromium_perf steps for Android

Add resource_sizes to chromium_perf steps for Android

BUG=609365
Committed: https://chromium.googlesource.com/chromium/tools/build/+/8a19fa3fb93e1cddbd10a26f6573aa7881fb6781

[agrieve, 2016-09-02 18:24:54]

Description was changed from

==========
Add resource_sizes to chromium_perf steps for Android

BUG=609365
==========

to

==========
Add resource_sizes to chromium_perf steps for Android

BUG=609365
==========

[agrieve, 2016-09-02 18:24:55]

agrieve@chromium.org changed reviewers:
+ jbudorick@chromium.org

[agrieve, 2016-09-02 18:25:42]

On 2016/09/02 18:24:55, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:jbudorick@chromium.org

Note: will not submit until after dependent patchset has landed and run through
the bots.

[jbudorick, 2016-09-02 19:07:22]

jbudorick@chromium.org changed reviewers:
+ dtu@chromium.org

[jbudorick, 2016-09-02 19:07:22]

lgtm but please wait for dtu

[dtu, 2016-09-02 19:13:34]

Sorry, this is misleading -- the perf builder doesn't use this recipe yet
(http://crbug.com/639530)
You'll also need to update android/builder.py for this to take effect.
https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave...

[agrieve, 2016-09-06 17:08:50]

On 2016/09/02 19:13:34, dtu wrote:
> Sorry, this is misleading -- the perf builder doesn't use this recipe yet
> (http://crbug.com/639530)
> You'll also need to update android/builder.py for this to take effect.
>
https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave...

Thanks! Done.

[dtu, 2016-09-07 16:47:00]

lgtm

[agrieve, 2016-09-07 18:37:36]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-07 18:37:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2306853003/#ps40001
(title: "Add resource_sizes to chromium_perf steps for Android")

[commit-bot: I haz the power, 2016-09-07 18:37:44]

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-07 18:45:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 18:45:53]

Try jobs failed on following builders:
  Build Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/311ec2be4d045010)

[agrieve, 2016-09-07 19:08:56]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-07 19:08:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dtu@chromium.org,
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2306853003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-07 19:09:03]

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-07 19:20:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 19:20:34]

Try jobs failed on following builders:
  Build Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/311edf62c5d94110)

[agrieve, 2016-09-07 19:34:52]

agrieve@chromium.org changed reviewers:
+ stip@chromium.org

[ghost stip (do not use), 2016-09-07 21:30:50]

let's talk a bit about what we're trying to do here any why we need another step
class.

we may end up needing a class because of implementation details, but I want to
make sure we've exhausted other options before we do that.

https://codereview.chromium.org/2306853003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/steps.py (right):

https://codereview.chromium.org/2306853003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/steps.py:102: class
ResourceSizesStep(Test):
aw man, another step type? do we really need this?

[jbudorick, 2016-09-07 21:35:43]

https://codereview.chromium.org/2306853003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/steps.py (right):

https://codereview.chromium.org/2306853003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/steps.py:102: class
ResourceSizesStep(Test):
On 2016/09/07 21:30:50, stip wrote:
> aw man, another step type? do we really need this?

The last time a new step vs an existing one (i.e., ScriptTest) came up on
review, phajdan suggested that steps were ok now:
https://codereview.chromium.org/2063323002/#msg25

[agrieve, 2016-09-08 00:25:36]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-08 00:25:36]

The patchset sent to the CQ was uploaded after l-g-t-m from dtu@chromium.org,
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2306853003/#ps100001
(title: "Revert steps.py for a follow-up")

[commit-bot: I haz the power, 2016-09-08 00:25:47]

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

[agrieve, 2016-09-08 00:28:26]

On 2016/09/08 00:25:47, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

As dtu pointed out the steps.py aren't even being used yet, I've moved them to a
follow-up here:
https://codereview.chromium.org/2322453003

Let's continue discussing there :)

[commit-bot: I haz the power, 2016-09-08 00:30:17]

Description was changed from

==========
Add resource_sizes to chromium_perf steps for Android

BUG=609365
==========

to

==========
Add resource_sizes to chromium_perf steps for Android

BUG=609365

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/8a19fa3fb93e1cddbd10...
==========

[commit-bot: I haz the power, 2016-09-08 00:30:18]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/tools/build/+/8a19fa3fb93e1cddbd10...