Merge lp:~doanac/phablet-tools/unlock_screen into lp:phablet-tools

Proposed by Andy Doan on 2013-10-15
Status: Rejected
Rejected by: Sergio Schvezov on 2013-10-29
Proposed branch: lp:~doanac/phablet-tools/unlock_screen
Merge into: lp:phablet-tools
Diff against target: 189 lines (+118/-5)
3 files modified
phablet-test-run (+23/-5)
setup.py (+3/-0)
target/unlock_screen.py (+92/-0)
To merge this branch: bzr merge lp:~doanac/phablet-tools/unlock_screen
Reviewer Review Type Date Requested Status
Michał Sawicz 2013-10-15 Disapprove on 2013-10-15
PS Jenkins bot continuous-integration Approve on 2013-10-15
Sergio Schvezov 2013-10-15 Needs Fixing on 2013-10-15
Review via email: mp+191268@code.launchpad.net

Commit message

add option to phablet-test-run for unlocking the screen

This allows a user to pass "-u" to phablet-test-run. The
unlock_screen.py utility will be copied to the target and
then run right before executing the autopilot testsuite.

Description of the change

We've disputed where to put this in the past, but as we near the end of the release, we need a way to run tests with the screen unlocked. I've copied the unlock_screen.py script that the CI team inherited into the branch. Its mostly un-modified, but I did fix some pep8 complaints.

This adds a new "-u" option to phablet-test-run to unlock the screen.

To post a comment you must log in.
Sergio Schvezov (sergiusens) wrote :

do we really need to rm this way?
108 + os.system('rm -f /tmp/mir_socket')

Can't we have unity use the env setup? This shouldn't be a problem once the autopilot tests for unity8 are fixed anyways, so I'm ok with it for now, but doing it with os.system instead of open?
101 + override_file = "~/.config/upstart/unity8.override"
102 +
103 + os.system(
104 + 'echo "exec unity8 -testability" > ~/.config/upstart/unity8.override')

Can't we make unlocking the default action?
21 + -u Unlock the greeter before running the testsuite

review: Needs Fixing
Sergio Schvezov (sergiusens) wrote :

Adding Saviq to the review; I read somewhere that he was going to detect app launch with upstart and unlocking automatically

Andy Doan (doanac) wrote :

> do we really need to rm this way?
> 108 + os.system('rm -f /tmp/mir_socket')

was just copy/pasting the original. i'll fix that.

