Split out instrumentation test name map for further use.

Split out instrumentation test name map for further use.

BUG=525873
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298737

[ghost stip (do not use), 2016-02-08 22:20:40]

Description was changed from

==========
Split out instrumentation test name map for further use.

BUG=525873
==========

to

==========
Split out instrumentation test name map for further use.

BUG=525873
==========

[ghost stip (do not use), 2016-02-08 22:20:41]

stip@chromium.org changed reviewers:
+ bpastene@chromium.org, sergeyberezin@chromium.org

[ghost stip (do not use), 2016-02-08 22:21:19]

bpastene: review
sergeyberezin: OWNERS

[bpastene, 2016-02-09 07:04:22]

Could you explain a little what the point of this change is? As far as I can
tell, it just creates another place to configure an instrumentation test's
compile targets, making an already obtuse file even more complex :(

https://codereview.chromium.org/1678273003/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium_tests/steps.py (right):

https://codereview.chromium.org/1678273003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium_tests/steps.py:1552:
'extra_compile_targets': ['system_webview_apk',
What makes these 'extra'? What's the point of designating one of them as the
'main' compile target?

[Sergey Berezin, 2016-02-09 19:10:48]

I don't understand this change either.

As an aside, I would happily stamp it if you *remove* some Android-specific
configs from this file (and consolidate them with whatever dedicated Android
configs we already have). This file is supposed to be project-independent as
much as possible.

[ghost stip (do not use), 2016-02-10 10:51:20]

This CL is part of a way to remove android-specific parts from this file.

All of these targets are defined by in an isolate, and the fact that the
buildsystem enumerates additional targets is a legacy artifact. Post-swarming
there will only be one target to build. By having a 1:1 mapping, we ease the
transition to this world. Currently, there is no way to map the SnakeCase names
to the isolate target.

I'm also happy using the snake_case names instead of CamelCase, but that will
require refactoring existing configs.

[Sergey Berezin, 2016-02-11 01:56:48]

On 2016/02/10 10:51:20, stip wrote:
> This CL is part of a way to remove android-specific parts from this file.
> 
> All of these targets are defined by in an isolate, and the fact that the
> buildsystem enumerates additional targets is a legacy artifact. Post-swarming
> there will only be one target to build. By having a 1:1 mapping, we ease the
> transition to this world. Currently, there is no way to map the SnakeCase
names
> to the isolate target.
> 
> I'm also happy using the snake_case names instead of CamelCase, but that will
> require refactoring existing configs.

Thanks for the explanation! LGTM.

[bpastene, 2016-02-11 11:07:59]

On 2016/02/10 10:51:20, stip wrote:
> This CL is part of a way to remove android-specific parts from this file.
> 
> All of these targets are defined by in an isolate, and the fact that the
> buildsystem enumerates additional targets is a legacy artifact. Post-swarming
> there will only be one target to build. By having a 1:1 mapping, we ease the
> transition to this world. Currently, there is no way to map the SnakeCase
names
> to the isolate target.
> 
> I'm also happy using the snake_case names instead of CamelCase, but that will
> require refactoring existing configs.

Maybe add a comment above the map saying as much?

lgtm

[ghost stip (do not use), 2016-02-11 13:26:33]

comment added!

[ghost stip (do not use), 2016-02-11 13:26:36]

The CQ bit was checked by stip@chromium.org

[ghost stip (do not use), 2016-02-11 13:26:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyberezin@chromium.org, bpastene@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1678273003/#ps20001
(title: "Added comment.")

[commit-bot: I haz the power, 2016-02-11 13:26:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1678273003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1678273003/20001

[commit-bot: I haz the power, 2016-02-11 13:29:14]

Description was changed from

==========
Split out instrumentation test name map for further use.

BUG=525873
==========

to

==========
Split out instrumentation test name map for further use.

BUG=525873

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298737
==========

[commit-bot: I haz the power, 2016-02-11 13:29:15]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298737