Add new GN junit binary template and new GYP java binary template.

Add new GN junit binary template and new GYP java binary template.

Adds a new junit_binary template for gyp which makes it easy to
make new Junit test suites by setting a lot of the options and
necessary dependencies for you.

Also, adds a new java_binary.gypi file which uses the
create_java_binary_script.py similiar to how the java_binary
GN template works when main_class is specified. This lets
GYP and GN work similiar when building a junit binary, so
our test runner can run them the same way.

Finally, because of the slightly different way were now running
the junit binary, the java classpath contains all of the jars
instead of just the test jar (before we were running
java -jar test_jar.jar which set the classpath to just be
test_jar.jar). Therefore, we can simplify things and just look
in the classpath for the paths to the Robolectric dependencies
when running Robolectric junit tests.

BUG=

Committed: https://crrev.com/1ad64324d470c43b29854282d5ad5b1ff75dfc6c
Cr-Commit-Position: refs/heads/master@{#324093}

[mikecase (-- gone --), 2015-04-02 03:54:11]

mikecase@chromium.org changed reviewers:
+ cjhopman@chromium.org, jbudorick@chromium.org

[mikecase (-- gone --), 2015-04-02 03:54:13]

Please take a look at this when you get the chance. It fixes all of the problems
building the junit tests with GN and also fixes the issue with not being able to
add things to the classpath when running junit tests.


Currently, gyp and GN build the junit jars differently...

GYP: Creates a junit jar with a main_class and classpath specified in the jar
manifest. Our test runner runs these jars with "java -jar test_jar.jar".
However, running tests this way makes java set the java classpath to just be the
test jar (e.g. test_jar.jar) and anything else added to the classpath by gyp is
ignored.

GN: Creates a junit jar with a basically empty manifest. Also generates a python
executable in OUT_DIR/bin/test_jar which runs something along the lines of "java
-classpath <classpath> <main_class>". Our test runner currently has no way to
run junit tests built this way.


This CL moves everything over to use the way GN currently builds the junit
tests. This means...
(1) Change the test runner to run junit tests using the generated python exe.
(2) Change GYP to also build the python exe.
(3) Take advantage of the fact that the classpath isn't overwritten (now
contains everything added by gyp/gn) when running junit tests. For example, I
can now get the Robolectric jar paths from the java classpath which simplifies
running Robolectric tests.


I tested building and running the content_junit_tests, chrome_junit_tests, and
junit_unit_tests using both gyp and GN and everything works.


+ cjhopman for the build file changes.
+ jbudorick for the test runner changes

[jbudorick, 2015-04-02 14:21:43]

https://codereview.chromium.org/1057783002/diff/20001/build/java_binary.gypi
File build/java_binary.gypi (right):

https://codereview.chromium.org/1057783002/diff/20001/build/java_binary.gypi#...
build/java_binary.gypi:1: # Copyright 2015 The Chromium Authors. All rights
reserved.
So this is in addition to build/host_jar.gypi? Do we need both? There's
obviously a lot of overlap.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java:16:
* A Robolectric dependency resolver that looks for the Robolectric dependencies
If this is specific to Robolectric, should that be reflected in either the
package or the name?

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java:27:
super();
I _think_ this is unnecessary.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestArgParser.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestArgParser.java:17:
private static final Pattern COLON = Pattern.compile(":");
nit: Separate this from the members.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java:37:
LinkedList<String> testJarPaths = new LinkedList<String>();
I know I'm responsible for the code below that you based this on, but this
should probably be something like:

  List<String> testJarPaths = new ArrayList<String>(testJars.length);

(and similarly for classes below)

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/LocalRobolectricTestRunner.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/LocalRobolectricTestRunner.java:32:
return new ClasspathDependencyResolver();
This is pretty cool.

[mikecase (-- gone --), 2015-04-02 17:11:40]

https://codereview.chromium.org/1057783002/diff/20001/build/java_binary.gypi
File build/java_binary.gypi (right):

https://codereview.chromium.org/1057783002/diff/20001/build/java_binary.gypi#...
build/java_binary.gypi:1: # Copyright 2015 The Chromium Authors. All rights
reserved.
On 2015/04/02 14:21:43, jbudorick wrote:
> So this is in addition to build/host_jar.gypi? Do we need both? There's
> obviously a lot of overlap.

You're right. It looks like most of the things using host_jar.gypi are our junit
tests suites anyway. Will add the create_java_binary_script action to
host_jar.gypi and run the action is main_class is specified.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java:16:
* A Robolectric dependency resolver that looks for the Robolectric dependencies
On 2015/04/02 14:21:43, jbudorick wrote:
> If this is specific to Robolectric, should that be reflected in either the
> package or the name?

