Allow testserver to run without chromiumsync.

Allow testserver to run without chromiumsync.

BUG=43624
TEST=Able to run 'PYTHONPATH=third_party/tlslite:third_party/pyftpdlib python net/tools/testserver/testserver.py --data-dir=chrome/test/data/'

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

[ncarter (slow), 2010-07-26 23:18:20]

The approach looks fine (wish I'd thought of it).

You can mention bug 43624.

http://codereview.chromium.org/3014036/diff/1/2
File net/tools/testserver/testserver.py (right):

http://codereview.chromium.org/3014036/diff/1/2#newcode1022
net/tools/testserver/testserver.py:1022: if not self._sync_handler:
Should this be mutexed?

[ncarter (slow), 2010-07-26 23:21:03]

BTW, how did this come up in practice?  Are tests launching the test server
without using the TestServerLauncher from ssl_test_util?  Or are you launching
the script manually?

[Patrick Horn, 2010-07-26 23:25:56]

I was actually working on an expermintal changelist for pyauto to be able to
launch the HTTP server. Because to the way the automation hooks work, python
threads would hang, so I was trying to get the testserver to run in a different
python instance with the correct sys.path.

http://codereview.chromium.org/3014036/diff/1/2
File net/tools/testserver/testserver.py (right):

http://codereview.chromium.org/3014036/diff/1/2#newcode1022
net/tools/testserver/testserver.py:1022: if not self._sync_handler:
On 2010/07/26 23:18:20, ncarter wrote:
> Should this be mutexed?

I think the forking HTTP server was recently removed since it wasn't being used,
and I thought BaseHTTPServer was not threaded.

[ncarter (slow), 2010-07-26 23:35:00]

LGTM.

Didn't know BaseHTTPServer was necessarily single-threaded.

[Patrick Horn, 2010-07-26 23:51:19]

I think usually you add in ThreadingMixIn to get a multithreaded server, as in
here:
http://docs.python.org/release/2.6/library/socketserver.html

But based on looking over the code, I think there might be a few other
thread-safety problems in this case.

[Patrick Horn, 2010-07-27 00:17:13]

I just moved the _sync_handler into the server rather than keeping it a class
varaible. If we want to make it threadable, the thread safety problems should be
fixable by searching for "self.server" and adding locks around those.

Could you check it over one more time?

Tested by running:
PYTHONPATH=third_party/tlslite:third_party/pyftpdlib:out/Debug/pyproto/sync_pb:net/tools/testserver:out/Debug/pyproto
python net/tools/testserver/testserver.py --data-dir=chrome/test/data/

Not sure how to run tests, but with the change I manually sent a POST request to
/chromiumsync/command and it gave back some numbers.

[ncarter (slow), 2010-07-27 00:22:53]

On Mon, Jul 26, 2010 at 5:17 PM, <pathorn@chromium.org> wrote:

> I just moved the _sync_handler into the server rather than keeping it a
> class
> varaible. If we want to make it threadable, the thread safety problems
> should be
> fixable by searching for "self.server" and adding locks around those.
>
> Could you check it over one more time?
>
> Tested by running:
>
>
PYTHONPATH=third_party/tlslite:third_party/pyftpdlib:out/Debug/pyproto/sync_pb:net/tools/testserver:out/Debug/pyproto
> python net/tools/testserver/testserver.py --data-dir=chrome/test/data/
>
> Not sure how to run tests, but with the change I manually sent a POST
> request to
> /chromiumsync/command and it gave back some numbers.
>
> http://codereview.chromium.org/3014036/show
>

Getting back numbers from the time path is probably proof enough, but the
tests are not hard to run.  Compile the sync_integration_tests target, and
run with the command line flags --sync-user-for-test= and
--sync-password-for-test= with a real Google Account (the test does real
authentication, but syncs to a fake store).  They are also on the FYI
waterfall.

 - nick

[ncarter (slow), 2010-07-27 00:24:52]

Latest patch set LGTM

[Timur Iskhodzhanov, 2010-07-27 07:44:00]

Probably, you've meant tim@, not timurrrr@ ;)

[Patrick Horn, 2010-07-27 10:58:35]

On 2010/07/27 07:44:00, Timur Iskhodzhanov wrote:
> Probably, you've meant tim@, not timurrrr@ ;)

Sorry, I mistook you as the author because your revert commit was in the "svn
blame". ncarter (the actual author of cl 1622012) has reviewed it, so I removed
your name from reviewers.