Upstream: Test scripts for Android (phase 1)

Upstream: Test scripts for Android (phase 1)

These are the utilities

BUG=
TEST=

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

[michaelbai, 2011-10-19 17:38:22]

Hi Nirnimesh

Please help to review this

thanks

[Nirnimesh, 2011-10-19 22:01:47]

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands.py
File build/android/android_commands.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:27: sys.path.append(os.path.join(sys.path[0],
'..', '..', 'third_party',
depending on sys.path[0] might be risky. Construct relative path wrt __file__

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:32: from run_tests_helper import *
the style guide forbids this. Use one of the following:

import x

from x import y, z

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:38: PEXPECT_LINE_RE =
re.compile('\n([^\r]*)\r')
Looks a little cluttered. Leave an empty line after each?

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:62: """Returns a list of emulators. Does not
filter by status (e.g. offline).
Leave 2 spaces after end of sentence.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:108: return (os.path.getsize(file_name),
utc_time - time_delta)
remove outer parens

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:189: # TODO(tonyg): Remove this after
debugging.
Remove now?

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:241: To workaround http://b/5201039, we
restart the shell and retry if the
Is there a public counterpart to this bug?

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:347: int number of processess killed
remove 'int'

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:430: # TODO(tonyg): Remove this after
debugging.
Remove?

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:607: except pexpect.TIMEOUT as e:
e is unused

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py
File build/android/debug_info.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:88: The prefix name is consist of the following
factors:
is consist -> consists
remove 'factors'

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:125: if self.logcat_process.poll() == None:
s/==/is/

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:179: """A helper method to zip all bebug information
results into a dump file.
s/bebug/debug/

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py
File build/android/emulator.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newcode6
build/android/emulator.py:6: """Provides an interface start and stop Android
emulator.
start -> to start

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:22: def _get_available_port():
_GetAvailablePort

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:56: raise Exception
Why not just raise the current exception?

raise

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:61: def launch(self):
Style guide requires all non-trivial method names to begin with a capital letter

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:74: def shutdown(self):
Shutdown

[michaelbai, 2011-10-19 23:17:15]

All comments were addressed expect one, please check my reply.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands.py
File build/android/android_commands.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:27: sys.path.append(os.path.join(sys.path[0],
'..', '..', 'third_party',
On 2011/10/19 22:01:47, Nirnimesh wrote:
> depending on sys.path[0] might be risky. Construct relative path wrt __file__

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:32: from run_tests_helper import *
On 2011/10/19 22:01:47, Nirnimesh wrote:
> the style guide forbids this. Use one of the following:
> 
> import x
> 
> from x import y, z

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:38: PEXPECT_LINE_RE =
re.compile('\n([^\r]*)\r')
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Looks a little cluttered. Leave an empty line after each?

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:62: """Returns a list of emulators. Does not
filter by status (e.g. offline).
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Leave 2 spaces after end of sentence.

Sorry, I am not sure I understand, could you give me a example?

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:108: return (os.path.getsize(file_name),
utc_time - time_delta)
On 2011/10/19 22:01:47, Nirnimesh wrote:
> remove outer parens

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:189: # TODO(tonyg): Remove this after
debugging.
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Remove now?

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:241: To workaround http://b/5201039, we
restart the shell and retry if the
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Is there a public counterpart to this bug?

Sorry, we don't have

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:347: int number of processess killed
On 2011/10/19 22:01:47, Nirnimesh wrote:
> remove 'int'

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:430: # TODO(tonyg): Remove this after
debugging.
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Remove?

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:607: except pexpect.TIMEOUT as e:
On 2011/10/19 22:01:47, Nirnimesh wrote:
> e is unused

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py
File build/android/debug_info.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:88: The prefix name is consist of the following
factors:
On 2011/10/19 22:01:47, Nirnimesh wrote:
> is consist -> consists
> remove 'factors'

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:125: if self.logcat_process.poll() == None:
On 2011/10/19 22:01:47, Nirnimesh wrote:
> s/==/is/

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/debug_info.py#new...
build/android/debug_info.py:179: """A helper method to zip all bebug information
results into a dump file.
On 2011/10/19 22:01:47, Nirnimesh wrote:
> s/bebug/debug/

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py
File build/android/emulator.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newcode6
build/android/emulator.py:6: """Provides an interface start and stop Android
emulator.
On 2011/10/19 22:01:47, Nirnimesh wrote:
> start -> to start

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:22: def _get_available_port():
On 2011/10/19 22:01:47, Nirnimesh wrote:
> _GetAvailablePort

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:56: raise Exception
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Why not just raise the current exception?
> 
> raise

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:61: def launch(self):
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Style guide requires all non-trivial method names to begin with a capital
letter

Done.

http://codereview.chromium.org/8356013/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:74: def shutdown(self):
On 2011/10/19 22:01:47, Nirnimesh wrote:
> Shutdown

Done.

[Nirnimesh, 2011-10-20 02:06:50]

LGTM

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands.py
File build/android/android_commands.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:62: """Returns a list of emulators. Does not
filter by status (e.g. offline).
On 2011/10/19 23:17:15, michaelbai wrote:
> On 2011/10/19 22:01:47, Nirnimesh wrote:
> > Leave 2 spaces after end of sentence.
> 
> Sorry, I am not sure I understand, could you give me a example?

I meant something like:

First sentence.  Second sentence.  Third sentence.

ie. 2 spaces (not 1) after the period.

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py
File build/android/debug_info.py (right):

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py#...
build/android/debug_info.py:201: assert(os.path.exists(log_file))
remove outer parens

[Jing Zhao, 2011-10-20 06:10:23]

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py
File build/android/debug_info.py (right):

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py#...
build/android/debug_info.py:88: The prefix name is consist of the following:
is consist of -> consists

[michaelbai, 2011-10-20 16:23:02]

Addressed all comments and submitted

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands.py
File build/android/android_commands.py (right):

http://codereview.chromium.org/8356013/diff/1/build/android/android_commands....
build/android/android_commands.py:62: """Returns a list of emulators. Does not
filter by status (e.g. offline).
On 2011/10/20 02:06:50, Nirnimesh wrote:
> On 2011/10/19 23:17:15, michaelbai wrote:
> > On 2011/10/19 22:01:47, Nirnimesh wrote:
> > > Leave 2 spaces after end of sentence.
> > 
> > Sorry, I am not sure I understand, could you give me a example?
> 
> I meant something like:
> 
> First sentence.  Second sentence.  Third sentence.
> 
> ie. 2 spaces (not 1) after the period.

Done.

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py
File build/android/debug_info.py (right):

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py#...
build/android/debug_info.py:88: The prefix name is consist of the following:
On 2011/10/20 06:10:23, Jing Zhao wrote:
> is consist of -> consists

Done.

http://codereview.chromium.org/8356013/diff/5002/build/android/debug_info.py#...
build/android/debug_info.py:201: assert(os.path.exists(log_file))
On 2011/10/20 02:06:50, Nirnimesh wrote:
> remove outer parens

Done.