Add json output to Junit tests.

Add json output to Junit tests.

This change will allow us to get the test results information
to our testrunner without having to parse stdout. This will fix
the issue of junit tests not showing up on the test report step
on our bots.

BUG=465755
Committed: https://crrev.com/c638a07bf65457b24956785146d3429045a47fe2
Cr-Commit-Position: refs/heads/master@{#323255}

[mikecase (-- gone --), 2015-03-11 18:37:36]

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

[mikecase (-- gone --), 2015-03-11 18:37:37]

This will fix the issue...
https://code.google.com/p/chromium/issues/detail?id=465755

Basically, that issue was caused because we never called
report_results.LogFull() on the junit results. This change  lets us create a
base_test_result.TestRunResults() object and calls LogFull using it.

[mikecase (-- gone --), 2015-03-11 18:44:27]

https://codereview.chromium.org/1003463002/diff/20001/testing/android/junit/j...
File testing/android/junit/junit_test.gyp (right):

https://codereview.chromium.org/1003463002/diff/20001/testing/android/junit/j...
testing/android/junit/junit_test.gyp:14:
'../../../third_party/robolectric/robolectric.gyp:robolectric_jar',
Just to clarify. I need to add this as a dependency because I am using
org.json.JSONObject and org.json.JSONArray from the Android SDK. Once the
robolectric custom testrunner CL gets in, this line will be here anyways.

Otherwise, I need to find some other json java library to use.

[jbudorick, 2015-03-12 19:41:49]

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/jun...
File build/android/pylib/junit/test_runner.py (right):

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/jun...
build/android/pylib/junit/test_runner.py:32: '%s.jar' % self._test_suite),
'-json-results-file',
nit: drop this onto its own line w/ json_file.name

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
File build/android/pylib/results/json_results.py (right):

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:35: #           "losless_snippet":
"".
nit: .?

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:44: #           "losless_snippet":
"".
nit: .?

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:126: elif s == 'UNKNOWN':
This should just be an else case.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:132: for test in testsuite_run:
for test, test_runs in testsuite_run.iteritems():
  results_list.extend(
      base_test_result.BaseTestResult(
          test, string_as_status(tr['status']), duration=tr['elapsed_time_ms'])
      for tr in test_runs)

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:135: name = test
Why does this exist?

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

https://codereview.chromium.org/1003463002/diff/60001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitListener.java:17:
public class JunitListener extends RunListener {
Don't add the Json listening logic into the GtestListener and rename it -- just
create another listener for Json.

[mikecase (-- gone --), 2015-03-13 01:07:24]

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/jun...
File build/android/pylib/junit/test_runner.py (right):

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/jun...
build/android/pylib/junit/test_runner.py:32: '%s.jar' % self._test_suite),
'-json-results-file',
On 2015/03/12 19:41:49, jbudorick wrote:
> nit: drop this onto its own line w/ json_file.name

Done.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
File build/android/pylib/results/json_results.py (right):

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:35: #           "losless_snippet":
"".
On 2015/03/12 19:41:49, jbudorick wrote:
> nit: .?

Done.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:44: #           "losless_snippet":
"".
On 2015/03/12 19:41:49, jbudorick wrote:
> nit: .?

Done.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:126: elif s == 'UNKNOWN':
On 2015/03/12 19:41:49, jbudorick wrote:
> This should just be an else case.

Done.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:132: for test in testsuite_run:
On 2015/03/12 19:41:49, jbudorick wrote:
> for test, test_runs in testsuite_run.iteritems():
>   results_list.extend(
>       base_test_result.BaseTestResult(
>           test, string_as_status(tr['status']),
duration=tr['elapsed_time_ms'])
>       for tr in test_runs)

Done.

https://codereview.chromium.org/1003463002/diff/60001/build/android/pylib/res...
build/android/pylib/results/json_results.py:135: name = test
On 2015/03/12 19:41:49, jbudorick wrote:
> Why does this exist?

Done.

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

