Merge lp:~liuyq0307/lava-dispatcher/timeout_wait_home_screen into lp:lava-dispatcher

[Yongqin Liu]

delete the timeout in response in wait_home_screen method

[Yongqin Liu (liuyq0307)]

not tested yet. please just review from the source point.

And is there somebody can me have permission to modify the file below:
 /srv/lava/instances/staging/lib/python2.7/site-packages/lava_dispatcher/client/base.py

[Yongqin Liu (liuyq0307)]

reverted the panda.conf. seems no need to modify this file.

[lp:~liuyq0307/lava-dispatcher/timeout_wait_home_screen]

299. By Yongqin Liu on 2012-05-31

revert panda.conf

[Zygmunt Krynicki (zyga)]

8 - self.run(cmd, response=['stopped', pexpect.TIMEOUT], timeout=5)
9 - if self.match_id == 0:
10 - return True
11 - time.sleep(1)
12 + try:
13 + self.run(cmd, response=['stopped'], timeout=5)
14 + if self.match_id == 0:
15 + return True
16 + except pexpect.TIMEOUT:
17 + time.sleep(1)

What is the logical change this part is supposed to make? I don't see any. You've traded the explicit timeout handling to an exception based timeout handling

review: Needs Information

[Yongqin Liu (liuyq0307)]

below is the talk with mwhudson about this.

<liuyq> mwhudson, in method wait_home_screen, it is now using "self.run(cmd, response=['stopped', pexpect.TIMEOUT], timeout=5)" to catch timeout
<liuyq> mwhudson, but in fact, the "TIMEOUT: Timeout exceeded in read_nonblocking()." is not caught
<liuyq> mwhudson, so I consider how about trying catch TIMEOUT sentence there
<liuyq> mwhudson, not catch the timeout in the response
<liuyq> mwhudson, how do you think about that?
<mwhudson> liuyq: oh heh
<mwhudson> liuyq: that sounds like a bug actually
<mwhudson> liuyq: if you pass pexpect.TIMEOUT in response the code still waits for the prompt
<mwhudson> and that probably won't appear
<mwhudson> so it will raise an exception
<mwhudson> liuyq: can you file a bug on lava-dispatcher about this?
<mwhudson> liuyq: in the mean time, just catch pexpect.TIMEOUT I guess
<liuyq> mwhudson, sorry, why do you say "and that probably won't appear", the prompt “root@linaro” should appear
<mwhudson> liuyq: but it only waits one more second for the root@linaro to appear
<mwhudson> liuyq: if the timeout is 5 seconds and nothing has appeared, the root@linaro _probably_ won't appear in that 1 more second

[Zygmunt Krynicki (zyga)]

Ok, please open a bug on pexpect about this and link it here.

[Yongqin Liu (liuyq0307)]

The test has been done as below:
1. 6 jobs are submitted to staging
2. each job contain deploy_linaro_android_image action and 20 boot_linaro_android_image actions.
And the result is that all the actions are passed.

review: Approve (test with staging)

[Yongqin Liu (liuyq0307)]

> Ok, please open a bug on pexpect about this and link it here.
About this, even it's a bug on pexpect, we still need to work around it to make our lava works well until it's fixed on pexpect.
So this branch is necessary for us at least now.

[Michael Hudson-Doyle (mwhudson)]

The bug in timeout handling is in the dispatcher, not pexpect. Can you file a bug in lava-dispatcher and link to it in a comment in the code?

Otherwise, this makes sense to me.

review: Approve

[Yongqin Liu (liuyq0307)]

> The bug in timeout handling is in the dispatcher, not pexpect. Can you file a
> bug in lava-dispatcher and link to it in a comment in the code?
This bug has already been linked to bug https://bugs.launchpad.net/lava-dispatcher/+bug/1001112
Is that bug not what you want?

[Michael Hudson-Doyle (mwhudson)]

Yongqin Liu <email address hidden> writes:

>> The bug in timeout handling is in the dispatcher, not pexpect. Can you file a
>> bug in lava-dispatcher and link to it in a comment in the code?
> This bug has already been linked to bug https://bugs.launchpad.net/lava-dispatcher/+bug/1001112
> Is that bug not what you want?

I meant to file a new bug, so I did that:
https://bugs.launchpad.net/lava-dispatcher/+bug/1009302

Cheers,
mwh

=== modified file 'lava_dispatcher/client/base.py'
--- lava_dispatcher/client/base.py	2012-05-23 21:49:59 +0000
+++ lava_dispatcher/client/base.py	2012-05-31 11:09:19 +0000
@@ -235,10 +235,12 @@
     def wait_home_screen(self):
         cmd = 'getprop init.svc.bootanim'
         for count in range(100):
-            self.run(cmd, response=['stopped', pexpect.TIMEOUT], timeout=5)
-            if self.match_id == 0:
-                return True
-            time.sleep(1)
+            try:
+                self.run(cmd, response=['stopped'], timeout=5)
+                if self.match_id == 0:
+                    return True
+            except pexpect.TIMEOUT:
+                time.sleep(1)
         raise GeneralError('The home screen has not displayed')
 
     def check_device_state(self):
@@ -439,12 +441,12 @@
         self.proc.expect(self.tester_str, timeout=120)
         #TODO: set up proxy
 
+        self._disable_suspend()
         if self.config.get("enable_network_after_boot_android"):
             time.sleep(1)
             self._enable_network()
 
         self._enable_adb_over_tcpip()
-        self._disable_suspend()
 
     def _disable_suspend(self):
         """ disable the suspend of images.