Python sync server impl, for test

[Third time landing] Python implementation of sync server, for testing.

Implement the server side of chromium sync inside of testserver.py.  The implementation supports at most one account (and ignores authentication credentials), but is otherwise reasonably full featured.

Make the sync_integration_tests run by default against the test server.  An externally-provided --sync-url will give the old behavior.

Protocol buffers stuff: The test sync server requires Python generated code for .proto files.  I've put generated code, as well as the python protocol buffers runtime library, in the output directory + "/python" (e.g, on windows, src/chrome/Debug/python/google/protobuf).

Flakiness fix: In the InProcessBrowserTest framework, improve the mechanism for tests that want to manually set up a user data directory.  The new way ensures that the user data directory is always wiped; tests can't accidentally forget to do this anymore.

Flakiness fix: Make testserver try to /kill any old instance that might be hogging the port.  Very useful if a test failure leaves a server running.  Tested this against all combos of protocols, and it seems to work.

Flakiness fix: Port sync_integration_tests to the out-of-process test runner.

Flakiness fix: For IN_PROC_BROWSER_TESTS, don't run the test body if the setup triggered a fatal (ASSERT_) failure.

BUG=20905, 40980
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=44708

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

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

[ncarter (slow), 2010-04-09 20:50:52]

Hi Zach,

Please review.

[Zachary Kuznia, 2010-04-09 23:07:12]

__init__.py seems to be empty.  Is this intentional?

http://codereview.chromium.org/1622012/diff/8001/9001
File chrome/browser/sync/engine/syncer_proto_util.cc (right):

http://codereview.chromium.org/1622012/diff/8001/9001#newcode1
chrome/browser/sync/engine/syncer_proto_util.cc:1: // Copyright (c) 2006-2008
The Chromium Authors. All rights reserved.
There doesn't seem to be any changes here.

http://codereview.chromium.org/1622012/diff/8001/9002
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/1622012/diff/8001/9002#newcode495
chrome/browser/sync/profile_sync_service.cc:495: preferred_types->clear();
nit: indent two spaces.

http://codereview.chromium.org/1622012/diff/8001/9007
File chrome/test/live_sync/live_sync_test.h (right):

http://codereview.chromium.org/1622012/diff/8001/9007#newcode63
chrome/test/live_sync/live_sync_test.h:63: server_.kHostName,
server_.kOKHTTPSPort,
Where does the host and port get set?

http://codereview.chromium.org/1622012/diff/8001/9013
File net/tools/testserver/chromiumsync.py (right):

http://codereview.chromium.org/1622012/diff/8001/9013#newcode1
net/tools/testserver/chromiumsync.py:1: #!/usr/bin/python2.4
/usr/bin/python

http://codereview.chromium.org/1622012/diff/8001/9013#newcode2
net/tools/testserver/chromiumsync.py:2: # Copyright (c) 2006-2008 The Chromium
Authors. All rights reserved.
2010

http://codereview.chromium.org/1622012/diff/8001/9013#newcode22
net/tools/testserver/chromiumsync.py:22: class SyncTypes(object):
It's a little weird for this class to be entirely static methods.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode132
net/tools/testserver/chromiumsync.py:132: # TODO(nick): uuid.uuid1() is better,
but python 2.5 only.
install-build-deps.sh installs python2.5-dev, so I think you can use this?