https://codereview.chromium.org/1003463002/diff/60001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JunitListener.java:17:
public class JunitListener extends RunListener {
On 2015/03/12 19:41:49, jbudorick wrote:
> Don't add the Json listening logic into the GtestListener and rename it --
just
> create another listener for Json.

Yeah, I kinda figured I should do this. Its just the two listeners are basically
the same. But, added a JsonListener and brought back GtestListener.

[kzaikin, 2015-03-19 09:12:10]

konstantin.zaikin@gmail.com changed reviewers:
+ konstantin.zaikin@gmail.com

[kzaikin, 2015-03-19 09:12:11]

https://codereview.chromium.org/1003463002/diff/80001/build/android/test_runn...
File build/android/test_runner.py (right):

https://codereview.chromium.org/1003463002/diff/80001/build/android/test_runn...
build/android/test_runner.py:771: results, exit_code =
junit_dispatcher.RunTests(tests, runner_factory)
In fact your dispatcher returns bool as second tuple item, not int.
True when tests have passed, not 0.
>return (test_run_results, test_run_passed)

[kzaikin, 2015-03-20 11:38:18]

https://codereview.chromium.org/1003463002/diff/80001/testing/android/junit/j...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/80001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:80:
return test.getClassName() + "." + test.getMethodName();
Please use # sign between class name and test name like it is done in
instrumentation tests

[mikecase (-- gone --), 2015-03-23 20:38:44]

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

[mikecase (-- gone --), 2015-03-23 20:38:45]

Adding nyquist for review of testing/android

https://codereview.chromium.org/1003463002/diff/80001/build/android/test_runn...
File build/android/test_runner.py (right):

https://codereview.chromium.org/1003463002/diff/80001/build/android/test_runn...
build/android/test_runner.py:771: results, exit_code =
junit_dispatcher.RunTests(tests, runner_factory)
On 2015/03/19 09:12:11, kzaikin wrote:
> In fact your dispatcher returns bool as second tuple item, not int.
> True when tests have passed, not 0.
> >return (test_run_results, test_run_passed)

Good catch. Fixed

https://codereview.chromium.org/1003463002/diff/80001/testing/android/junit/j...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/80001/testing/android/junit/j...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:80:
return test.getClassName() + "." + test.getMethodName();
On 2015/03/20 11:38:18, kzaikin wrote:
> Please use # sign between class name and test name like it is done in
> instrumentation tests

Done.

[nyquist, 2015-03-23 23:53:48]

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:10: 
Nit: I don't think this empty line should be here.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:24:
private JSONObject mBaseJsonObject;
final

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:24:
private JSONObject mBaseJsonObject;
final for all these three fields

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:55:
testInfoJsonObject.put("losless_snippet", "");
Is this missing an s in lossless?

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:73:
stream.print(mBaseJsonObject);
Does this end with a newline or do you have to flush it? Also; should this be
closed?

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
File testing/android/junit/junit_test.gyp (right):

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/junit_test.gyp:8: # GN:
//testing/android/junit:junit_test_support
You probably meant to update the GN target too.

[mikecase (-- gone --), 2015-03-24 00:35:24]

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:10: 
On 2015/03/23 23:53:48, nyquist wrote:
> Nit: I don't think this empty line should be here.

Done.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:24:
private JSONObject mBaseJsonObject;
On 2015/03/23 23:53:48, nyquist wrote:
> final for all these three fields

Done.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:24:
private JSONObject mBaseJsonObject;
On 2015/03/23 23:53:48, nyquist wrote:
> final

Done.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:55:
testInfoJsonObject.put("losless_snippet", "");
On 2015/03/23 23:53:48, nyquist wrote:
> Is this missing an s in lossless?

Its misspelled everywhere else like this. Keeping it this way right now for
consistency (because things expect the field to be called 'losless'). Will fix
the mispellings everywhere in another CL.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:73:
stream.print(mBaseJsonObject);
On 2015/03/23 23:53:48, nyquist wrote:
> Does this end with a newline or do you have to flush it? Also; should this be
> closed?

added a call to stream.close() which will flush and close the file.

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
File testing/android/junit/junit_test.gyp (right):

https://codereview.chromium.org/1003463002/diff/120001/testing/android/junit/...
testing/android/junit/junit_test.gyp:8: # GN:
//testing/android/junit:junit_test_support
On 2015/03/23 23:53:48, nyquist wrote:
> You probably meant to update the GN target too.

After a rebase, this line was no longer needed (added in a separate CL which
made changes to the GN and gyp files). Removing this file from this CL.

[nyquist, 2015-03-31 22:16:55]

testing/android lgtm

https://codereview.chromium.org/1003463002/diff/140001/testing/android/junit/...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/140001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:77: }
catch (Exception e) {
Is this meant to only catch RuntimeException?

[mikecase (-- gone --), 2015-03-31 23:20:29]

https://codereview.chromium.org/1003463002/diff/140001/testing/android/junit/...
File testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java
(right):

https://codereview.chromium.org/1003463002/diff/140001/testing/android/junit/...
testing/android/junit/java/src/org/chromium/testing/local/JsonLogger.java:77: }
catch (Exception e) {
On 2015/03/31 22:16:55, nyquist wrote:
> Is this meant to only catch RuntimeException?

Yes, I'll change it to explicitly catch Runtime exceptions to make its purpose
clear.

[jbudorick, 2015-04-01 01:21:34]

build/android lgtm

[mikecase (-- gone --), 2015-04-01 14:50:11]

The CQ bit was checked by mikecase@chromium.org

[mikecase (-- gone --), 2015-04-01 14:50:12]

The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/1003463002/#ps180001
(title: "Rebase and addressed nyquist's comment.")

[commit-bot: I haz the power, 2015-04-01 14:50:21]

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

[commit-bot: I haz the power, 2015-04-01 16:35:47]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-04-01 16:36:25]

Patchset 10 (id:??) landed as
https://crrev.com/c638a07bf65457b24956785146d3429045a47fe2
Cr-Commit-Position: refs/heads/master@{#323255}