[Android] Adds python option for EMMA coverage

[Android] Adds python option for EMMA coverage

This is a piece of the overall java coverage change.
See: https://codereview.chromium.org/20210002/

NOTRY=True
BUG=255644
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=218531

[gkanwar1, 2013-08-20 18:23:07]

This is the Python piece of the java code coverage. No bots exercise this code,
so I'm not going to wait on them if this looks good.

[frankf, 2013-08-20 18:30:47]

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
File build/android/generate_emma_html.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
build/android/generate_emma_html.py:59: if os.path.exists(sources_file):
when would this not exist?

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
build/android/pylib/constants.py:96: def GetCoverageDir(build_type=None):
Where is this used?

[gkanwar1, 2013-08-20 18:37:13]

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
File build/android/generate_emma_html.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
build/android/generate_emma_html.py:59: if os.path.exists(sources_file):
On 2013/08/20 18:30:47, frankf wrote:
> when would this not exist?

It shouldn't ever not exist, since the metadata and sources files should be
generated at the same time, but in the name of robustness in case of future
changes I thought it would be a good idea to just put a check here instead of
failing. Perhaps we want it to fail so it is noticed if someone makes a change
to the coverage build system?

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
build/android/pylib/constants.py:96: def GetCoverageDir(build_type=None):
On 2013/08/20 18:30:47, frankf wrote:
> Where is this used?

This is used by the buildbot scripts. I put it with this CL, since that one will
be the most time-constrained at the end, and the fewer changes included in it,
the better.

[frankf, 2013-08-20 18:47:42]

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
File build/android/generate_emma_html.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
build/android/generate_emma_html.py:59: if os.path.exists(sources_file):
Right, robustness only makes sense if there are expected failure cases. It's
better to throw a exception instead of sweeping it under the rug. Please
consider robustness on the buildbot side though with GS which is expected to
fail.

On 2013/08/20 18:37:13, gkanwar1 wrote:
> On 2013/08/20 18:30:47, frankf wrote:
> > when would this not exist?
> 
> It shouldn't ever not exist, since the metadata and sources files should be
> generated at the same time, but in the name of robustness in case of future
> changes I thought it would be a good idea to just put a check here instead of
> failing. Perhaps we want it to fail so it is noticed if someone makes a change
> to the coverage build system?

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
build/android/pylib/constants.py:96: def GetCoverageDir(build_type=None):
Why do you need to take build_type as parameter?

On 2013/08/20 18:37:13, gkanwar1 wrote:
> On 2013/08/20 18:30:47, frankf wrote:
> > Where is this used?
> 
> This is used by the buildbot scripts. I put it with this CL, since that one
will
> be the most time-constrained at the end, and the fewer changes included in it,
> the better.

[gkanwar1, 2013-08-20 18:56:18]

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
File build/android/generate_emma_html.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/generate_emm...
build/android/generate_emma_html.py:59: if os.path.exists(sources_file):
On 2013/08/20 18:47:43, frankf wrote:
> Right, robustness only makes sense if there are expected failure cases. It's
> better to throw a exception instead of sweeping it under the rug. Please
> consider robustness on the buildbot side though with GS which is expected to
> fail.
> 
> On 2013/08/20 18:37:13, gkanwar1 wrote:
> > On 2013/08/20 18:30:47, frankf wrote:
> > > when would this not exist?
> > 
> > It shouldn't ever not exist, since the metadata and sources files should be
> > generated at the same time, but in the name of robustness in case of future
> > changes I thought it would be a good idea to just put a check here instead
of
> > failing. Perhaps we want it to fail so it is noticed if someone makes a
change
> > to the coverage build system?
> 

Okay, removed the check.

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/23293006/diff/2001/build/android/pylib/consta...
build/android/pylib/constants.py:96: def GetCoverageDir(build_type=None):
On 2013/08/20 18:47:43, frankf wrote:
> Why do you need to take build_type as parameter?
> 
> On 2013/08/20 18:37:13, gkanwar1 wrote:
> > On 2013/08/20 18:30:47, frankf wrote:
> > > Where is this used?
> > 
> > This is used by the buildbot scripts. I put it with this CL, since that one
> will
> > be the most time-constrained at the end, and the fewer changes included in
it,
> > the better.
> 

Test-generated files (like logs, etc.) generally are placed into the
out/<target> folder, rather than the top-level out folder, so this needs to
accept build_type to place it in the correct folder.

[gkanwar1, 2013-08-20 19:03:26]

GetCoverageDir removed, will just define this inside the relevant buildbot
function.

[frankf, 2013-08-20 19:04:13]

lgtm

[gkanwar1, 2013-08-20 19:10:53]

On 2013/08/20 19:04:13, frankf wrote:
> lgtm

Thanks. None of the changes since patch set 1 are tested by the trybots, so I'm
landing this.

[commit-bot: I haz the power, 2013-08-20 19:11:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gkanwar@chromium.org/23293006/14001

[commit-bot: I haz the power, 2013-08-20 19:11:34]

Change committed as 218531