Add support for the ChromiumTestShellTest APK.

Add support for the ChromiumTestShellTest APK.

- Add the gyp rules for the ChromiumTestShellTestAPK.
- Add the initial APK base class.
- Add a sample test that launches the activity.

BUG=


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

[David Trainor- moved to gerrit, 2012-09-18 22:11:05]

ptal

[Ted C, 2012-09-19 21:18:49]

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
File chrome/android/testshell/java/chromium_testshell_apk.xml (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
chrome/android/testshell/java/chromium_testshell_apk.xml:35: <filelist
files="${INPUT_JARS_PATHS}"/>
tab char here

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
File
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java
(right):

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java:31:
ChromiumTestShellActivity.class));
I would do:

setActivityIntent(intent);
return getActivity();

I think that is more compliant with how the test base expects it to work.

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi...
chrome/chrome_android.gypi:46:
#'browser/component/components.gyp:web_contents_delegate_android_java'
remove?

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi...
chrome/chrome_android.gypi:71: # Right now comment out the find call, relying on
the jar version instead.
huh?  what does this mean?  Should this be a TODO as well?

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_tests.gypi#n...
chrome/chrome_tests.gypi:4704: ['OS == "android" and sdk_build==1', {
any reason for the sdk_build=1?  Shouldn't this work either way?  Or does ant
require the sdk-buildness.

[David Trainor- moved to gerrit, 2012-09-20 02:28:17]

- Addressed the comments.
- Added support for chrome/android/javatests tests and put a dummy test there so
it properly compiles.
- Had to add a dummy resource so the new apk creation gyp scripts work (my next
cl will use this resource).

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
File chrome/android/testshell/java/chromium_testshell_apk.xml (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
chrome/android/testshell/java/chromium_testshell_apk.xml:35: <filelist
files="${INPUT_JARS_PATHS}"/>
On 2012/09/19 21:18:49, Ted C wrote:
> tab char here

Done.

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
File
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java
(right):

https://codereview.chromium.org/10943014/diff/2001/chrome/android/testshell/j...
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java:31:
ChromiumTestShellActivity.class));
Done.  Made the change in ContentShellTestBase as well.

On 2012/09/19 21:18:49, Ted C wrote:
> I would do:
> 
> setActivityIntent(intent);
> return getActivity();
> 
> I think that is more compliant with how the test base expects it to work.

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi...
chrome/chrome_android.gypi:46:
#'browser/component/components.gyp:web_contents_delegate_android_java'
On 2012/09/19 21:18:49, Ted C wrote:
> remove?

Done.

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_android.gypi...
chrome/chrome_android.gypi:71: # Right now comment out the find call, relying on
the jar version instead.
Sorry, remnants of my fight with gyp and ant.

On 2012/09/19 21:18:49, Ted C wrote:
> huh?  what does this mean?  Should this be a TODO as well?

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/10943014/diff/2001/chrome/chrome_tests.gypi#n...
chrome/chrome_tests.gypi:4704: ['OS == "android" and sdk_build==1', {
ant requires the sdk, so need this downstream at least.  I can take it out here
upstream.

On 2012/09/19 21:18:49, Ted C wrote:
> any reason for the sdk_build=1?  Shouldn't this work either way?  Or does ant
> require the sdk-buildness.

[Yaron, 2012-09-20 03:05:17]

https://codereview.chromium.org/10943014/diff/13001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java
(right):

https://codereview.chromium.org/10943014/diff/13001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java:16:
@DisabledTest
Add // TODO(dtrainor): Remove when an actual test is added. Wait, how does this
differ from ChromiumTestShellUrlTest.java? This is just to show that both
locations for tests work?

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
File chrome/android/testshell/java/chromium_testshell_apk.xml (right):

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/java/chromium_testshell_apk.xml:30: <!-- Set the output
directory for the final apk to the ${apks.dir}. -->
nit: indentaiton

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
File chrome/android/testshell/javatests/chromium_testshell_test_apk.xml (right):

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:28: <property
name="generate.test.jar" value="true"/>
nit: indentation.

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:37: </path>
nit: remove tab

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:51: <target
name="-post-compile">
Remove this section: java apk tests shouldn't include gdbserver

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:69: <!-- We
also want a .jar as well as an .apk for ChromiumTestShellTest-debug
Remove this as you're using generate.test.jar. See changes in
http://codereview.chromium.org/10905138/

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_android.gyp...
chrome/chrome_android.gypi:44: '../media/media.gyp:media_java',
Where's web_contents_delegate_android_java? Oh, picked up by chrome_java? I
think you can pare this down to just chrome_java and the rest is picked up
transitively.

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_tests.gypi#...
chrome/chrome_tests.gypi:4748: '../base/base.gyp:base',
Consider reducing this to just chromium_testshell?

[David Trainor- moved to gerrit, 2012-09-20 06:39:26]

Thanks for the comments!  Replied to a few.  Will make changes tomorrow and get
this landed.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/jav...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java
(right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/jav...
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java:16:
@DisabledTest
Yeah it's to make this path (chrome/android/javatests) compile.  Without any
files here the folder wouldn't exist and the gyp/ant build would fail.  I wanted
to set it up to the point where people could start adding tests and not have to
worry about build configurations.

On 2012/09/20 03:05:17, Yaron wrote:
> Add // TODO(dtrainor): Remove when an actual test is added. Wait, how does
this
> differ from ChromiumTestShellUrlTest.java? This is just to show that both
> locations for tests work?

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_andr...
File chrome/chrome_android.gypi (right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_andr...
chrome/chrome_android.gypi:44: '../media/media.gyp:media_java',
I think I'm an ant build change CL behind or two.  Will it automatically pick up
these files in my ant INPUT_JAR_PATHS if they are not explicitly listed in the
dependencies array?  I can try it regardless :).

On 2012/09/20 03:05:17, Yaron wrote:
> Where's web_contents_delegate_android_java? Oh, picked up by chrome_java? I
> think you can pare this down to just chrome_java and the rest is picked up
> transitively.

[Ted C, 2012-09-20 17:07:04]

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
File
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java
(right):

https://codereview.chromium.org/10943014/diff/13001/chrome/android/testshell/...
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java:31:
ChromiumTestShellActivity.class));
I think this needs +2 indent...looks like 6 to me.

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/10943014/diff/13001/chrome/chrome_android.gyp...
chrome/chrome_android.gypi:44: '../media/media.gyp:media_java',
On 2012/09/20 06:39:26, dtrainor wrote:
> I think I'm an ant build change CL behind or two.  Will it automatically pick
up
> these files in my ant INPUT_JAR_PATHS if they are not explicitly listed in the
> dependencies array?  I can try it regardless :).

Yeah, the exported dependent settings for the input jar path are additive, so if
chrome depends on everything, then it should just "work".

> 
> On 2012/09/20 03:05:17, Yaron wrote:
> > Where's web_contents_delegate_android_java? Oh, picked up by chrome_java? I
> > think you can pare this down to just chrome_java and the rest is picked up
> > transitively.
>

https://codereview.chromium.org/10943014/diff/13001/content/shell/android/jav...
File
content/shell/android/javatests/src/org/chromium/content_shell/ContentShellTestBase.java
(right):

https://codereview.chromium.org/10943014/diff/13001/content/shell/android/jav...
content/shell/android/javatests/src/org/chromium/content_shell/ContentShellTestBase.java:30:
ContentShellActivity.class));
same...+2 indent

