Merged Android and non-Android code paths to reuse code in GTestTest

Merged Android and non-Android code paths to reuse code in GTestTest

With this change results for Android GTests will be correctly parsed and
interpreted by the *-ng bots.

BUG=451020
R=phajdan.jr@chromium.org

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

[Sergiy Byelozyorov, 2015-01-29 17:09:58]

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
File scripts/slave/recipe_modules/chromium/steps.py (right):

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
scripts/slave/recipe_modules/chromium/steps.py:250: kwargs['json_results_file']
= gtest_results_file
Since Android test launcher tries to return test results in the same format as
gTest tests, we should be able to parse these results with the same placeholder
type. Since Android instrumentation tests are using the same launcher, we can
probably also replace custom parsing code there with api.json.gtest_results too,
but I'll do this in a separate CL.

[Paweł Hajdan Jr., 2015-01-29 20:19:12]

LGTM w/comments

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
File scripts/slave/recipe_modules/chromium/steps.py (right):

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
scripts/slave/recipe_modules/chromium/steps.py:248: isolate_fpath =
api.path['checkout'].join(self._android_isolate_path)
nit: Instead of somewhat weird isolate_fpath why not just isolate_path (without
the "f")?

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
scripts/slave/recipe_modules/chromium/steps.py:250: kwargs['json_results_file']
= gtest_results_file
On 2015/01/29 17:09:58, Sergiy Byelozyorov wrote:
> Since Android test launcher tries to return test results in the same format as
> gTest tests, we should be able to parse these results with the same
placeholder
> type. Since Android instrumentation tests are using the same launcher, we can
> probably also replace custom parsing code there with api.json.gtest_results
too,
> but I'll do this in a separate CL.

Have you verified this?

Both are JSON, but Android might be emulating the LayoutTests format instead of
our C++ gtest launcher. Just making sure it's been empirically checked rather
just being an assumption that since both are JSON they must be compatible.

[Sergiy Byelozyorov, 2015-01-29 20:22:37]

On 2015/01/29 20:19:12, Paweł Hajdan Jr. wrote:
> LGTM w/comments
> 
>
https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
> File scripts/slave/recipe_modules/chromium/steps.py (right):
> 
>
https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
> scripts/slave/recipe_modules/chromium/steps.py:248: isolate_fpath =
> api.path['checkout'].join(self._android_isolate_path)
> nit: Instead of somewhat weird isolate_fpath why not just isolate_path
(without
> the "f")?
> 
>
https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
> scripts/slave/recipe_modules/chromium/steps.py:250:
kwargs['json_results_file']
> = gtest_results_file
> On 2015/01/29 17:09:58, Sergiy Byelozyorov wrote:
> > Since Android test launcher tries to return test results in the same format
as
> > gTest tests, we should be able to parse these results with the same
> placeholder
> > type. Since Android instrumentation tests are using the same launcher, we
can
> > probably also replace custom parsing code there with api.json.gtest_results
> too,
> > but I'll do this in a separate CL.
> 
> Have you verified this?
> 
> Both are JSON, but Android might be emulating the LayoutTests format instead
of
> our C++ gtest launcher. Just making sure it's been empirically checked rather
> just being an assumption that since both are JSON they must be compatible.

I based my assumption on the fact that keys have the same names and this
comment:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli....
AFAIK, GTest uses this method too.

[Sergiy Byelozyorov, 2015-01-29 20:24:22]

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
File scripts/slave/recipe_modules/chromium/steps.py (right):

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
scripts/slave/recipe_modules/chromium/steps.py:248: isolate_fpath =
api.path['checkout'].join(self._android_isolate_path)
On 2015/01/29 20:19:12, Paweł Hajdan Jr. wrote:
> nit: Instead of somewhat weird isolate_fpath why not just isolate_path
(without
> the "f")?

Done.

https://codereview.chromium.org/892463002/diff/1/scripts/slave/recipe_modules...
scripts/slave/recipe_modules/chromium/steps.py:250: kwargs['json_results_file']
= gtest_results_file
On 2015/01/29 20:19:12, Paweł Hajdan Jr. wrote:
> On 2015/01/29 17:09:58, Sergiy Byelozyorov wrote:
> > Since Android test launcher tries to return test results in the same format
as
> > gTest tests, we should be able to parse these results with the same
> placeholder
> > type. Since Android instrumentation tests are using the same launcher, we
can
> > probably also replace custom parsing code there with api.json.gtest_results
> too,
> > but I'll do this in a separate CL.
> 
> Have you verified this?
> 
> Both are JSON, but Android might be emulating the LayoutTests format instead
of
> our C++ gtest launcher. Just making sure it's been empirically checked rather
> just being an assumption that since both are JSON they must be compatible.

(sorry for not replying directly to the comment first time)

I based my assumption on the fact that keys have the same names and this
comment:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli....
AFAIK, GTest uses this method too.

[Sergiy Byelozyorov, 2015-01-29 20:24:27]

The CQ bit was checked by sergiyb@chromium.org

[commit-bot: I haz the power, 2015-01-29 20:24:44]

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

[commit-bot: I haz the power, 2015-01-29 20:26:02]

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