Done.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/ClasspathDependencyResolver.java:27:
super();
On 2015/04/02 14:21:43, jbudorick wrote:
> I _think_ this is unnecessary.

Done.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java:37:
LinkedList<String> testJarPaths = new LinkedList<String>();
On 2015/04/02 14:21:43, jbudorick wrote:
> I know I'm responsible for the code below that you based this on, but this
> should probably be something like:
> 
>   List<String> testJarPaths = new ArrayList<String>(testJars.length);
> 
> (and similarly for classes below)

Done.

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/LocalRobolectricTestRunner.java
(right):

https://codereview.chromium.org/1057783002/diff/20001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/LocalRobolectricTestRunner.java:32:
return new ClasspathDependencyResolver();
On 2015/04/02 14:21:43, jbudorick wrote:
> This is pretty cool.

Done.

[mikecase (-- gone --), 2015-04-02 17:18:40]

cc'ing dgn@google.com. This change will fix the Robolectric GN problems and
should also fix the NoClassDefFound errors you had.

[cjhopman, 2015-04-06 18:56:32]

https://codereview.chromium.org/1057783002/diff/60001/build/host_jar.gypi
File build/host_jar.gypi (right):

https://codereview.chromium.org/1057783002/diff/60001/build/host_jar.gypi#new...
build/host_jar.gypi:69: 'java_sources!': ['<!@(find <@(excluded_src_paths) -name
"*.java")'],
if excluded_src_paths is empty, this will do the wrong thing. Even if something
somewhere else guarantees that it isn't empty, I'd move it back into the
conditions (or at least comment about that guarantee.

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java
(right):

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java:39:
for (String testJar: testJars) {
Do we have to do this filtering of the jars on the class path? I'd prefer if we
didn't require the --test-jars arg.

[mikecase (-- gone --), 2015-04-06 21:03:41]

https://codereview.chromium.org/1057783002/diff/60001/build/host_jar.gypi
File build/host_jar.gypi (right):

https://codereview.chromium.org/1057783002/diff/60001/build/host_jar.gypi#new...
build/host_jar.gypi:69: 'java_sources!': ['<!@(find <@(excluded_src_paths) -name
"*.java")'],
On 2015/04/06 18:56:32, cjhopman wrote:
> if excluded_src_paths is empty, this will do the wrong thing. Even if
something
> somewhere else guarantees that it isn't empty, I'd move it back into the
> conditions (or at least comment about that guarantee.

Done, but I kept 2 changes I made to the condition statements. 

1) --excluded-classes should be --jar-excluded-classes
2) '">(main_class)" != ""' was always returning true no matter what. Changed to
'main_class != ""' which fixed the problem.

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java
(right):

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java:39:
for (String testJar: testJars) {
On 2015/04/06 18:56:32, cjhopman wrote:
> Do we have to do this filtering of the jars on the class path? I'd prefer if
we
> didn't require the --test-jars arg.

With this change, System.getProperty("java.class.path") now returns all of the
jars added to the classpath by the build files, instead of just the test jar.
And when trying to load all these classes, we run into lots of errors. for
example

Exception in thread "main" java.lang.SecurityException: Prohibited package name:
java.net
	at java.lang.ClassLoader.preDefineClass(ClassLoader.java:661)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:797)
	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
	at java.net.URLClassLoader.defineClass(URLClassLoader.java:449)
	at java.net.URLClassLoader.access$100(URLClassLoader.java:71)
        .


Exception in thread "main" java.lang.UnsatisfiedLinkError:
org.chromium.content.browser.DownloadController.nativeInit()V
	at org.chromium.content.browser.DownloadController.nativeInit(Native Method)
	at
org.chromium.content.browser.DownloadController.<init>(DownloadController.java:47)
	at
org.chromium.content.browser.DownloadController.<clinit>(DownloadController.java:20)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:187)
	at org.chromium.testing.local.JunitTestMain.classOrNull(JunitTestMain.java:74)
	at
org.chromium.testing.local.JunitTestMain.findClassesFromClasspath(JunitTestMain.java:59)
	at org.chromium.testing.local.JunitTestMain.main(JunitTestMain.java:93)


Exception in thread "main" java.lang.AssertionError: Couldn't create /data/anr.
	at com.android.server.am.DeviceMonitor.<clinit>(DeviceMonitor.java:82)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:187)
	at org.chromium.testing.local.JunitTestMain.classOrNull(JunitTestMain.java:78)
	at
org.chromium.testing.local.JunitTestMain.findClassesFromClasspath(JunitTestMain.java:59)
	at org.chromium.testing.local.JunitTestMain.main(JunitTestMain.java:101)