http://codereview.chromium.org/1622012/diff/8001/9013#newcode233
net/tools/testserver/chromiumsync.py:233: if spec[3] in requested_types:
Why is _PERMANENT_ITEM_SPECS a list instead of a dictionary?  spec[3] and
spec[1] are difficult to read.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode356
net/tools/testserver/chromiumsync.py:356: response.authenticate.user.email =
'ncarter@gmail.com'
I wouldn't use a real email address here.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode357
net/tools/testserver/chromiumsync.py:357:
response.authenticate.user.display_name = 'Nick Carter'
...or a real name.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode404
net/tools/testserver/chromiumsync.py:404: requested_types =
SyncTypes.GetRequestedTypes(
nit: This line shouldn't need to be split.

http://codereview.chromium.org/1622012/diff/8001/9014
File net/tools/testserver/chromiumsync_test.py (right):

http://codereview.chromium.org/1622012/diff/8001/9014#newcode1
net/tools/testserver/chromiumsync_test.py:1: #!/usr/bin/python2.4
/usr/bin/python

http://codereview.chromium.org/1622012/diff/8001/9014#newcode2
net/tools/testserver/chromiumsync_test.py:2: # Copyright (c) 2006-2008 The
Chromium Authors. All rights reserved.
2010

http://codereview.chromium.org/1622012/diff/8001/9014#newcode22
net/tools/testserver/chromiumsync_test.py:22: def testBatchSize(self):
Is this test complete?

http://codereview.chromium.org/1622012/diff/8001/9018
File third_party/protobuf2/protobuf.gyp (right):

http://codereview.chromium.org/1622012/diff/8001/9018#newcode258
third_party/protobuf2/protobuf.gyp:258: #
'src/python/google/protobuf/internal/descriptor_test.py',
Why are these commented out?

http://codereview.chromium.org/1622012/diff/8001/9018#newcode282
third_party/protobuf2/protobuf.gyp:282: #   'rules': [
Why is this commented out?

[ncarter (slow), 2010-04-13 20:12:30]

Patch set updated.  It is correct for __init__.py to be empty.  See
http://groups.google.com/group/protobuf/browse_thread/thread/401e6e54e6b4657b...
for a discussion.

http://codereview.chromium.org/1622012/diff/8001/9002
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/1622012/diff/8001/9002#newcode495
chrome/browser/sync/profile_sync_service.cc:495: preferred_types->clear();
On 2010/04/09 23:07:12, zork wrote:
> nit: indent two spaces.

Tragic.  Done.

http://codereview.chromium.org/1622012/diff/8001/9007
File chrome/test/live_sync/live_sync_test.h (right):

http://codereview.chromium.org/1622012/diff/8001/9007#newcode63
chrome/test/live_sync/live_sync_test.h:63: server_.kHostName,
server_.kOKHTTPSPort,
On 2010/04/09 23:07:12, zork wrote:
> Where does the host and port get set?

We load these values into the --sync-url command line flag, four lines down in
this same method.  That propagates the values to the sync code.

http://codereview.chromium.org/1622012/diff/8001/9013
File net/tools/testserver/chromiumsync.py (right):

http://codereview.chromium.org/1622012/diff/8001/9013#newcode1
net/tools/testserver/chromiumsync.py:1: #!/usr/bin/python2.4
On 2010/04/09 23:07:12, zork wrote:
> /usr/bin/python

The style guide says to use as specific a version as possible; and I did this to
be consistent with testserver.py.  If you're certain that this is wrong, let me
know and I'll fix it there too.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode2
net/tools/testserver/chromiumsync.py:2: # Copyright (c) 2006-2008 The Chromium
Authors. All rights reserved.
On 2010/04/09 23:07:12, zork wrote:
> 2010

Done.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode22
net/tools/testserver/chromiumsync.py:22: class SyncTypes(object):
On 2010/04/09 23:07:12, zork wrote:
> It's a little weird for this class to be entirely static methods.

Removed the class in favor of top-level defs.  I suppose the module namespace
gives us everything we need.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode132
net/tools/testserver/chromiumsync.py:132: # TODO(nick): uuid.uuid1() is better,
but python 2.5 only.
On 2010/04/09 23:07:12, zork wrote:
> install-build-deps.sh installs python2.5-dev, so I think you can use this?

Sadly, on Windows we seem to get a 2.4 python.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode233
net/tools/testserver/chromiumsync.py:233: if spec[3] in requested_types:
On 2010/04/09 23:07:12, zork wrote:
> Why is _PERMANENT_ITEM_SPECS a list instead of a dictionary?  spec[3] and
> spec[1] are difficult to read.

Good suggestion.  I moved PermanentItem to be a record instead of a tuple --
hope that's better.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode356
net/tools/testserver/chromiumsync.py:356: response.authenticate.user.email =
'ncarter@gmail.com'
On 2010/04/09 23:07:12, zork wrote:
> I wouldn't use a real email address here.

Done.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode357
net/tools/testserver/chromiumsync.py:357:
response.authenticate.user.display_name = 'Nick Carter'
On 2010/04/09 23:07:12, zork wrote:
> ...or a real name.

Done.

http://codereview.chromium.org/1622012/diff/8001/9013#newcode404
net/tools/testserver/chromiumsync.py:404: requested_types =
SyncTypes.GetRequestedTypes(
On 2010/04/09 23:07:12, zork wrote:
> nit: This line shouldn't need to be split.

Done.

http://codereview.chromium.org/1622012/diff/8001/9014
File net/tools/testserver/chromiumsync_test.py (right):

http://codereview.chromium.org/1622012/diff/8001/9014#newcode1
net/tools/testserver/chromiumsync_test.py:1: #!/usr/bin/python2.4
On 2010/04/09 23:07:12, zork wrote:
> /usr/bin/python

Not done per comment in previous file.

http://codereview.chromium.org/1622012/diff/8001/9014#newcode2
net/tools/testserver/chromiumsync_test.py:2: # Copyright (c) 2006-2008 The
Chromium Authors. All rights reserved.
On 2010/04/09 23:07:12, zork wrote:
> 2010

Done.

http://codereview.chromium.org/1622012/diff/8001/9014#newcode22
net/tools/testserver/chromiumsync_test.py:22: def testBatchSize(self):
On 2010/04/09 23:07:12, zork wrote:
> Is this test complete?

Nope.  Added more tests.

http://codereview.chromium.org/1622012/diff/8001/9018
File third_party/protobuf2/protobuf.gyp (right):

http://codereview.chromium.org/1622012/diff/8001/9018#newcode258
third_party/protobuf2/protobuf.gyp:258: #
'src/python/google/protobuf/internal/descriptor_test.py',
On 2010/04/09 23:07:12, zork wrote:
> Why are these commented out?

Removed them.

http://codereview.chromium.org/1622012/diff/8001/9018#newcode282
third_party/protobuf2/protobuf.gyp:282: #   'rules': [
On 2010/04/09 23:07:12, zork wrote:
> Why is this commented out?

The reason was given in the comment about the descriptor_pb2.py copy.  I added a
comment here to be clearer.  Unless you compile descriptor.proto from a pwd of
src/src, what happens is that you wind up with a descriptor_pb2.py that tries to
import itself, and this is illegal.

[ncarter (slow), 2010-04-14 00:43:18]

Adding bradnelson as a second reviewer, due to the gyp complexity that arose on
Mac.

Brad: I'm generating python code from .proto files; the python code gets
imported by another python script launched by C++ code in
net.gyp!net_test_support.  The .proto files live under chrome/browser/..., and
net.gyp can't depend on chrome.gyp due to the circular dependency rule.  So I've
moved the code generation into a new, isolated .gyp file:
chrome/browser/sync/protocol/sync_proto.gyp.

Please let me know how this approach sits with you.

[bradn, 2010-04-14 01:41:58]

So the gyp looks reasonable.
Adding a 3rd thing to break the cycle sounds good, but I am a little puzzled why
the circularity exists. It seems a little odd to have net's testserver depend on
a chrome component, granted a protobuf. Maybe the sync protocol should live up
top? But that also seems weird. I suppose the whole set of tests could live over
in chrome and something over there could make use of testserver's innards. But
that sounds like a pain.

It's probably fine. I'd rationalize it like this: sync is just another type of
request the test server can handle, it happens to be one where using a protocol
from chrome makes it easier to implement.

Guess we're really never going back to pulling in net/base via DEPS :-)

http://codereview.chromium.org/1622012/diff/37001/15020
File third_party/protobuf2/protobuf.gyp (right):

http://codereview.chromium.org/1622012/diff/37001/15020#newcode264
third_party/protobuf2/protobuf.gyp:264: # file, making it tough to invoke protoc
from src/src.  Until this
Seems like a reasonable feature to add (running from a particular cwd). Add an
issue to gyp.googlecode.com

http://codereview.chromium.org/1622012/diff/37001/15020#newcode274
third_party/protobuf2/protobuf.gyp:274: #   'rules': [
Does all this need to be here comment out?

[ncarter (slow), 2010-04-14 19:20:52]

FYI: There are apparently still some unresolved issues with the
testserver/unittest/gyp integration.  I'm working on it.

[Zachary Kuznia, 2010-04-14 22:27:35]

Python LGTM

http://codereview.chromium.org/1622012/diff/8001/9007
File chrome/test/live_sync/live_sync_test.h (right):

http://codereview.chromium.org/1622012/diff/8001/9007#newcode63
chrome/test/live_sync/live_sync_test.h:63: server_.kHostName,
server_.kOKHTTPSPort,
On 2010/04/13 20:12:30, nick wrote:
> On 2010/04/09 23:07:12, zork wrote:
> > Where does the host and port get set?
> 
> We load these values into the --sync-url command line flag, four lines down in
> this same method.  That propagates the values to the sync code.

Pardon, I misread this and missed that they were constants.

http://codereview.chromium.org/1622012/diff/8001/9013
File net/tools/testserver/chromiumsync.py (right):

http://codereview.chromium.org/1622012/diff/8001/9013#newcode1
net/tools/testserver/chromiumsync.py:1: #!/usr/bin/python2.4
On 2010/04/13 20:12:30, nick wrote:
> On 2010/04/09 23:07:12, zork wrote:
> > /usr/bin/python
> 
> The style guide says to use as specific a version as possible; and I did this
to
> be consistent with testserver.py.  If you're certain that this is wrong, let
me
> know and I'll fix it there too.

A grep for bin/python in src/chrome has every instance using /usr/bin/python,
but all instances in src/net seem to use python2.4.  Also, the style guide does
agree with you.  I retract my comment.

http://codereview.chromium.org/1622012/diff/29001/2014
File net/tools/testserver/chromiumsync_test.py (right):

http://codereview.chromium.org/1622012/diff/29001/2014#newcode230
net/tools/testserver/chromiumsync_test.py:230: proto1, result1 =
DoCommit(name='namae', id='Foo',
いい名前だね

[Timur Iskhodzhanov, 2010-04-30 07:31:12]

Hi Nick, looks like your CL has made half of the Memory waterfall red.

[Timur Iskhodzhanov, 2010-04-30 10:02:28]

The CL was reverted ( http://codereview.chromium.org/1822001 ).
You can find the post-mortem here http://codereview.chromium.org/1736026 (the
issue seems to be fixed)
I'll try to revert the revert soon.

[Timur Iskhodzhanov, 2010-04-30 15:15:30]

Re-landed as http://codereview.chromium.org/1763023
On 2010/04/30 10:02:28, Timur Iskhodzhanov wrote:
> The CL was reverted ( http://codereview.chromium.org/1822001 ).
> You can find the post-mortem here http://codereview.chromium.org/1736026 (the
> issue seems to be fixed)
> I'll try to revert the revert soon.