Fixed a crash in the coverage script....

Fixed a crash in the coverage script.

The .coverage -> lcov converter tool ended up crashing the coverage script as it ran out of memory while processing a huge file (it threw an .Net 'Out of Memory' exception). I have now changed it to generate smaller intermediate LCOV files (one per test binary) and then consolidate them all into one large LCOV file.


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

[Allen, 2008-10-24 20:55:07]

Looks good!

http://codereview.chromium.org/8157/diff/1/2
File tools/code_coverage/coverage.py (right):

http://codereview.chromium.org/8157/diff/1/2#newcode205
Line 205: retcode = subprocess.call(counters_command)
If you wanted to interact with the process, p = subprocess.Popen() then p.wait()
might un-confuse things.  (But it looks like you don't, and furthermore I'm not
sure if RunCommandFull does a wait() or not)

[Patrick Johnson, 2008-10-24 21:00:43]

LGTM

You crashed Python?  I'm impressed.

http://codereview.chromium.org/8157/diff/1/2
File tools/code_coverage/coverage.py (right):

http://codereview.chromium.org/8157/diff/1/2#newcode150
Line 150: else:
Nit: You can get rid of the else here.

http://codereview.chromium.org/8157/diff/1/2#newcode201
Line 201: coverage_file))
Nit: Indentation looks off by one space.

http://codereview.chromium.org/8157/diff/1/2#newcode216
Line 216: (os.path.join(self.tools_path, 'vsperfcmd.exe')))
Nit: Indentation looks off by one space.

http://codereview.chromium.org/8157/diff/1/2#newcode249
Line 249: (self.revision))
Nit: Indentation looks off by one space.

http://codereview.chromium.org/8157/diff/1/2#newcode326
Line 326: if cov.SetUp(windows_binaries) == True:
The "== True" is not necessary.

[Niranjan Tulpule, 2008-10-24 21:19:49]

The crash was in coverage_analyzer.exe (System.OutOfMemoryException) and not in
python.

http://codereview.chromium.org/8157/diff/1/2
File tools/code_coverage/coverage.py (right):

http://codereview.chromium.org/8157/diff/1/2#newcode150
Line 150: else:
On 2008/10/24 21:00:43, Patrick Johnson wrote:
> Nit: You can get rid of the else here.

Done.

http://codereview.chromium.org/8157/diff/1/2#newcode201
Line 201: coverage_file))
On 2008/10/24 21:00:43, Patrick Johnson wrote:
> Nit: Indentation looks off by one space.

Done.

http://codereview.chromium.org/8157/diff/1/2#newcode205
Line 205: retcode = subprocess.call(counters_command)
On 2008/10/24 20:55:07, Allen wrote:
> If you wanted to interact with the process, p = subprocess.Popen() then
p.wait()
> might un-confuse things.  (But it looks like you don't, and furthermore I'm
not
> sure if RunCommandFull does a wait() or not)

RunCommandFull does a wait(), but since the VSPerfCmd spawns an extra processes
(used for monitoring code coverage), RunCommandFull gets confused and the wait()
never returns, even though the original VSPerfCmd has exited. Hence this
workaround.

http://codereview.chromium.org/8157/diff/1/2#newcode216
Line 216: (os.path.join(self.tools_path, 'vsperfcmd.exe')))
On 2008/10/24 21:00:43, Patrick Johnson wrote:
> Nit: Indentation looks off by one space.

Done.

http://codereview.chromium.org/8157/diff/1/2#newcode249
Line 249: (self.revision))
On 2008/10/24 21:00:43, Patrick Johnson wrote:
> Nit: Indentation looks off by one space.

Done.

http://codereview.chromium.org/8157/diff/1/2#newcode326
Line 326: if cov.SetUp(windows_binaries) == True:
On 2008/10/24 21:00:43, Patrick Johnson wrote:
> The "== True" is not necessary.

Done.

[Patrick Johnson, 2008-10-24 21:25:32]

On 2008/10/24 21:19:49, Niranjan Tulpule wrote:
> The crash was in coverage_analyzer.exe (System.OutOfMemoryException) and not
in
> python.

Ah!  That would explain the .Net 'Out of Memory' exception.  Not really a very
Python-like error, that.