Split out policy code from net/tools/testserver.

Split out policy code from net/tools/testserver.

This moves the policy-specific code to chrome/browser/policy/cloud/test
and adds LocalPolicyTestServer which simplifies policy test server use
in unit tests.

BUG=chromium:119403
TEST=unit tests
TBR=ben@chromium.org


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183159

[Mattias Nissler (ping if slow), 2013-02-11 19:17:09]

Here's the test server CL. Just FYI at this point, I need to rebase so this
applies before my proto move CL.

[Mattias Nissler (ping if slow), 2013-02-12 16:31:50]

Ready for review!

[Mattias Nissler (ping if slow), 2013-02-12 18:04:04]

Adding Paweł for reviewing the test infrastructure bits.

[Joao da Silva, 2013-02-12 19:06:05]

lgtm with nits

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
File chrome/browser/policy/cloud_policy_browsertest.cc (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:5: #include
"base/base_paths.h"
remove

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:10: #include
"base/path_service.h"
remove

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:126: std::string url =
test_server_->GetURL("device_management").spec();
GetServiceURL()?

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
File chrome/browser/policy/test/local_policy_test_server.cc (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/local_policy_test_server.cc:7: #include
"base/command_line.h"
not used

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/local_policy_test_server.cc:32: .AppendASCII("data")
FWIW there's also chrome::DIR_TEST_DATA

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/local_policy_test_server.cc:94: }  // namespace
policy;
remove ;

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
File chrome/browser/policy/test/policy_testserver.py (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:113:
BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, request,
Does this work?

super(PolicyRequestHandler, self).__init__(request, ...)

(I think it requires new-style classes, but I'm not sure if
BaseHTTPRequestHandler is one)

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:114: client_address, server);
remove ;

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:133: def do_POST(self):
OVERRIDE

... oh wait.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:137:
self.send_header('Content-Type', 'application/x-protobuffer')
device_management_service.cc uses "application/protobuf" for its requests but
expects "application/x-protobuffer" from the server, weird.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:561:
testserver_base.StoppableHTTPServer):
indent

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:568: server_address: Serer host
and port.
*Server

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:572:
testserver_base.StoppableHTTPServer.__init__(self, server_address,
Same re super()

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:693:
testserver_base.TestServerRunner.__init__(self)
Same re super()

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:697: host = self.options.host
These 2 aren't used

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:715: 'through them in at each
request a '
through them at each request _in_ a ...

[Mattias Nissler (ping if slow), 2013-02-13 11:54:10]

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
File chrome/browser/policy/cloud_policy_browsertest.cc (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:5: #include
"base/base_paths.h"
On 2013/02/12 19:06:05, Joao da Silva wrote:
> remove

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:10: #include
"base/path_service.h"
On 2013/02/12 19:06:05, Joao da Silva wrote:
> remove

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/clo...
chrome/browser/policy/cloud_policy_browsertest.cc:126: std::string url =
test_server_->GetURL("device_management").spec();
On 2013/02/12 19:06:05, Joao da Silva wrote:
> GetServiceURL()?

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
File chrome/browser/policy/test/local_policy_test_server.cc (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/local_policy_test_server.cc:7: #include
"base/command_line.h"
On 2013/02/12 19:06:05, Joao da Silva wrote:
> not used

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/local_policy_test_server.cc:32: .AppendASCII("data")
On 2013/02/12 19:06:05, Joao da Silva wrote:
> FWIW there's also chrome::DIR_TEST_DATA

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
File chrome/browser/policy/test/policy_testserver.py (right):

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:113:
BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, request,
On 2013/02/12 19:06:05, Joao da Silva wrote:
> Does this work?
> 
> super(PolicyRequestHandler, self).__init__(request, ...)
> 
> (I think it requires new-style classes, but I'm not sure if
> BaseHTTPRequestHandler is one)

BaseRequestHandler (which is the root class here) is not a new-style class.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:114: client_address, server);
On 2013/02/12 19:06:05, Joao da Silva wrote:
> remove ;

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:133: def do_POST(self):
On 2013/02/12 19:06:05, Joao da Silva wrote:
> OVERRIDE
> 
> ... oh wait.

NOT Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:137:
self.send_header('Content-Type', 'application/x-protobuffer')
On 2013/02/12 19:06:05, Joao da Silva wrote:
> device_management_service.cc uses "application/protobuf" for its requests but
> expects "application/x-protobuffer" from the server, weird.

I think that's because DMServer sends the latter and doesn't care about the
former. Anyhow, this code is just copied net/tools/testserver/testserver.py

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:561:
testserver_base.StoppableHTTPServer):
On 2013/02/12 19:06:05, Joao da Silva wrote:
> indent

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:568: server_address: Serer host
and port.
On 2013/02/12 19:06:05, Joao da Silva wrote:
> *Server

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:572:
testserver_base.StoppableHTTPServer.__init__(self, server_address,
On 2013/02/12 19:06:05, Joao da Silva wrote:
> Same re super()

Doesn't work here due to multiple inheritance.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:693:
testserver_base.TestServerRunner.__init__(self)
On 2013/02/12 19:06:05, Joao da Silva wrote:
> Same re super()

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:697: host = self.options.host
On 2013/02/12 19:06:05, Joao da Silva wrote:
> These 2 aren't used

Done.

https://codereview.chromium.org/12235003/diff/11001/chrome/browser/policy/tes...
chrome/browser/policy/test/policy_testserver.py:715: 'through them in at each
request a '
On 2013/02/12 19:06:05, Joao da Silva wrote:
> through them at each request _in_ a ...

Done.

[Mattias Nissler (ping if slow), 2013-02-13 11:55:24]

Paweł: Friendly ping.

[Paweł Hajdan Jr., 2013-02-13 13:32:49]

LGTM, thanks!

[commit-bot: I haz the power, 2013-02-18 14:30:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/12235003/25008

[commit-bot: I haz the power, 2013-02-18 14:30:12]

Presubmit check for 12235003-25008 failed and returned exit status 1.

INFO:root:Found 14 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/policy/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/net/PRESUBMIT.py

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  chrome/browser/policy/test/local_policy_test_server.h, line 36, 81 chars

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 3.7s to calculate.

[commit-bot: I haz the power, 2013-02-18 14:35:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/12235003/34002

[commit-bot: I haz the power, 2013-02-18 14:35:56]

Presubmit check for 12235003-34002 failed and returned exit status 1.

INFO:root:Found 14 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/policy/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/net/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 1.7s to calculate.

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

[anagy4698, 2013-02-18 14:50:42]

Ezt a beszélgetést a program automatikusan lefordította: angol... check
useful? Please send feedback & hate mail to!"""",,,


2013/2/18 <commit-bot@chromium.org>

> Presubmit check for 12235003-34002 failed and returned exit status 1.
>
>
> INFO:root:Found 14 file(s).
>
> Running presubmit commit checks ...
> Running /b/commit-queue/workdir/**chromium/PRESUBMIT.py
> Running /b/commit-queue/workdir/**chromium/chrome/PRESUBMIT.py
> Running /b/commit-queue/workdir/**chromium/chrome/browser/**
> policy/PRESUBMIT.py
> Running /b/commit-queue/workdir/**chromium/net/PRESUBMIT.py
>
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     chrome
>
> Presubmit checks took 1.7s to calculate.
>
> Was the presubmit check useful? Please send feedback & hate mail to
> maruel@chromium.org!
>
>
>
https://chromiumcodereview.**appspot.com/12235003/<https://chromiumcodereview...
>



-- 
anagy4698@gmail.com.

[Mattias Nissler (ping if slow), 2013-02-18 14:53:59]

And finally Ben to TBR for the gyp change.

[commit-bot: I haz the power, 2013-02-18 14:54:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/12235003/34002

[commit-bot: I haz the power, 2013-02-19 05:13:52]

Change committed as 183159

[Ben Goodger (Google), 2013-02-19 17:50:17]

lgtm