[Android] Extract test server spawner logic from BaseTestRunner.

[Android] Extract test server spawner logic from BaseTestRunner.

BUG=428729
Committed: https://crrev.com/d610e4c872c64e6a35681ee0c237ccfc0ba4930f
Cr-Commit-Position: refs/heads/master@{#303784}

[jbudorick, 2014-11-11 19:09:30]

jbudorick@chromium.org changed reviewers:
+ klundberg@chromium.org, mikecase@chromium.org, rnephew@google.com

[klundberg, 2014-11-11 19:24:57]

One question and one nit so giving LGTM

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/base...
File build/android/pylib/base/test_server.py (right):

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/base...
build/android/pylib/base/test_server.py:7: """
nit: does this fit on the previous line?

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/gtes...
File build/android/pylib/gtest/test_runner.py (right):

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/gtes...
build/android/pylib/gtest/test_runner.py:61: ports.AllocateTestServerPort(),
self.device, self.tool)]
I'm wondering if you should have a TODO here to support multiple modes?
At the moment you are hardcoding "local" for the test server spawner. I'm
assuming that should change?

[jbudorick, 2014-11-11 23:19:22]

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/base...
File build/android/pylib/base/test_server.py (right):

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/base...
build/android/pylib/base/test_server.py:7: """
On 2014/11/11 19:24:57, klundberg wrote:
> nit: does this fit on the previous line?

Yes. Fixed.

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/gtes...
File build/android/pylib/gtest/test_runner.py (right):

https://codereview.chromium.org/713323002/diff/40001/build/android/pylib/gtes...
build/android/pylib/gtest/test_runner.py:61: ports.AllocateTestServerPort(),
self.device, self.tool)]
On 2014/11/11 19:24:57, klundberg wrote:
> I'm wondering if you should have a TODO here to support multiple modes?
> At the moment you are hardcoding "local" for the test server spawner. I'm
> assuming that should change?

No, having the new module be specific to local was intentional. I anticipate the
logic for supporting a test server spawner for nonlocal tests being sufficiently
different from the local test server spawner logic to necessitate a separate
class that derives from TestServer.

[klundberg, 2014-11-12 02:19:08]

Still lgtm and thanks for the answer.

[jbudorick, 2014-11-12 02:20:27]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-11-12 02:21:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/713323002/60001

[commit-bot: I haz the power, 2014-11-12 03:17:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-11-12 03:17:39]

Patchset 4 (id:??) landed as
https://crrev.com/d610e4c872c64e6a35681ee0c237ccfc0ba4930f
Cr-Commit-Position: refs/heads/master@{#303784}