Phablet Tools

Merge lp:~ogra/phablet-tools/phablet-test-run-fix-broken-shell into lp:phablet-tools

  1. phablet-test-run-fix-broken-shell
  2. Merge into trunk
Proposed by Oliver Grawert
Status: Merged
Approved by: Ricardo Mendoza
Approved revision: 324
Merged at revision: 323
Proposed branch: lp:~ogra/phablet-tools/phablet-test-run-fix-broken-shell
Merge into: lp:phablet-tools
Diff against target: 125 lines (+37/-15)
1 file modified
phablet-test-run (+37/-15)
To merge this branch: bzr merge lp:~ogra/phablet-tools/phablet-test-run-fix-broken-shell
Reviewer Review Type Date Requested Status
Para Siva (community) Approve
Paul Larson (community) Needs Fixing
PS Jenkins bot continuous-integration Approve
Ubuntu Phablet Team Pending
Review via email: mp+236252@code.launchpad.net

Commit message

phablet-test-run:
Do not force an environment-less shell (to prevent unsetting of DBUS_SESSION_BUS_ADDRESS by using /bin/sh in commands).
Add password/PIN option for using root commands.
Require password or PIN for package installation tasks.

Description of the change

phablet-test-run: do not force an environment-less shell, add password/PIN option for using root commands, require password or PIN for package installation tasks

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:323
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/412/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/93/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/412/rebuild

review: Needs Fixing (continuous-integration)
lp:~ogra/phablet-tools/phablet-test-run-fix-broken-shell updated
324. By Oliver Grawert

drop backslash

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:324
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/413/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/94
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/94
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/94

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/413/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paul Larson (pwlars) wrote :

In CI, we need to set up sudo before we get as far as phablet-test-run. So by the time we get here it's already been configured to allow passwordless sudo for the phablet user on the device. Can we not make -r a requirement for us since this is already done?

review: Needs Fixing
Revision history for this message
Oliver Grawert (ogra) wrote :

i thought the test scripts do the package setup in advance anyway so that you do not need the -p option (which means you wont need the -r option either) at least i have never seen -p used for phablet-test-run in the logs.

in a normal lab test run -p should not be needed.

more important for smoke testing is the removal of "/bin/sh -c" from the commands, which in the new world does the right thing and drops the environment to the one of /bin/sh ... which in turn means it also drops all knowledge about dbus and upstart sessions. without thid no app test will run at all.

Revision history for this message
Para Siva (psivaa) wrote :

