783839 - Tegra verify script should not print out "True" and confuse sheriffs

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Justin Wood (:Callek)]

Currently our verify.py script will print out a Boolean return code, as converted to a string.

see: e.g.

python /builds/sut_tools/verify.py
 in dir /builds/tegra-171/test/build (timeout 1200 secs)
 watching logfiles {}
 argv: ['python', '/builds/sut_tools/verify.py']
 environment:
  PATH=/opt/local/bin:/opt/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin
  PWD=/builds/tegra-171/test/build
  SUT_IP=10.250.50.81
  SUT_NAME=tegra-171
  __CF_USER_TEXT_ENCODING=0x1F5:0:0
 closing stdin
 using PTY: False
DEBUG: updateSUT: Using tegra 'tegra-171' found in env variable
INFO: Using tegra 'tegra-171' found in env variable
INFO: attempting to ping tegra
reconnecting socket
INFO: updateSUT.py: We're running SUTAgentAndroid Version 1.11
INFO: Got expected SUTAgent version '1.11'
INFO: attempting to create file /mnt/sdcard/writetest
removing file: /mnt/sdcard/writetest
True


This is due to just an erroneous line in http://mxr.mozilla.org/build/source/tools/sut_tools/cleanup.py#45

What [I think] should happen is we check the return value of that function, and if it returns True to print an INFO "removed install of <> from tegra". Or some such.

On thinking about it this could be goodfirstbug fodder, so I CC'ed ewong, but if the thought of touching the mobile infra scares him, that is fine, it would also be a good "feet wet" task for anyone, so also CC'ed kim.

Comment 1

[Edmund Wong (:ewong)]

Attachment: Tegra verify script should not print out "True". (v1)

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 653133 [details] [diff] [review]
Tegra verify script should not print out "True". (v1)

># HG changeset patch
># Parent 3f01be09b5f3481f561a9d031e27923bcfd5172b
># User Edmund Wong <[e-mail]>
>Bug 783839 - Tegra verify script should not print out 'True'.
>
>diff --git a/sut_tools/cleanup.py b/sut_tools/cleanup.py
>--- a/sut_tools/cleanup.py
>+++ b/sut_tools/cleanup.py
>@@ -37,19 +37,22 @@ def main(tegra=None, dm=None):
> 
>     if dm is None:
>         print "Connecting to: " + tegra
>         dm = devicemanager.DeviceManagerSUT(tegra)
>         dm.debug = 5
> 
>     for p in processNames:
>         if dm.dirExists('/data/data/%s' % p):
>-            print dm.uninstallAppAndReboot(p)
>+            if dm.uninstallAppAndReboot(p):
>+                print "INFO: removed install of %s from tegra" % p

On thinking about this lets go with this message, "INFO: uninstalling %s from device"  since we have other types of boards coming, and the process, and that this line alone doesn't actually tell us for sure that we *did* remove it.

>+            else:
>+                print "DM Error"

And I'd rather ignore the non-true return code, for now. We verify that we did remove this below anyway, and "DM Error" doesn't really tell the reader of these logs much, unless they understand what it means to fail here already.

r- for that, but I expect an r+ shortly after I am able to test your v2 patch

Comment 3

[Edmund Wong (:ewong)]

Attachment: Tegra verify script should not print out "True". (v2))

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 653134 [details] [diff] [review]
Tegra verify script should not print out "True". (v2))

Looks good, please hold off on landing, as with all things (even trivial things) with mobile, I want to stage this, but won't be tossing it on staging tonight. f? to remind me to do so.

Thanks!

Comment 5

[Justin Wood (:Callek)]

this line changed a few times since ewongs patch, and I blame myself for not staging it in time, I folded it into my own patch in Bug 811454 therefore I'm closing this one as "fixed" (since we have the fix, and one way or another it will get landed)

Thanks for the patch!