Cleanup test server instance after running each a test in gtest test runner

Android: Cleanup test server instance after running each a test in gtest test runner

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

[craigdh, 2013-03-08 00:09:03]

On 2013/03/08 00:04:24, nileshagrawal1 wrote:

lgtm.

[frankf, 2013-03-08 01:44:28]

lgtm

[commit-bot: I haz the power, 2013-03-08 17:36:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/12659003/1

[nilesh, 2013-03-08 18:52:57]

On 2013/03/08 17:36:43, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/nileshagrawal%40chromium.org/12659003/1

I am holding this CL as it increases cycle time for content_browsertests by a
lot. Me and pliard are trying to find out if just killing the httpserver will
solve the issue.

[nilesh, 2013-03-08 19:41:45]

I have another patch which just cleans up the test server instance (if it was
not killed). PTAL.

[craigdh, 2013-03-08 19:50:42]

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/base/...
File build/android/pylib/base/base_test_runner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/base/...
build/android/pylib/base/base_test_runner.py:169: def
CleanupAfterEachTest(self):
This name is a bit misleading as it's only used by gtests. How about naming it
something like CleanupTestServer() and adding a docstring explaining when it
should be called.

[craigdh, 2013-03-08 19:53:08]

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/gtest...
File build/android/pylib/gtest/test_runner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/gtest...
build/android/pylib/gtest/test_runner.py:274: self.CleanupAfterEachTest()
Should this be in a finally block attached to the above try block?

[frankf, 2013-03-08 19:59:52]

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
File build/android/pylib/chrome_test_server_spawner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
build/android/pylib/chrome_test_server_spawner.py:409: def
CleanupTestServer(self):
Could you also add dosctrings for all the methods in this class? There has been
confusion in the past about who's responsible for starting/stopping/cleaning
servers.

[Philippe, 2013-03-08 22:44:07]

On 2013/03/08 19:59:52, frankf wrote:
>
https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
> File build/android/pylib/chrome_test_server_spawner.py (right):
> 
>
https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
> build/android/pylib/chrome_test_server_spawner.py:409: def
> CleanupTestServer(self):
> Could you also add dosctrings for all the methods in this class? There has
been
> confusion in the past about who's responsible for starting/stopping/cleaning
> servers.

lgtm

[nilesh, 2013-03-08 23:37:52]

PTAL

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/base/...
File build/android/pylib/base/base_test_runner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/base/...
build/android/pylib/base/base_test_runner.py:169: def
CleanupAfterEachTest(self):
Since test server is something internal to spawning server, I am renaming this
to cleanupSpawningServerState.

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
File build/android/pylib/chrome_test_server_spawner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/chrom...
build/android/pylib/chrome_test_server_spawner.py:409: def
CleanupTestServer(self):
On 2013/03/08 19:59:52, frankf wrote:
> Could you also add dosctrings for all the methods in this class? There has
been
> confusion in the past about who's responsible for starting/stopping/cleaning
> servers.

Renamed and added doc.

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/gtest...
File build/android/pylib/gtest/test_runner.py (right):

https://codereview.chromium.org/12659003/diff/24001/build/android/pylib/gtest...
build/android/pylib/gtest/test_runner.py:274: self.CleanupAfterEachTest()
On 2013/03/08 19:53:08, craigdh wrote:
> Should this be in a finally block attached to the above try block?

Done.

[frankf, 2013-03-08 23:39:17]

lgtm. Thanks nilesh.

[craigdh, 2013-03-08 23:40:03]

lgtm. Thanks!

[commit-bot: I haz the power, 2013-03-08 23:58:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/12659003/39001

[commit-bot: I haz the power, 2013-03-09 14:56:58]

Change committed as 187158