We need to build browser_tests during 'compile' step, so enabling in gypi. Disabled from running in…

We need to build browser_tests during 'compile' step, so enabling in gypi. Disabled from running in coverage_posix.py. 

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

[John Grabowski, 2012-11-09 18:47:54]

LGTM

I believe you also need OWNERS approval.

https://codereview.chromium.org/11377077/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/11377077/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:4816: # We need to build this during compile step, so
enabling here.
Add comment stating the test is disabled in coverage_posix.py else things will
be a mystery

https://codereview.chromium.org/11377077/diff/1/tools/code_coverage/coverage_...
File tools/code_coverage/coverage_posix.py (right):

https://codereview.chromium.org/11377077/diff/1/tools/code_coverage/coverage_...
tools/code_coverage/coverage_posix.py:654: continue
Add a logging.info() call here to make debugging easier if we forget about this
change.

[pshenoy, 2012-11-09 19:00:48]

https://codereview.chromium.org/11377077/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/11377077/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:4816: # We need to build this during compile step, so
enabling here.
On 2012/11/09 18:47:54, John Grabowski wrote:
> Add comment stating the test is disabled in coverage_posix.py else things will
> be a mystery
It's already updated in the comment above.

https://codereview.chromium.org/11377077/diff/1/tools/code_coverage/coverage_...
File tools/code_coverage/coverage_posix.py (right):

https://codereview.chromium.org/11377077/diff/1/tools/code_coverage/coverage_...
tools/code_coverage/coverage_posix.py:654: continue
On 2012/11/09 18:47:54, John Grabowski wrote:
> Add a logging.info() call here to make debugging easier if we forget about
this
> change.

Done.

[pshenoy, 2012-11-09 19:01:52]

Hi Nico, Looking for OWNER approval.

--
Prashanth

[John Grabowski, 2012-11-09 19:02:06]

LGTM

[Nico, 2012-11-09 19:09:03]

lgtm

https://codereview.chromium.org/11377077/diff/5001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/11377077/diff/5001/chrome/chrome_tests.gypi#n...
chrome/chrome_tests.gypi:4816: # We need to build this during compile step, so
enabling here.
Can you just remove the 3 comment lines above?

[pshenoy, 2012-11-09 19:16:04]

https://codereview.chromium.org/11377077/diff/5001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/11377077/diff/5001/chrome/chrome_tests.gypi#n...
chrome/chrome_tests.gypi:4816: # We need to build this during compile step, so
enabling here.
On 2012/11/09 19:09:03, Nico wrote:
> Can you just remove the 3 comment lines above?
Done. Retained the comment saying it is disabled in coverage_posix.py. Do you
suggest to remove that as well?

[Nico, 2012-11-09 19:22:37]

lgtm

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

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

[commit-bot: I haz the power, 2012-11-09 21:35:50]

Change committed as 166975