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

[Oliver Grawert]

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.

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

[PS Jenkins bot (ps-jenkins)]

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]

324. By Oliver Grawert on 2014-09-28

drop backslash

[PS Jenkins bot (ps-jenkins)]

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)

[Paul Larson (pwlars)]

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

[Oliver Grawert (ogra)]

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.

[Para Siva (psivaa)]

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

review: Approve

=== 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 @@
 NOSHELL=0
 RETVAL=0
 VERBOSE=""
+ASKPASS=""
+SUDO=""
 
 print_usage() {
     cat << EOF
-usage: $0 [-s SERIAL] [-s] [-n] [-x] [-v] [-p PACKAGENAME] [-c PACKAGE] [-o DIR] [-a ARTIFACT] [-f xml|subunit|text] [-A args] testsuite
+usage: $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
 
   Run the specified testsuite on the device
 
   -s SERIAL        Device serial number
   -p PACKAGENAME   Additional package to be installed, may be used multiple
-                   times
+                   times (requires -r option)
+  -r PASSWORD/PIN  Device password or PIN for package installation
   -c PACKAGE       Copy and install local package to device, may be used
                    multiple times
   -n               Stop the shell during the test run
@@ -61,12 +64,30 @@
     adb wait-for-device
 }
 
+set_up_sudo() {
+    if [ -z "$PW" ]; then
+        echo "Please use the -r option to provide a device password or PIN"
+        echo
+        print_usage
+        exit 0
+    fi
+    ASKPASS="$(adb shell "mktemp -t sudo_askpass.XXXX|tr -d '\n'")"
+    adb shell "printf '#\x21/bin/sh\necho \"%s\"\n' '${PW}' >$ASKPASS; chmod +x $ASKPASS"
+    SUDO="SUDO_ASKPASS=$ASKPASS sudo -A"
+}
+
+clean_up_sudo() {
+    [ -n "$ASKPASS" ] && adb shell "rm -rf $ASKPASS"
+}
+
 exec_with_adb() {
-    adb shell /usr/bin/env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/tools/bin "$@"
+    set_up_sudo
+    adb shell "LC_ALL=C $SUDO /usr/bin/env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/tools/bin $@"
+    clean_up_sudo
 }
 
 exec_with_adb_user() {
-    adb shell sudo -u $USER -i "$@"
+    adb shell "LC_ALL=C $@"
 }
 
 install_packages() {
@@ -113,39 +134,37 @@
 
 
 run_autopilot_test() {
-    exec_with_adb chmod 666 /dev/uinput
-
     # This directory doesn't exist in a freshly flashed device, so create it.
-    exec_with_adb mkdir -p /home/phablet/autopilot
+    exec_with_adb_user mkdir -p /home/phablet/autopilot
 
     # adb shell always returns 0, so we have to do some hackery to get the
     # actual RC from the test
     {
-        apbase="sudo -iu $USER sh -c \"cd /home/phablet/autopilot; autopilot3 run $VERBOSE $APEXTRA"
+        apbase="cd /home/phablet/autopilot; autopilot3 run $VERBOSE $APEXTRA"
         if [ "$RESULTDIR" ]; then
-            adb shell "$apbase -o /tmp/test_results.$APFORMAT -f $APFORMAT $TESTSUITE\"; echo ADB_RC=\$?"
+            adb shell "$apbase -o /tmp/test_results.$APFORMAT -f $APFORMAT $TESTSUITE; echo ADB_RC=\$?"
 	    if [ $APFORMAT = "xml" ]; then
                 echo "***** Test summary *****"
-                exec_with_adb head -n 1 /tmp/test_results.$APFORMAT
+                exec_with_adb_user "head -n 1 /tmp/test_results.$APFORMAT"
             fi
         else
-            adb shell "$apbase $TESTSUITE\"; echo ADB_RC=\$?"
+            adb shell "$apbase $TESTSUITE; echo ADB_RC=\$?"
         fi
     } | get_returncode
 }
 
 run_cli_command() {
-    apbase="sudo -iu $USER sh -c \"cd /home/phablet; $TESTSUITE"
+    apbase="cd /home/phablet; $TESTSUITE"
     # adb shell always returns 0, so we have to do some hackery to get the
     # actual RC from the test
     {
         if [ "$RESULTDIR" ]; then
             result_output=/tmp/test_results.$APFORMAT
             adb shell 'rm -rf $result_output'
-            adb shell "$apbase | tee $result_output\"; echo ADB_RC=\$?"
+            adb shell "$apbase | tee $result_output; echo ADB_RC=\$?"
             adb pull $result_output $RESULTDIR
         else
-            adb shell "$apbase\"; echo ADB_RC=\$?"
+            adb shell "$apbase; echo ADB_RC=\$?"
         fi
     } | get_returncode
 
@@ -166,7 +185,7 @@
     fi
 }
 
-while getopts A:a:c:f:hnxo:p:s:v opt; do
+while getopts A:a:c:f:hnxo:p:r:s:v opt; do
     case $opt in
     A)
         APEXTRA=$OPTARG
@@ -193,6 +212,9 @@
     p)
         TESTPACKAGES="$TESTPACKAGES $OPTARG"
         ;;
+    r)
+        PW=$OPTARG
+        ;;
     s)
         SERIAL="$OPTARG"
         ;;