[Yaron, 2012-09-20 20:05:14]

oh, looks like you hadn't made the changes from my review. I'll take a look at
the next patchset

https://codereview.chromium.org/10943014/diff/13001/build/all_android.gyp
File build/all_android.gyp (left):

https://codereview.chromium.org/10943014/diff/13001/build/all_android.gyp#old...
build/all_android.gyp:25: 'conditions': [
Please add this back. We'll need this condition for when we start gardening, and
regardless, we don't want to carry this diff.

https://codereview.chromium.org/10943014/diff/13001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java
(right):

https://codereview.chromium.org/10943014/diff/13001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java:16:
@DisabledTest
On 2012/09/20 06:39:26, dtrainor wrote:
> Yeah it's to make this path (chrome/android/javatests) compile.  Without any
> files here the folder wouldn't exist and the gyp/ant build would fail.  I
wanted
> to set it up to the point where people could start adding tests and not have
to
> worry about build configurations.
> 
> On 2012/09/20 03:05:17, Yaron wrote:
> > Add // TODO(dtrainor): Remove when an actual test is added. Wait, how does
> this
> > differ from ChromiumTestShellUrlTest.java? This is just to show that both
> > locations for tests work?
> 

Can you add the TODO

[David Trainor- moved to gerrit, 2012-09-20 21:49:50]

https://chromiumcodereview.appspot.com/10943014/diff/13001/build/all_android.gyp
File build/all_android.gyp (left):

https://chromiumcodereview.appspot.com/10943014/diff/13001/build/all_android....
build/all_android.gyp:25: 'conditions': [
On 2012/09/20 20:05:15, Yaron wrote:
> Please add this back. We'll need this condition for when we start gardening,
and
> regardless, we don't want to carry this diff.

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/jav...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java
(right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/jav...
chrome/android/javatests/src/org/chromium/chrome/browser/test/DummyIntegrationTest.java:16:
@DisabledTest
On 2012/09/20 20:05:15, Yaron wrote:
> On 2012/09/20 06:39:26, dtrainor wrote:
> > Yeah it's to make this path (chrome/android/javatests) compile.  Without any
> > files here the folder wouldn't exist and the gyp/ant build would fail.  I
> wanted
> > to set it up to the point where people could start adding tests and not have
> to
> > worry about build configurations.
> > 
> > On 2012/09/20 03:05:17, Yaron wrote:
> > > Add // TODO(dtrainor): Remove when an actual test is added. Wait, how does
> > this
> > > differ from ChromiumTestShellUrlTest.java? This is just to show that both
> > > locations for tests work?
> > 
> 
> Can you add the TODO

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
File chrome/android/testshell/java/chromium_testshell_apk.xml (right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/java/chromium_testshell_apk.xml:30: <!-- Set the output
directory for the final apk to the ${apks.dir}. -->
On 2012/09/20 03:05:17, Yaron wrote:
> nit: indentaiton

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
File chrome/android/testshell/javatests/chromium_testshell_test_apk.xml (right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:28: <property
name="generate.test.jar" value="true"/>
On 2012/09/20 03:05:17, Yaron wrote:
> nit: indentation.

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:37: </path>
On 2012/09/20 03:05:17, Yaron wrote:
> nit: remove tab

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:51: <target
name="-post-compile">
On 2012/09/20 03:05:17, Yaron wrote:
> Remove this section: java apk tests shouldn't include gdbserver

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/javatests/chromium_testshell_test_apk.xml:69: <!-- We
also want a .jar as well as an .apk for ChromiumTestShellTest-debug
On 2012/09/20 03:05:17, Yaron wrote:
> Remove this as you're using generate.test.jar. See changes in
> http://codereview.chromium.org/10905138/

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
File
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java
(right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/android/tes...
chrome/android/testshell/javatests/src/org/chromium/chrome/testshell/ChromiumTestShellTestBase.java:31:
ChromiumTestShellActivity.class));
On 2012/09/20 17:07:04, Ted C wrote:
> I think this needs +2 indent...looks like 6 to me.

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_andr...
File chrome/chrome_android.gypi (right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_andr...
chrome/chrome_android.gypi:44: '../media/media.gyp:media_java',
On 2012/09/20 17:07:04, Ted C wrote:
> On 2012/09/20 06:39:26, dtrainor wrote:
> > I think I'm an ant build change CL behind or two.  Will it automatically
pick
> up
> > these files in my ant INPUT_JAR_PATHS if they are not explicitly listed in
the
> > dependencies array?  I can try it regardless :).
> 
> Yeah, the exported dependent settings for the input jar path are additive, so
if
> chrome depends on everything, then it should just "work".
> 
> > 
> > On 2012/09/20 03:05:17, Yaron wrote:
> > > Where's web_contents_delegate_android_java? Oh, picked up by chrome_java?
I
> > > think you can pare this down to just chrome_java and the rest is picked up
> > > transitively.
> > 
> 

Done.

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_test...
File chrome/chrome_tests.gypi (right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/chrome/chrome_test...
chrome/chrome_tests.gypi:4748: '../base/base.gyp:base',
Done.

On 2012/09/20 03:05:17, Yaron wrote:
> Consider reducing this to just chromium_testshell?

https://chromiumcodereview.appspot.com/10943014/diff/13001/content/shell/andr...
File
content/shell/android/javatests/src/org/chromium/content_shell/ContentShellTestBase.java
(right):

https://chromiumcodereview.appspot.com/10943014/diff/13001/content/shell/andr...
content/shell/android/javatests/src/org/chromium/content_shell/ContentShellTestBase.java:30:
ContentShellActivity.class));
Eagle eye...

      .'''''.
     '       .
  __' 0       ;
 /  \         ;
|  .-|         ;
\/~~'          ;
   /;.  .;;.  . ;
  /;;;;;;;;;;;;;\
 |;;;;;;;;;;;;;;;\

On 2012/09/20 17:07:04, Ted C wrote:
> same...+2 indent

[Yaron, 2012-09-20 22:06:49]

lgtm

https://chromiumcodereview.appspot.com/10943014/diff/21001/chrome/android/tes...
File chrome/android/testshell/java/chromium_testshell_apk.xml (right):

https://chromiumcodereview.appspot.com/10943014/diff/21001/chrome/android/tes...
chrome/android/testshell/java/chromium_testshell_apk.xml:22: <!--property
name="asset.absolute.dir" location="${out.dir}/assets"/-->
Remove commented-out property.

[commit-bot: I haz the power, 2012-09-21 18:48:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtrainor@chromium.org/10943014/17005

[commit-bot: I haz the power, 2012-09-21 18:48:50]

Failed to apply patch for
chrome/android/testshell/javatests/AndroidManifest.xml:
While running patch -p1 --forward --force;
A         chrome/android/testshell/javatests
patching file chrome/android/testshell/javatests/AndroidManifest.xml
Hunk #1 FAILED at 5.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/android/testshell/javatests/AndroidManifest.xml.rej

[commit-bot: I haz the power, 2012-09-21 19:00:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtrainor@chromium.org/10943014/15004

[commit-bot: I haz the power, 2012-09-21 19:01:04]

Presubmit check for 10943014-15004 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 1.2s to calculate.

[David Trainor- moved to gerrit, 2012-09-21 19:04:29]

Scott do you mind taking a look?  All of this is android specific and got
Yaron's lgtm, but need a chrome/ owner, so up to you what you think you want to
review :).  Thanks!

[sky, 2012-09-21 20:19:00]

LGTM

[commit-bot: I haz the power, 2012-09-21 20:54:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtrainor@chromium.org/10943014/15004

[commit-bot: I haz the power, 2012-09-21 23:00:54]

Change committed as 158106