Goal here is to collect total_coverage and unittests coverage numbers separately. Below are the ste…

Goal here is to collect total_coverage and unittests coverage numbers separately. Below are the steps performed.
1. Added one more step in AddRunCoverageBundles() function in 'chromium_commands.py' to run coverage_posix.py script with different set of parameters (Run only unittests).
2. Modified 'coverage_posix.py' to save the coverage.info file into separate folder depending on parameters passed.(This is done in a separate CL https://codereview.chromium.org/11274053)
3. Modified 'process_coverage.py' to process each of the coverage.info file generated and save it onto separate folders.
4. Finally in 'archive_coverage.py', upload the each of these folders onto web server.

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

[John Grabowski, 2012-10-30 21:34:02]

Best to get an ell gee tee emm from chase (or designated chrome-infrastructure
person) before landing.

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/archive_coverage.py (right):

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:178: 
remove blank line

need to sys.exit(ac.run()) and auc.run()

[cmp, 2012-11-01 23:46:22]

lgtm with changes

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/archive_coverage.py (right):

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:63: 
remove this line

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:128: if self.is_posix:
let's rewrite this so lines 127 is:

if os.path.exists(self.from_dir) and self.is_posix:

then line 128 goes away and lines 129-150 get put back at their original indent

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:184: Main()
let's put the sys.exit here:
  sys.exit(Main())
and then in Main, you'll return int codes (eg. after line 180, return 0)

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:84: print "total_coverage
directory exists"
single quotes instead of double quotes

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:94: '--html',
os.path.join(target_dir, 'total_coverage', 'coverage_croc_html'),
wrap at 80 chars

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:99: print "unittests_coverage
directory exists"
single quotes instead of double quotes

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:100: unittests_cov_file =
os.path.join(target_dir, 'unittests_coverage', 'coverage.info')
wrap at 80 chars

https://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:109: '--html',
os.path.join(target_dir, 'unittests_coverage', 'coverage_croc_html'),
wrap at 80 chars

[pshenoy, 2012-11-05 18:47:22]

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
File build/scripts/slave/chromium/archive_coverage.py (right):

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/archive_coverage.py:63: 
On 2012/11/01 23:46:22, cmp wrote:
> remove this line

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/archive_coverage.py:128: if self.is_posix:
On 2012/11/01 23:46:22, cmp wrote:
> let's rewrite this so lines 127 is:
> 
> if os.path.exists(self.from_dir) and self.is_posix:
> 
> then line 128 goes away and lines 129-150 get put back at their original
indent

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/archive_coverage.py:178: 
On 2012/10/30 21:34:02, John Grabowski wrote:
> remove blank line
> 
> need to sys.exit(ac.run()) and auc.run()
>  
Main() is called as sys.exit(Main())

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/archive_coverage.py:184: Main()
On 2012/11/01 23:46:22, cmp wrote:
> let's put the sys.exit here:
>   sys.exit(Main())
> and then in Main, you'll return int codes (eg. after line 180, return 0)

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/process_coverage.py:84: print "total_coverage
directory exists"
On 2012/11/01 23:46:22, cmp wrote:
> single quotes instead of double quotes

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/process_coverage.py:94: '--html',
os.path.join(target_dir, 'total_coverage', 'coverage_croc_html'),
On 2012/11/01 23:46:22, cmp wrote:
> wrap at 80 chars

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/process_coverage.py:99: print "unittests_coverage
directory exists"
On 2012/11/01 23:46:22, cmp wrote:
> single quotes instead of double quotes

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/process_coverage.py:100: unittests_cov_file =
os.path.join(target_dir, 'unittests_coverage', 'coverage.info')
On 2012/11/01 23:46:22, cmp wrote:
> wrap at 80 chars

Done.

http://codereview.chromium.org/11273046/diff/2001/build/scripts/slave/chromiu...
build/scripts/slave/chromium/process_coverage.py:109: '--html',
os.path.join(target_dir, 'unittests_coverage', 'coverage_croc_html'),
On 2012/11/01 23:46:22, cmp wrote:
> wrap at 80 chars

Done.

[cmp, 2012-11-05 21:09:47]

I don't see the latest patch set here.  Can you upload it?

[pshenoy, 2012-11-05 21:17:22]

Sorry. Uploaded now.

[cmp, 2012-11-05 21:26:35]

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/archive_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:55: coverage_folder_name,
indent by one space

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:114: return
I assume main_common returns something useful.  If so, instead of returning
None, you should return any exit return value of main_common here.

[pshenoy, 2012-11-05 21:52:14]

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/archive_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:55: coverage_folder_name,
On 2012/11/05 21:26:35, cmp wrote:
> indent by one space
I think it is properly indented.

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:114: return
On 2012/11/05 21:26:35, cmp wrote:
> I assume main_common returns something useful.  If so, instead of returning
> None, you should return any exit return value of main_common here.

Done. Not sure how do I return the result of previous main_common() call.

[cmp, 2012-11-05 21:59:28]

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/archive_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/archive_coverage.py:55: coverage_folder_name,
You're right, it looked out of alignment to be but I was wrong.  Thanks for
pointing that out.

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:114: return
On 2012/11/05 21:52:14, pshenoy wrote:
> On 2012/11/05 21:26:35, cmp wrote:
> > I assume main_common returns something useful.  If so, instead of returning
> > None, you should return any exit return value of main_common here.
> 
> Done. Not sure how do I return the result of previous main_common() call. 

There are a couple approaches:
- add result values together (0 is success), but this gets confusing
- set result to 0 to start with, then set it to the result of the first
main_common(), then carefully call the second main_common() and either keep
result if the second main_common() is 0, or set result to the return value of
the second main_common()

http://codereview.chromium.org/11273046/diff/12004/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/12004/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:101: unittests_cov_file =
os.path.join(target_dir, 'unittests_coverage', 'coverage.info')
wrap at 80 chars

[pshenoy, 2012-11-05 22:22:23]

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/11001/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:114: return
On 2012/11/05 21:59:29, cmp wrote:
> On 2012/11/05 21:52:14, pshenoy wrote:
> > On 2012/11/05 21:26:35, cmp wrote:
> > > I assume main_common returns something useful.  If so, instead of
returning
> > > None, you should return any exit return value of main_common here.
> > 
> > Done. Not sure how do I return the result of previous main_common() call. 
> 
> There are a couple approaches:
> - add result values together (0 is success), but this gets confusing
> - set result to 0 to start with, then set it to the result of the first
> main_common(), then carefully call the second main_common() and either keep
> result if the second main_common() is 0, or set result to the return value of
> the second main_common()

Thanks. Done.

http://codereview.chromium.org/11273046/diff/12004/build/scripts/slave/chromi...
File build/scripts/slave/chromium/process_coverage.py (right):

http://codereview.chromium.org/11273046/diff/12004/build/scripts/slave/chromi...
build/scripts/slave/chromium/process_coverage.py:101: unittests_cov_file =
os.path.join(target_dir, 'unittests_coverage', 'coverage.info')
On 2012/11/05 21:59:29, cmp wrote:
> wrap at 80 chars

Done.

[cmp, 2012-11-05 22:23:35]

lgtm

[John Grabowski, 2012-11-06 17:53:45]

LGTM

Be sure to coordinate master restart with cmp

[commit-bot: I haz the power, 2012-11-06 19:25:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pshenoy@chromium.org/11273046/8002

[commit-bot: I haz the power, 2012-11-06 19:27:43]

Presubmit check for 11273046-8002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found line ending with white spaces in:

***************
build/scripts/master/factory/chromium_commands.py, line 1394
***************

Presubmit checks took 101.1s to calculate.

[commit-bot: I haz the power, 2012-11-06 19:27:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pshenoy@chromium.org/11273046/8002

[commit-bot: I haz the power, 2012-11-06 19:29:20]

Presubmit check for 11273046-8002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found line ending with white spaces in:

***************
build/scripts/master/factory/chromium_commands.py, line 1394
***************

Presubmit checks took 78.2s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[commit-bot: I haz the power, 2012-11-06 19:36:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pshenoy@chromium.org/11273046/12005

[commit-bot: I haz the power, 2012-11-06 19:37:44]

Change committed as 166246