>
> Can't we have unity use the env setup? This shouldn't be a problem once the
> autopilot tests for unity8 are fixed anyways, so I'm ok with it for now, but
> doing it with os.system instead of open?
> 101 + override_file = "~/.config/upstart/unity8.override"
> 102 +
> 103 + os.system(
> 104 + 'echo "exec unity8 -testability" >
> ~/.config/upstart/unity8.override')

yeah, that's terrible. I'll fix.

> Can't we make unlocking the default action?
> 21 + -u Unlock the greeter before running the testsuite

I started with that approach. Given how close we are to the end, I thought this would offer the best chance of not causing a regression. If you are +1, then I'll make it default.

210. By Andy Doan on 2013-10-15

review comments from sergio for unlock_screen.py

211. By Andy Doan on 2013-10-15

make phablet-test-run default to unlocking the screen

also clean up the usage text so it displays properly on 80 columns

Michał Sawicz (saviq) wrote :

The plan is that we should restart unity8 under testability and use an emulator from the unity8 suite to do this, directly in app tests.

We might build most of that *into* the unity8 emulator, like you would import it, tell it to unlock, and it would do the things it needs (restart unity8 under testability, introspect it and use stuff maintained by unity8 to unlock).

Otherwise we'll end up with 10 places unlocking unity8, none of which are reliable or the unity8 team has control over.

review: Disapprove
Andy Doan (doanac) wrote :

On 10/15/2013 05:26 PM, Michał Sawicz wrote:
> The plan is that we should restart unity8 under testability and use an emulator from the unity8 suite to do this, directly in app tests.

I'm all for that, but until now hadn't been given an indication this was
going to happen. Is there a bug or something I can subscribe to for
updates on progress for this?

Christopher Lee (veebers) wrote :

Hi Andy,

You're right this has been talked about but nothing has happened yet. I think right now this should change :-)

I've filed this bug: https://bugs.launchpad.net/unity8/+bug/1240261

I'm currently working on it now, hopefully I should have something useful pretty soon.

Christopher Lee (veebers) wrote :

Andy,

I put this together today: https://code.launchpad.net/~veebers/unity8/adding_unlock_emulator/+merge/191328

The intention being that you consume these functions (namely unlock_unity and restart_unity_with_testability) in the unlock script (both here and any other script that does something similar i.e. Utah (and I think there is another CI one?).
This means that lines 117 - 181 aren't copied about in n-many scripts.

I'm not 100% certain this fits exactly Saviqs vision (i.e. done within the test app), I'll touch base with him in his morning and clarify then.

Let me know if you have any questions or suggestions.

Chris

Michał Sawicz (saviq) wrote :

On 16.10.2013 06:53, Christopher Lee wrote:
> I'm not 100% certain this fits exactly Saviqs vision (i.e. done within the test app), I'll touch base with him in his morning and clarify then.

I'm OK with that, even though this still means each test setup needs to
call those explicitly - but at least the unity8 team is owning this and
will know in case things break.

Andy Doan (doanac) wrote :

On 10/15/2013 11:53 PM, Christopher Lee wrote:

> The intention being that you consume these functions (namely unlock_unity and restart_unity_with_testability) in the unlock script (both here and any other script that does something similar i.e. Utah (and I think there is another CI one?).
> This means that lines 117 - 181 aren't copied about in n-many scripts.
>
> I'm not 100% certain this fits exactly Saviqs vision (i.e. done within the test app), I'll touch base with him in his morning and clarify then.
>
> Let me know if you have any questions or suggestions.

Sounds like there are three ways we could consume this code:

1) call these two functions from phablet-tools
2) modify all our autopilot tests to call this (or a common base class
if possible).

I'm guessing we don't have time to accomplish 2 before we release. So do
we want to try 1 or just keep pushing for 2?

-andy

Christopher Lee (veebers) wrote :

Hi Andy,

Yep those make sense to me. I'll get https://code.launchpad.net/~veebers/unity8/adding_unlock_emulator/+merge/191328 approved/merged and we can go ahead with option 1 as soon as that's done.

We/I can this start working on making it consumable within the ap tests themselves etc.

Sergio Schvezov (sergiusens) wrote :

On Wed, Oct 16, 2013 at 4:24 PM, Andy Doan <email address hidden>wrote:

> On 10/15/2013 11:53 PM, Christopher Lee wrote:
>
> > The intention being that you consume these functions (namely
> unlock_unity and restart_unity_with_testability) in the unlock script (both
> here and any other script that does something similar i.e. Utah (and I
> think there is another CI one?).
> > This means that lines 117 - 181 aren't copied about in n-many scripts.
> >
> > I'm not 100% certain this fits exactly Saviqs vision (i.e. done within
> the test app), I'll touch base with him in his morning and clarify then.
> >
> > Let me know if you have any questions or suggestions.
>
> Sounds like there are three ways we could consume this code:
>
> 1) call these two functions from phablet-tools
> 2) modify all our autopilot tests to call this (or a common base class
> if possible).
>
> I'm guessing we don't have time to accomplish 2 before we release. So do
> we want to try 1 or just keep pushing for 2?
>

I had a similar response to this that I wiped earlier. The problem with 2
is, once we test on desktop and desktop is unity8 and you run locally you
are going to end up killing your own session. So first we have to make sure
this doesn't happen.

Omer Akram (om26er) wrote :

I have a branch that calls the unity8 provided helpers to make sure the screen is unlocked. Also I had a chat with thomi and he believes that the unlocking logic should not be used inside each App' test suite and external tools like phablet-test-run should take care of that.