So, I could catch and ignore all the exceptions I ran into (LinkageError and
AssertionError when calling Class.forName(className) and SecurityException when
calling c.isAnnotationPresent(RunWith.class), or I could keep filtering out just
the test jar. I don't know if you would prefer one over the other.

[cjhopman, 2015-04-07 02:10:40]

lgtm

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
File
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java
(right):

https://codereview.chromium.org/1057783002/diff/60001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitTestMain.java:39:
for (String testJar: testJars) {
On 2015/04/06 21:03:39, mikecase wrote:
> On 2015/04/06 18:56:32, cjhopman wrote:
> > Do we have to do this filtering of the jars on the class path? I'd prefer if
> we
> > didn't require the --test-jars arg.
> 
> With this change, System.getProperty("java.class.path") now returns all of the
> jars added to the classpath by the build files, instead of just the test jar.
> And when trying to load all these classes, we run into lots of errors. for
> example
> 
> Exception in thread "main" java.lang.SecurityException: Prohibited package
name:
> http://java.net
> 	at java.lang.ClassLoader.preDefineClass(ClassLoader.java:661)
> 	at java.lang.ClassLoader.defineClass(ClassLoader.java:797)
> 	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
> 	at java.net.URLClassLoader.defineClass(URLClassLoader.java:449)
> 	at java.net.URLClassLoader.access$100(URLClassLoader.java:71)
>         .
> 
> 
> Exception in thread "main" java.lang.UnsatisfiedLinkError:
> org.chromium.content.browser.DownloadController.nativeInit()V
> 	at org.chromium.content.browser.DownloadController.nativeInit(Native Method)
> 	at
>
org.chromium.content.browser.DownloadController.<init>(DownloadController.java:47)
> 	at
>
org.chromium.content.browser.DownloadController.<clinit>(DownloadController.java:20)
> 	at java.lang.Class.forName0(Native Method)
> 	at java.lang.Class.forName(Class.java:187)
> 	at
org.chromium.testing.local.JunitTestMain.classOrNull(JunitTestMain.java:74)
> 	at
>
org.chromium.testing.local.JunitTestMain.findClassesFromClasspath(JunitTestMain.java:59)
> 	at org.chromium.testing.local.JunitTestMain.main(JunitTestMain.java:93)
> 
> 
> Exception in thread "main" java.lang.AssertionError: Couldn't create
/data/anr.
> 	at com.android.server.am.DeviceMonitor.<clinit>(DeviceMonitor.java:82)
> 	at java.lang.Class.forName0(Native Method)
> 	at java.lang.Class.forName(Class.java:187)
> 	at
org.chromium.testing.local.JunitTestMain.classOrNull(JunitTestMain.java:78)
> 	at
>
org.chromium.testing.local.JunitTestMain.findClassesFromClasspath(JunitTestMain.java:59)
> 	at org.chromium.testing.local.JunitTestMain.main(JunitTestMain.java:101)
> 
> 
> So, I could catch and ignore all the exceptions I ran into (LinkageError and
> AssertionError when calling Class.forName(className) and SecurityException
when
> calling c.isAnnotationPresent(RunWith.class), or I could keep filtering out
just
> the test jar. I don't know if you would prefer one over the other.

I think as is is fine.

[mikecase (-- gone --), 2015-04-07 16:37:53]

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

[mikecase (-- gone --), 2015-04-07 16:37:53]

+tedchoc for review of

chrome/android/BUILD.gn
content/public/android/BUILD.gn
and testing/android/junit/*

Basically, added a new GN template so there are changes to the BUILD.gn files.
And changed the code in testing/android/junit/* so that we can run the junit
tests in the way that GN builds them.

[Ted C, 2015-04-07 17:32:38]

On 2015/04/07 16:37:53, mikecase wrote:
> +tedchoc for review of
> 
> chrome/android/BUILD.gn
> content/public/android/BUILD.gn
> and testing/android/junit/*
> 
> Basically, added a new GN template so there are changes to the BUILD.gn files.
> And changed the code in testing/android/junit/* so that we can run the junit
> tests in the way that GN builds them.

owners lgtm

[jbudorick, 2015-04-07 18:46:40]

build/android/pylib lgtm

[mikecase (-- gone --), 2015-04-07 18:47:24]

The CQ bit was checked by mikecase@chromium.org

[commit-bot: I haz the power, 2015-04-07 18:48:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1057783002/80001

[commit-bot: I haz the power, 2015-04-07 19:45:25]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-07 19:46:22]

Patchset 5 (id:??) landed as
https://crrev.com/1ad64324d470c43b29854282d5ad5b1ff75dfc6c
Cr-Commit-Position: refs/heads/master@{#324093}