Tested the messaging_app with this patch on a krillin device and the tests were executed properly and passed. Approving.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'phablet-test-run'
--- phablet-test-run 2014-09-08 16:30:46 +0000
+++ phablet-test-run 2014-09-28 16:44:17 +0000
@@ -33,16 +33,19 @@
33NOSHELL=033NOSHELL=0
34RETVAL=034RETVAL=0
35VERBOSE=""35VERBOSE=""
36ASKPASS=""
37SUDO=""
3638
37print_usage() {39print_usage() {
38 cat << EOF40 cat << EOF
39usage: $0 [-s SERIAL] [-s] [-n] [-x] [-v] [-p PACKAGENAME] [-c PACKAGE] [-o DIR] [-a ARTIFACT] [-f xml|subunit|text] [-A args] testsuite41usage: $0 [-s SERIAL] [-s] [-n] [-x] [-v] [-p PACKAGENAME] [ -r PASSWORD/PIN ] [-c PACKAGE] [-o DIR] [-a ARTIFACT] [-f xml|subunit|text] [-A args] testsuite
4042
41 Run the specified testsuite on the device43 Run the specified testsuite on the device
4244
43 -s SERIAL Device serial number45 -s SERIAL Device serial number
44 -p PACKAGENAME Additional package to be installed, may be used multiple46 -p PACKAGENAME Additional package to be installed, may be used multiple
45 times47 times (requires -r option)
48 -r PASSWORD/PIN Device password or PIN for package installation
46 -c PACKAGE Copy and install local package to device, may be used49 -c PACKAGE Copy and install local package to device, may be used
47 multiple times50 multiple times
48 -n Stop the shell during the test run51 -n Stop the shell during the test run
@@ -61,12 +64,30 @@
61 adb wait-for-device64 adb wait-for-device
62}65}
6366
67set_up_sudo() {
68 if [ -z "$PW" ]; then
69 echo "Please use the -r option to provide a device password or PIN"
70 echo
71 print_usage
72 exit 0
73 fi
74 ASKPASS="$(adb shell "mktemp -t sudo_askpass.XXXX|tr -d '\n'")"
75 adb shell "printf '#\x21/bin/sh\necho \"%s\"\n' '${PW}' >$ASKPASS; chmod +x $ASKPASS"
76 SUDO="SUDO_ASKPASS=$ASKPASS sudo -A"
77}
78
79clean_up_sudo() {
80 [ -n "$ASKPASS" ] && adb shell "rm -rf $ASKPASS"
81}
82
64exec_with_adb() {83exec_with_adb() {
65 adb shell /usr/bin/env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/tools/bin "$@"84 set_up_sudo
85 adb shell "LC_ALL=C $SUDO /usr/bin/env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/tools/bin $@"
86 clean_up_sudo
66}87}
6788
68exec_with_adb_user() {89exec_with_adb_user() {
69 adb shell sudo -u $USER -i "$@"90 adb shell "LC_ALL=C $@"
70}91}
7192
72install_packages() {93install_packages() {
@@ -113,39 +134,37 @@
113134
114135
115run_autopilot_test() {136run_autopilot_test() {
116 exec_with_adb chmod 666 /dev/uinput
117
118 # This directory doesn't exist in a freshly flashed device, so create it.137 # This directory doesn't exist in a freshly flashed device, so create it.
119 exec_with_adb mkdir -p /home/phablet/autopilot138 exec_with_adb_user mkdir -p /home/phablet/autopilot
120139
121 # adb shell always returns 0, so we have to do some hackery to get the140 # adb shell always returns 0, so we have to do some hackery to get the
122 # actual RC from the test141 # actual RC from the test
123 {142 {
124 apbase="sudo -iu $USER sh -c \"cd /home/phablet/autopilot; autopilot3 run $VERBOSE $APEXTRA"143 apbase="cd /home/phablet/autopilot; autopilot3 run $VERBOSE $APEXTRA"
125 if [ "$RESULTDIR" ]; then144 if [ "$RESULTDIR" ]; then
126 adb shell "$apbase -o /tmp/test_results.$APFORMAT -f $APFORMAT $TESTSUITE\"; echo ADB_RC=\$?"145 adb shell "$apbase -o /tmp/test_results.$APFORMAT -f $APFORMAT $TESTSUITE; echo ADB_RC=\$?"
127 if [ $APFORMAT = "xml" ]; then146 if [ $APFORMAT = "xml" ]; then
128 echo "***** Test summary *****"147 echo "***** Test summary *****"
129 exec_with_adb head -n 1 /tmp/test_results.$APFORMAT148 exec_with_adb_user "head -n 1 /tmp/test_results.$APFORMAT"
130 fi149 fi
131 else150 else
132 adb shell "$apbase $TESTSUITE\"; echo ADB_RC=\$?"151 adb shell "$apbase $TESTSUITE; echo ADB_RC=\$?"
133 fi152 fi
134 } | get_returncode153 } | get_returncode
135}154}
136155
137run_cli_command() {156run_cli_command() {
138 apbase="sudo -iu $USER sh -c \"cd /home/phablet; $TESTSUITE"157 apbase="cd /home/phablet; $TESTSUITE"
139 # adb shell always returns 0, so we have to do some hackery to get the158 # adb shell always returns 0, so we have to do some hackery to get the
140 # actual RC from the test159 # actual RC from the test
141 {160 {
142 if [ "$RESULTDIR" ]; then161 if [ "$RESULTDIR" ]; then
143 result_output=/tmp/test_results.$APFORMAT162 result_output=/tmp/test_results.$APFORMAT
144 adb shell 'rm -rf $result_output'163 adb shell 'rm -rf $result_output'
145 adb shell "$apbase | tee $result_output\"; echo ADB_RC=\$?"164 adb shell "$apbase | tee $result_output; echo ADB_RC=\$?"
146 adb pull $result_output $RESULTDIR165 adb pull $result_output $RESULTDIR
147 else166 else
148 adb shell "$apbase\"; echo ADB_RC=\$?"167 adb shell "$apbase; echo ADB_RC=\$?"
149 fi168 fi
150 } | get_returncode169 } | get_returncode
151170
@@ -166,7 +185,7 @@
166 fi185 fi
167}186}
168187
169while getopts A:a:c:f:hnxo:p:s:v opt; do188while getopts A:a:c:f:hnxo:p:r:s:v opt; do
170 case $opt in189 case $opt in
171 A)190 A)
172 APEXTRA=$OPTARG191 APEXTRA=$OPTARG
@@ -193,6 +212,9 @@
193 p)212 p)
194 TESTPACKAGES="$TESTPACKAGES $OPTARG"213 TESTPACKAGES="$TESTPACKAGES $OPTARG"
195 ;;214 ;;
215 r)
216 PW=$OPTARG
217 ;;
196 s)218 s)
197 SERIAL="$OPTARG"219 SERIAL="$OPTARG"
198 ;;220 ;;

Subscribers

People subscribed via source and target branches