https://code.launchpad.net/~om26er/phablet-tools/unlock_screen/+merge/195529

Unmerged revisions

211. By Andy Doan on 2013-10-15

make phablet-test-run default to unlocking the screen

also clean up the usage text so it displays properly on 80 columns

210. By Andy Doan on 2013-10-15

review comments from sergio for unlock_screen.py

209. By Andy Doan on 2013-10-15

add option to phablet-test-run for unlocking the screen

This allows a user to pass "-u" to phablet-test-run. The
unlock_screen.py utility will be copied to the target and
then run right before executing the autopilot testsuite.

208. By Andy Doan on 2013-10-15

add unlock_screen.py utility for target device

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'phablet-test-run'
2--- phablet-test-run 2013-09-25 14:39:02 +0000
3+++ phablet-test-run 2013-10-15 19:17:48 +0000
4@@ -24,19 +24,25 @@
5 NOSHELL=0
6 RETVAL=0
7 VERBOSE=""
8+UNLOCK_SCREEN=1
9
10 print_usage() {
11 cat << EOF
12-usage: $0 [-s SERIAL] [-s] [-n] [-v] [-p PACKAGENAME] [-c PACKAGE] [-o DIR] [-a ARTIFACT] testsuite
13+usage: $0 [-s SERIAL] [-s] [-n] [-U] [-v] [-p PACKAGENAME] [-c PACKAGE] [-o DIR] [-a ARTIFACT] testsuite
14
15 Run the specified testsuite on the device
16
17 -s SERIAL Device serial number
18- -p PACKAGENAME Additional package to be installed, may be used multiple times
19- -c PACKAGE Copy and install local package to device, may be used multiple times
20+ -p PACKAGENAME Additional package to be installed, may be used multiple
21+ times
22+ -c PACKAGE Copy and install local package to device, may be used
23+ multiple times
24 -n Stop the shell during the test run
25 -o DIR Test report and artifacts output dir
26- -a ARTIFACT Artifact to fetch after the test, may be used multiple times (requires -o)
27+ -a ARTIFACT Artifact to fetch after the test, may be used multiple times
28+ (requires -o)
29+ -U By default the greeter is unlocked before a test runs. This
30+ disables that logic.
31 -v Run autopilot with -v option for more verbose output
32 EOF
33 }
34@@ -86,8 +92,16 @@
35 exec_with_adb_user initctl start unity8
36 }
37
38+unlock_screen() {
39+ adb push /usr/share/phablet-tools/target/unlock_screen.py /tmp/
40+ exec_with_adb_user python /tmp/unlock_screen.py
41+}
42+
43 run_test() {
44 exec_with_adb chmod 666 /dev/uinput
45+
46+ [ $UNLOCK_SCREEN -eq 1 ] && unlock_screen
47+
48 # adb shell always returns 0, so we have to do some hackery to get the
49 # actual RC from the test
50 {
51@@ -123,7 +137,7 @@
52 fi
53 }
54
55-while getopts a:c:hno:p:s:v opt; do
56+while getopts a:c:hno:p:s:Uv opt; do
57 case $opt in
58 a)
59 ARTIFACTS="$ARTIFACTS $OPTARG"
60@@ -137,6 +151,7 @@
61 ;;
62 n)
63 NOSHELL=1
64+ UNLOCK_SCREEN=0
65 ;;
66 o)
67 RESULTDIR=$OPTARG
68@@ -147,6 +162,9 @@
69 s)
70 ANDROID_SERIAL=$OPTARG
71 ;;
72+ U)
73+ UNLOCK_SCREEN=0
74+ ;;
75 v)
76 VERBOSE="-v"
77 ;;
78
79=== modified file 'setup.py'
80--- setup.py 2013-09-23 01:43:32 +0000
81+++ setup.py 2013-10-15 19:17:48 +0000
82@@ -26,6 +26,9 @@
83 author_email='sergio.schvezov@canonical.com',
84 license='GPLv3',
85 packages=find_packages(exclude=("tests",)),
86+ data_files=[
87+ ('share/phablet-tools/target', ['target/unlock_screen.py']),
88+ ],
89 scripts=PYTHON_SCRIPTS + SH_SCRIPTS,
90 test_suite='tests',
91 )
92
93=== added directory 'target'
94=== added file 'target/unlock_screen.py'
95--- target/unlock_screen.py 1970-01-01 00:00:00 +0000
96+++ target/unlock_screen.py 2013-10-15 19:17:48 +0000
97@@ -0,0 +1,92 @@
98+#!/usr/bin/env python
99+
100+from autopilot import introspection
101+from autopilot.display import Display
102+from autopilot.input import Pointer, Touch
103+import dbus
104+import os
105+import sys
106+
107+
108+def help():
109+ print("""Usage:
110+Run the script without any argument to unlock with assertion
111+
112+option -q:
113+Execute with parameter -q to unlock the screen quickly without introspection.
114+""")
115+
116+
117+def _rm(path):
118+ if os.path.exists(path):
119+ os.unlink(path)
120+
121+
122+def start_unity_in_testability():
123+ override_file = os.path.expanduser("~/.config/upstart/unity8.override")
124+
125+ with open(override_file, 'w') as f:
126+ f.write("exec unity8 -testability\n")
127+
128+ print("Stopping Unity")
129+ os.system('/sbin/stop unity8')
130+ _rm('/tmp/mir_socket')
131+
132+ print("Taking screen lock (#1235000)")
133+ bus = dbus.SystemBus()
134+ powerd = bus.get_object('com.canonical.powerd', '/com/canonical/powerd')
135+ powerd_cookie = powerd.requestSysState(
136+ "autopilot-lock", 1, dbus_interface='com.canonical.powerd')
137+ print("Starting Unity with testability")
138+ os.system('/sbin/start unity8')
139+ print("Releasing screen lock (#1235000)")
140+ powerd.clearSysState(powerd_cookie, dbus_interface='com.canonical.powerd')
141+
142+ _rm(override_file)
143+
144+
145+def unlock_screen():
146+ input_device = Touch.create()
147+ pointing_device = Pointer(input_device)
148+ conn = "com.canonical.Shell.BottomBarVisibilityCommunicator"
149+ unity8 = introspection.get_proxy_object_for_existing_process(
150+ connection_name=conn)
151+ greeter = unity8.select_single("Greeter")
152+ x, y, w, h = greeter.globalRect
153+ tx = x + w
154+ ty = y + (h / 2)
155+
156+ pointing_device.drag(tx, ty, tx / 2, ty)
157+ try:
158+ greeter.shown.wait_for(False)
159+ except AssertionError:
160+ print("THE SCREEN DIDN'T UNLOCK THE FIRST TRY, NOW ATTEMPTING "
161+ "A BLIND SWIPE")
162+ unlock_screen_blind(greeter)
163+
164+
165+def unlock_screen_blind(greeter=None):
166+ input_device = Touch.create()
167+ pointing_device = Pointer(input_device)
168+ x, y, w, h = Display.create(preferred_backend='UPA').get_screen_geometry(0)
169+ tx = x + w
170+ ty = y + (h / 2)
171+
172+ pointing_device.drag(tx, ty, tx / 2, ty)
173+ if greeter is not None:
174+ try:
175+ greeter.shown.wait_for(False)
176+ if greeter.shown is False:
177+ print("THE SCREEN IS NOW UNLOCKED")
178+ except AssertionError:
179+ print("THE SCREEN DIDN'T UNLOCK, RESTART THE DEVICE AND RUN "
180+ "THE SCRIPT AGAIN")
181+
182+
183+if len(sys.argv) >= 2 and sys.argv[1] == '-q':
184+ unlock_screen_blind()
185+elif len(sys.argv) >= 2 and sys.argv[1] == '-h':
186+ help()
187+else:
188+ start_unity_in_testability()
189+ unlock_screen()

Subscribers

People subscribed via source and target branches