[Android]: Add content_gl_tests to test_runner.py gtest suite.

[Android]: add content_gl_tests to test_runner.py gtest suite.

Adding the content_gl_tests unit tests to the
gtest suite list supported by test_runner.py for Android platform.

BUG=none

R=bulach

TEST= Command to execute the content_gl_tests on Android:
path/to/build/android/test_runner.py gtest -s content_gl_tests <--gtest_filter=[TestFilter]>

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245299

[r.kasibhatla, 2014-01-16 06:40:34]

PTAL.

[bulach, 2014-01-16 11:58:03]

thanks! I have some important comments below, looks like it'll be simpler to
split these:

https://codereview.chromium.org/140623002/diff/1/build/android/pylib/gtest/gt...
File build/android/pylib/gtest/gtest_config.py (right):

https://codereview.chromium.org/140623002/diff/1/build/android/pylib/gtest/gt...
build/android/pylib/gtest/gtest_config.py:30: 'ui_unittests',
see below:

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp#newcode68
ui/gfx/gfx.gyp:68: #             ui_unittests.
if this TODO is not obsolete :) looks like it's already being built and executed
as part of ui_unittests?

http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%...

by changing it to gtest_target_type, it'd actually *remove* these tests from
other platforms until they're listed in the buildbot steps, etc... i.e., on
desktop it will be built as an "executable" rather than "static library", and
nobody will execute this test suite..

I'm not sure if original reason for this TODO was just so these tests could be
easily ran in the bots or if there are more complications, but anyways, I think
it should be done as a separate series of patches... wdyt?

[r.kasibhatla, 2014-01-16 12:39:15]

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp#newcode68
ui/gfx/gfx.gyp:68: #             ui_unittests.
On 2014/01/16 11:58:03, bulach wrote:
> if this TODO is not obsolete :) looks like it's already being built and
executed
> as part of ui_unittests?

Sorry, I didn't notice that gfx_geometry_unittests were part of ui_unittests :).
Agreed, separating gfx_geometry from ui unittests can be taken to separate patch
(if you feel separation is ok).

> 
>
http://build.chromium.org/p/chromium.linux/builders/Android%2520Tests%2520%25...
> 
> by changing it to gtest_target_type, it'd actually *remove* these tests from
> other platforms until they're listed in the buildbot steps, etc... i.e., on
> desktop it will be built as an "executable" rather than "static library", and
> nobody will execute this test suite..

Aah, you mean if I separate gfx_geometry_unittests from ui_unittests, we need to
update buildbot scripts as well to ensure the new binary is also executed now. I
am not sure how to get the buildbot scripts modified for the change? Any
pointers?

> 
> I'm not sure if original reason for this TODO was just so these tests could be
> easily ran in the bots or if there are more complications, but anyways, I
think
> it should be done as a separate series of patches... wdyt?

I actually wanted content_gl_tests and gfx_geometry_unittests to be ran for
Android also. Since, the core issue is now separation of gfx_geometry_unittests
from ui_unittests, I agree it should be done separately. 
Shall I modify the patch to just add content_gl_tests to gtest_config.py.
As for gfx_geometry_unittests, what is your suggestion? Should I also check with
beng about separation and if he is also ok with the separation?

[bulach, 2014-01-16 14:06:44]

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/140623002/diff/1/ui/gfx/gfx.gyp#newcode68
ui/gfx/gfx.gyp:68: #             ui_unittests.
On 2014/01/16 12:39:16, r.kasibhatla wrote:
> On 2014/01/16 11:58:03, bulach wrote:
> > if this TODO is not obsolete :) looks like it's already being built and
> executed
> > as part of ui_unittests?
> 
> Sorry, I didn't notice that gfx_geometry_unittests were part of ui_unittests
:).
> Agreed, separating gfx_geometry from ui unittests can be taken to separate
patch
> (if you feel separation is ok).

yeah, the tests from gfx_geometry_unittests are already running for android, so
I think it's less pressing to put on a target of its own...

> 
> > 
> >
>
http://build.chromium.org/p/chromium.linux/builders/Android%252520Tests%25252...
> > 
> > by changing it to gtest_target_type, it'd actually *remove* these tests from
> > other platforms until they're listed in the buildbot steps, etc... i.e., on
> > desktop it will be built as an "executable" rather than "static library",
and
> > nobody will execute this test suite..
> 
> Aah, you mean if I separate gfx_geometry_unittests from ui_unittests, we need
to
> update buildbot scripts as well to ensure the new binary is also executed now.
I
> am not sure how to get the buildbot scripts modified for the change? Any
> pointers?

it's a bit complicated, you'd probably better create a bug against
infrastructure and ask that team to take care of it...
there are implications about load, it has to run on try servers, commit queue,
etc...
phajdan.jr would be a good person to ask.

> 
> > 
> > I'm not sure if original reason for this TODO was just so these tests could
be
> > easily ran in the bots or if there are more complications, but anyways, I
> think
> > it should be done as a separate series of patches... wdyt?
> 
> I actually wanted content_gl_tests and gfx_geometry_unittests to be ran for
> Android also. Since, the core issue is now separation of
gfx_geometry_unittests
> from ui_unittests, I agree it should be done separately. 
> Shall I modify the patch to just add content_gl_tests to gtest_config.py.

yes, I think that's the best way to go.

> As for gfx_geometry_unittests, what is your suggestion? Should I also check
with
> beng about separation and if he is also ok with the separation?

since the gfx_geometry_unittests are already running (although under a different
name), I'd do that separately as it'll be a big effort on its own.. :)
perhaps ping beng@ first, and if it's important to keep them separately, then
create a "feature request" for infrastructure and see what they say?
I don't have any concrete pointers, but for starters, all these files that
reference "ui_unittests" would have to change to also add "gfx_unittests":

https://code.google.com/p/chromium/codesearch#search/&q=file:.*cfg%20%5C'ui_u...

[r.kasibhatla, 2014-01-16 14:28:45]

Thanks bulach for the pointers. I will check with beng and then proceed
accordingly on separation.

content_gl_tests is also required for android (apk generation already happens
but we cannot execute it with test_runner.py since it is not part of
configuration). I reverted all other changes and just added content_gl_tests to
gtest_config.py. Is that ok? Can you check :)

[bulach, 2014-01-16 14:34:31]

lgtm, thanks!

on the patch description: please make sure the "subject" matches the first line,
and also it's good to mention it's android, so something like:

Android: add content_gl_tests to test_runner.py gtest suite.

also, maybe add a TEST= with the command line to run it..

[r.kasibhatla, 2014-01-16 14:43:23]

On 2014/01/16 14:34:31, bulach wrote:
> lgtm, thanks!
> 
> on the patch description: please make sure the "subject" matches the first
line,
> and also it's good to mention it's android, so something like:
> 
> Android: add content_gl_tests to test_runner.py gtest suite.
> 
> also, maybe add a TEST= with the command line to run it..

Done. Thanks :)

[commit-bot: I haz the power, 2014-01-16 15:03:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/r.kasibhatla@samsung.com/140623002/90001

[commit-bot: I haz the power, 2014-01-16 20:03:04]

Change committed as 245299