Setup JS unittests for Chromoting

Setup JS unittests for Chromoting

BUG=none
TEST=remoting_unittests --gtest_filter=DebugLogUnitTest.*


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

[Sheridan Rawlins, 2011-11-19 02:36:26]

http://codereview.chromium.org/8599010/diff/6001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/8599010/diff/6001/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:1157: 'js_unittest_rules.gypi',
Add comment // TODO(scr): Use this in browser_tests too.

http://codereview.chromium.org/8599010/diff/6001/remoting/run_all_unittests.cc
File remoting/run_all_unittests.cc (right):

http://codereview.chromium.org/8599010/diff/6001/remoting/run_all_unittests.c...
remoting/run_all_unittests.cc:9: #include "chrome/common/chrome_paths.h"
You might have to move this runner somewhere else to pass the check_deps - maybe
chrome/test?

[garykac, 2011-11-21 20:46:17]

+jcivelli for chrome/test

http://codereview.chromium.org/8599010/diff/6001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/8599010/diff/6001/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:1157: 'js_unittest_rules.gypi',
On 2011/11/19 02:36:26, Sheridan Rawlins wrote:
> Add comment // TODO(scr): Use this in browser_tests too.

Done.

http://codereview.chromium.org/8599010/diff/6001/remoting/run_all_unittests.cc
File remoting/run_all_unittests.cc (right):

http://codereview.chromium.org/8599010/diff/6001/remoting/run_all_unittests.c...
remoting/run_all_unittests.cc:9: #include "chrome/common/chrome_paths.h"
On 2011/11/19 02:36:26, Sheridan Rawlins wrote:
> You might have to move this runner somewhere else to pass the check_deps -
maybe
> chrome/test?

Done.

[jcivelli, 2011-11-21 21:54:15]

LGTM

http://codereview.chromium.org/8599010/diff/10001/chrome/test/run_all_unittes...
File chrome/test/run_all_unittests.cc (right):

http://codereview.chromium.org/8599010/diff/10001/chrome/test/run_all_unittes...
chrome/test/run_all_unittests.cc:11: #include "base/file_path.h"
I don't think you need these 3 includes.

[garykac, 2011-11-21 22:04:53]

http://codereview.chromium.org/8599010/diff/10001/chrome/test/run_all_unittes...
File chrome/test/run_all_unittests.cc (right):

http://codereview.chromium.org/8599010/diff/10001/chrome/test/run_all_unittes...
chrome/test/run_all_unittests.cc:11: #include "base/file_path.h"
On 2011/11/21 21:54:15, jcivelli wrote:
> I don't think you need these 3 includes.

Removed.

[Sheridan Rawlins, 2011-11-21 22:13:15]

http://codereview.chromium.org/8599010/diff/16001/chrome/test/run_all_unittes...
File chrome/test/run_all_unittests.cc (right):

http://codereview.chromium.org/8599010/diff/16001/chrome/test/run_all_unittes...
chrome/test/run_all_unittests.cc:1: // Copyright (c) 2011 The Chromium Authors.
All rights reserved.
There's already a chrome/test/base/run_all_unittests.cc  How is this different,
and is it different enough to merit its own executable?  Why just build
chromoting tests into unit_tests?  Minimally, can the same
chrome/test/base/run_all_unittests.cc be used for both?

[garykac, 2011-11-23 21:36:25]

On 2011/11/21 22:13:15, Sheridan Rawlins wrote:
>
http://codereview.chromium.org/8599010/diff/16001/chrome/test/run_all_unittes...
> File chrome/test/run_all_unittests.cc (right):
> 
>
http://codereview.chromium.org/8599010/diff/16001/chrome/test/run_all_unittes...
> chrome/test/run_all_unittests.cc:1: // Copyright (c) 2011 The Chromium
Authors.
> All rights reserved.
> There's already a chrome/test/base/run_all_unittests.cc  How is this
different,
> and is it different enough to merit its own executable?  Why just build
> chromoting tests into unit_tests?  Minimally, can the same
> chrome/test/base/run_all_unittests.cc be used for both?

PTAL

Changes:
* Fixed up the comments
* Added the dependencies required for Windows.
* Fixed some hard-coded paths in some gypi files.

[garykac, 2011-11-23 23:44:57]

+bradnelson to review the gypi changes.

I added a bunch of "<(DEPTH)" prefixes to paths in some gypi files. This is
needed to remove the dependency on being included from within src/chrome.

[Sheridan Rawlins, 2011-11-24 00:58:58]

LGTM for rule refactor and gtestjs, with commenting nit.

http://codereview.chromium.org/8599010/diff/22001/remoting/webapp/me2mom/debu...
File remoting/webapp/me2mom/debug_log.gtestjs (right):

http://codereview.chromium.org/8599010/diff/22001/remoting/webapp/me2mom/debu...
remoting/webapp/me2mom/debug_log.gtestjs:21: TEST_F('DebugLogUnitTest',
'Sample', function() {
nit: Can you pick a more descriptive test name than "Sample" and add a comment? 
This in particular will test that loading debug_log.js doesn't have any console
errors.

[garykac, 2011-11-24 01:04:05]

On 2011/11/24 00:58:58, Sheridan Rawlins wrote:
> LGTM for rule refactor and gtestjs, with commenting nit.
> 
>
http://codereview.chromium.org/8599010/diff/22001/remoting/webapp/me2mom/debu...
> File remoting/webapp/me2mom/debug_log.gtestjs (right):
> 
>
http://codereview.chromium.org/8599010/diff/22001/remoting/webapp/me2mom/debu...
> remoting/webapp/me2mom/debug_log.gtestjs:21: TEST_F('DebugLogUnitTest',
> 'Sample', function() {
> nit: Can you pick a more descriptive test name than "Sample" and add a
comment? 
> This in particular will test that loading debug_log.js doesn't have any
console
> errors.

Done.

[garykac, 2011-11-29 20:41:28]

Brad, can you take a look?

It finally made it through the Windows trybots. I'm running it through the
max/linux bots again, but expect them to pass.

[bradn, 2011-11-30 03:51:11]

lgtm

[commit-bot: I haz the power, 2011-11-30 17:10:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/8599010/33001

[commit-bot: I haz the power, 2011-11-30 17:10:25]

Can't apply patch for file
content/browser/renderer_host/resource_dispatcher_host.cc.
While running patch -p1 --forward --force;
patching file content/browser/renderer_host/resource_dispatcher_host.cc
Hunk #1 FAILED at 389.
1 out of 1 hunk FAILED -- saving rejects to file
content/browser/renderer_host/resource_dispatcher_host.cc.rej

[commit-bot: I haz the power, 2011-11-30 17:22:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/8599010/35003

[commit-bot: I haz the power, 2011-11-30 18:24:52]

Change committed as 112252