Reland Switch Android gtests to use generated wrapper scripts

Reland Switch Android gtests to use generated wrapper scripts

TBR=sergeyberezin
BUG=599919
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299906

[agrieve, 2016-04-13 01:18:08]

Description was changed from

==========
Reland Switch Android gtests to use generated wrapper scripts

BUG=599919
==========

to

==========
Reland Switch Android gtests to use generated wrapper scripts

BUG=599919
==========

[agrieve, 2016-04-13 01:18:09]

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

[agrieve, 2016-04-13 01:19:46]

On 2016/04/13 01:18:09, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:bpastene@chromium.org

Changes since last time are in PS2. Certainly the reason it failed was that the
non-_apk targets were being built, but please advice if you know of any more
spots the compile targets might be getting set (I grepp'ed for _apk and this is
the only one I found)

[kjellander_chromium, 2016-04-13 05:49:31]

kjellander@chromium.org changed reviewers:
+ kjellander@chromium.org

[kjellander_chromium, 2016-04-13 05:49:32]

I think it'll be better to fix the underlying problem in GN first, then land
PS#1. See my comment for more details.

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json
(right):

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json:262:
"android_webview_unittests",
So, I verified that for the GN build, the run_ scripts are only generated when
building the non-suffixed targets.

Even if base_unittests_apk depends on base_unittests (see
https://code.google.com/p/chromium/codesearch#chromium/src/base/base.gyp&l=1642)
it doesn't generate the run_ scripts when building only that target.

I think the ideal solution would be to fix that problem instead, and then reland
PS#1 of this CL, since these additions adds noise and a bit of confusion for the
future.

[agrieve, 2016-04-13 13:56:02]

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json
(right):

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json:262:
"android_webview_unittests",
On 2016/04/13 05:49:32, kjellander (chromium) wrote:
> So, I verified that for the GN build, the run_ scripts are only generated when
> building the non-suffixed targets.
> 
> Even if base_unittests_apk depends on base_unittests (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/base.gyp&l=1642)
> it doesn't generate the run_ scripts when building only that target.
> 
> I think the ideal solution would be to fix that problem instead, and then
reland
> PS#1 of this CL, since these additions adds noise and a bit of confusion for
the
> future.

I debated going this route, but I think we eventually want to move the recipes
over to not use the "_apk" target at all. since that's what developers build,
and that's what non-android platforms use.

I've added a TODO in this patchset pointing out that the _apk version should be
removed once we transition (although we'll likely need to add an exception for
webrtc if you're not looking at migrating soon?)

[kjellander_chromium, 2016-04-13 15:21:36]

lgtm

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json
(right):

https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json:262:
"android_webview_unittests",
On 2016/04/13 13:56:02, agrieve wrote:
> On 2016/04/13 05:49:32, kjellander (chromium) wrote:
> > So, I verified that for the GN build, the run_ scripts are only generated
when
> > building the non-suffixed targets.
> > 
> > Even if base_unittests_apk depends on base_unittests (see
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/base.gyp&l=1642)
> > it doesn't generate the run_ scripts when building only that target.
> > 
> > I think the ideal solution would be to fix that problem instead, and then
> reland
> > PS#1 of this CL, since these additions adds noise and a bit of confusion for
> the
> > future.
> 
> I debated going this route, but I think we eventually want to move the recipes
> over to not use the "_apk" target at all. since that's what developers build,
> and that's what non-android platforms use.

That sounds even better! Many times I have built the non-suffixed target and
been confused why my test won't run. I imagine many others have experienced the
same.

> I've added a TODO in this patchset pointing out that the _apk version should
be
> removed once we transition

Sounds good! Having this in a transition period is certainly unavoidable.
Did you forget to upload the new patch set?

> (although we'll likely need to add an exception for webrtc if you're not
looking at migrating soon?)

We'll be happy to migrate over from the suffixed _apk targets as soon as we get
the chance, so I have no intention of slowing you down on that.

[agrieve, 2016-04-13 15:32:12]

On 2016/04/13 15:21:36, kjellander (chromium) wrote:
> lgtm
> 
>
https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
> File
>
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json
> (right):
> 
>
https://codereview.chromium.org/1881233003/diff/20001/scripts/slave/recipes/c...
>
scripts/slave/recipes/chromium.expected/full_chromium_fyi_Android_Builder__dbg_.json:262:
> "android_webview_unittests",
> On 2016/04/13 13:56:02, agrieve wrote:
> > On 2016/04/13 05:49:32, kjellander (chromium) wrote:
> > > So, I verified that for the GN build, the run_ scripts are only generated
> when
> > > building the non-suffixed targets.
> > > 
> > > Even if base_unittests_apk depends on base_unittests (see
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/base.gyp&l=1642)
> > > it doesn't generate the run_ scripts when building only that target.
> > > 
> > > I think the ideal solution would be to fix that problem instead, and then
> > reland
> > > PS#1 of this CL, since these additions adds noise and a bit of confusion
for
> > the
> > > future.
> > 
> > I debated going this route, but I think we eventually want to move the
recipes
> > over to not use the "_apk" target at all. since that's what developers
build,
> > and that's what non-android platforms use.
> 
> That sounds even better! Many times I have built the non-suffixed target and
> been confused why my test won't run. I imagine many others have experienced
the
> same.
> 
> > I've added a TODO in this patchset pointing out that the _apk version should
> be
> > removed once we transition
> 
> Sounds good! Having this in a transition period is certainly unavoidable.
> Did you forget to upload the new patch set?
It's there already in PS2 (scripts/slave/recipe_modules/chromium_tests/steps.py)

> 
> > (although we'll likely need to add an exception for webrtc if you're not
> looking at migrating soon?)
> 
> We'll be happy to migrate over from the suffixed _apk targets as soon as we
get
> the chance, so I have no intention of slowing you down on that

Great!

[mikecase (-- gone --), 2016-04-13 17:21:24]

mikecase@chromium.org changed reviewers:
+ mikecase@chromium.org

[mikecase (-- gone --), 2016-04-13 17:21:24]

lgtm

[bpastene, 2016-04-13 17:22:49]

lgtm

+ sergeyberezin for a recipe_modules/chromium_tests owner

[bpastene, 2016-04-13 17:23:23]

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

[agrieve, 2016-04-13 17:47:23]

The CQ bit was checked by agrieve@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-13 17:47:28]

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

[commit-bot: I haz the power, 2016-04-13 17:51:40]

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

[commit-bot: I haz the power, 2016-04-13 17:51:41]

Dry run: This issue passed the CQ dry run.

[agrieve, 2016-04-13 18:46:09]

Description was changed from

==========
Reland Switch Android gtests to use generated wrapper scripts

BUG=599919
==========

to

==========
Reland Switch Android gtests to use generated wrapper scripts

TBR=sergeyberezin
BUG=599919
==========

[agrieve, 2016-04-13 18:46:18]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-04-13 18:46:22]

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

[agrieve, 2016-04-13 18:47:17]

On 2016/04/13 18:46:22, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1881233003/20001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1881233003/20001

TBR'ing base on other lgtms. Don't want to have to wait another day.

[Sergey Berezin, 2016-04-13 18:50:44]

lgtm for chromium_tests/steps.py

[commit-bot: I haz the power, 2016-04-13 18:55:40]

Description was changed from

==========
Reland Switch Android gtests to use generated wrapper scripts

TBR=sergeyberezin
BUG=599919
==========

to

==========
Reland Switch Android gtests to use generated wrapper scripts

TBR=sergeyberezin
BUG=599919

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

[commit-bot: I haz the power, 2016-04-13 18:55:41]

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

[agrieve, 2016-04-13 20:27:34]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1885163002/ by agrieve@chromium.org.

The reason for reverting is: Broke
https://build.chromium.org/p/chromium.fyi/builders/Android%20Builder%20%28dbg....