fix test broken issue when using --use-emulator

fix test broken issue when using --use-emulator


BUG=136474
TEST=./build/android/run_tests.py -s out/Release/base_unittests_apk/base_unittests-debug.apk -e 1


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

[Wei James(wistoch), 2012-07-10 06:04:36]

The root cause should be that:

In the process of emulator booting, network interface of emulator may change,
which will result in the adb server lost the connection to the emulator.  as we
use adb logcat to monitor the test, so the test will fail.

[John Grabowski, 2012-07-10 06:14:09]

Overall like your intent but...

http://codereview.chromium.org/10692132/diff/1/build/android/pylib/single_tes...
File build/android/pylib/single_test_runner.py (right):

http://codereview.chromium.org/10692132/diff/1/build/android/pylib/single_tes...
build/android/pylib/single_test_runner.py:303: run_command.RunCommand('adb
kill-server')
prefer self.adb.SomethingOrOther() to a 'run command'.
I believe there is a KillServer() or RestartServer() function already.
If not, add one.

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

http://codereview.chromium.org/10692132/diff/1/build/android/run_tests.py#new...
build/android/run_tests.py:329: time.sleep(60)
Sadness.
Offhand I don't remember what ConfirmLaunch() does.  But a combination of 'adb
wait-for-device' and a WaitForPackageManager() should do what you need, yes?
Dislike sleeping.  Is flaky and wasteful.

[Wei James(wistoch), 2012-07-10 14:26:48]

thanks for the review. 

I have updated the patch to fix the issues from your comment. 

I have a question about the test: 
I met the test broken issue every time using --use-emulator, do you use
--use-emulator option in your test environment? do you have the same issue? 

thanks

http://codereview.chromium.org/10692132/diff/1/build/android/pylib/single_tes...
File build/android/pylib/single_test_runner.py (right):

http://codereview.chromium.org/10692132/diff/1/build/android/pylib/single_tes...
build/android/pylib/single_test_runner.py:303: run_command.RunCommand('adb
kill-server')
On 2012/07/10 06:14:09, John Grabowski wrote:
> prefer self.adb.SomethingOrOther() to a 'run command'.
> I believe there is a KillServer() or RestartServer() function already.
> If not, add one.
fixed. 

define Start/Kill/RestartAdbServer in adb_interface.py and use
RestartAdbServer() here. thanks

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

http://codereview.chromium.org/10692132/diff/1/build/android/run_tests.py#new...
build/android/run_tests.py:329: time.sleep(60)
On 2012/07/10 06:14:09, John Grabowski wrote:
> Sadness.
> Offhand I don't remember what ConfirmLaunch() does.  But a combination of 'adb
> wait-for-device' and a WaitForPackageManager() should do what you need, yes?
> Dislike sleeping.  Is flaky and wasteful.

ConfirmLaunch() is to make sure the emulator launched before the test starts. 

when the emulator starts, there are several steps. 
1. device offline
2. device online
3. flag dev.bootcomplete set to 1
4. flag sys.boot_completed set to 1

Previously, emulator launch will wait for step 2(wait-for-device). And if
parameter True passed, it will wait for step 3. 

The issue this patch want to fix is that the connection service manager will
reset all connections just before step 4, which will result in that the adb
server lost the connection to the emulator and so the test is broken. There will
be two possible issues:
1. error: device not found even emulator is there. 
2. adb logcat exits so pexpect return with EOF and no real test executed.  

I tested with WaitForDevicePm() but the connection still lost when the PM is
available. 

sleep(60) after the step 3 can make sure the device boot completely and will not
break the test. 

agree with you that adding sleep(60) is ugly. 
after some investigation, I added a function WaitForSystemBootCompleted in
adb_interface and use this to skip the connection lost issue. I don't  know
whether waiting for step 4 is enough, although I have tested on my machine 
for several times and it works.

[John Grabowski, 2012-07-10 22:25:49]

To answer your question, no, we don't use the emulator at this time.  I did not
find the ARM emulator to be up to the Chrome standard of reliability.  In
addition, since it doesn't (didn't?) support GPU mode, it didn't seem complete
enough.

LGTM if you adapt to SendCommand.

http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
File third_party/android_testrunner/adb_interface.py (right):

http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
third_party/android_testrunner/adb_interface.py:62: return
run_command.RunCommand(adb_cmd)
why RunCommand() instead of SendCommand()?

[Wei James(wistoch), 2012-07-11 01:09:24]

thanks for the clarification. 

I found several issues of --use-emulator. Currently we are using emulator for
unit tests and it works well. 

but previously we launched the emulator manually and then run_tests.py without
--use-emulator option(it then will test on the attached emulators). 

will --use-emulator be used later? or it is useless and will be removed later?

http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
File third_party/android_testrunner/adb_interface.py (right):

http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
third_party/android_testrunner/adb_interface.py:62: return
run_command.RunCommand(adb_cmd)
On 2012/07/10 22:25:49, John Grabowski wrote:
> why RunCommand() instead of SendCommand()?

as SendCommand will use a target arg, but at that time, the connection to the
target devices already lost. So using SendCommand('adb kill-server') will not
kill the adb server. 

So I use the run_command.RunCommand directly.

[John Grabowski, 2012-07-11 01:30:28]

On 2012/07/11 01:09:24, James Wei wrote:
> thanks for the clarification. 
> 
> I found several issues of --use-emulator. Currently we are using emulator for
> unit tests and it works well. 
> 
> but previously we launched the emulator manually and then run_tests.py without
> --use-emulator option(it then will test on the attached emulators). 
> 
> will --use-emulator be used later? or it is useless and will be removed later?

Personally I'd like to keep open the option of using emulators, and am pleased
you will help keep it working.

In addition, if your team uses it and needs it, that is important to us.

> 
>
http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
> File third_party/android_testrunner/adb_interface.py (right):
> 
>
http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
> third_party/android_testrunner/adb_interface.py:62: return
> run_command.RunCommand(adb_cmd)
> On 2012/07/10 22:25:49, John Grabowski wrote:
> > why RunCommand() instead of SendCommand()?
> 
> as SendCommand will use a target arg, but at that time, the connection to the
> target devices already lost. So using SendCommand('adb kill-server') will not
> kill the adb server. 
> 
> So I use the run_command.RunCommand directly.

OK; LGTM then

[Wei James(wistoch), 2012-07-11 01:33:44]

On 2012/07/11 01:30:28, John Grabowski wrote:
> On 2012/07/11 01:09:24, James Wei wrote:
> > thanks for the clarification. 
> > 
> > I found several issues of --use-emulator. Currently we are using emulator
for
> > unit tests and it works well. 
> > 
> > but previously we launched the emulator manually and then run_tests.py
without
> > --use-emulator option(it then will test on the attached emulators). 
> > 
> > will --use-emulator be used later? or it is useless and will be removed
later?
> 
> Personally I'd like to keep open the option of using emulators, and am pleased
> you will help keep it working.
> 
> In addition, if your team uses it and needs it, that is important to us.
> 
> > 
> >
>
http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
> > File third_party/android_testrunner/adb_interface.py (right):
> > 
> >
>
http://codereview.chromium.org/10692132/diff/5001/third_party/android_testrun...
> > third_party/android_testrunner/adb_interface.py:62: return
> > run_command.RunCommand(adb_cmd)
> > On 2012/07/10 22:25:49, John Grabowski wrote:
> > > why RunCommand() instead of SendCommand()?
> > 
> > as SendCommand will use a target arg, but at that time, the connection to
the
> > target devices already lost. So using SendCommand('adb kill-server') will
not
> > kill the adb server. 
> > 
> > So I use the run_command.RunCommand directly.
> 
> OK; LGTM then

thanks.

[commit-bot: I haz the power, 2012-07-11 01:33:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/james.wei@intel.com/10692132/5001

[commit-bot: I haz the power, 2012-07-11 01:34:06]

Failed to apply patch for build/android/pylib/single_test_runner.py:
While running patch -p1 --forward --force;
patching file build/android/pylib/single_test_runner.py
Hunk #1 FAILED at 292.
1 out of 1 hunk FAILED -- saving rejects to file
build/android/pylib/single_test_runner.py.rej

[commit-bot: I haz the power, 2012-07-11 01:56:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/james.wei@intel.com/10692132/8002

[commit-bot: I haz the power, 2012-07-11 03:30:05]

Change committed as 146045