the sut_tools need to be called as components (i.e. main() vs a raw script) and we should make many of the tasks optional via flags to the function.

one of the last patches

Comment on attachment 665599 [details] [diff] [review] refactor many of the sut_tools (1.0) Review of attachment 665599 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, I just want your answers on installApp.py before I r+ -- and I know this patch relies on the mozdevice one ;-) ::: sut_tools/installApp.py @@ +32,5 @@ > + try: > + print dm._runCmds([{'cwd': 'exec su -c "logcat -d -v time *:W"'}]) > + except devicemanager.DMError, e: > + setFlag(errorFile, "Remote Device Error: Exception hit while trying to run logcat: %s" % str(e)) > + sys.exit(1) nit: return 1 @@ +151,5 @@ > sys.exit(1) > > + except devicemanager.AgentError, err: > + log.error("remoteDeviceError: while doing one time setup for installation: %s" % err) > + return None, None Does this return None, None make sense with the sys.exit(1) right above it? @@ +175,5 @@ > + if not dm: > + sys.exit(1) > + > + if installOneApp(dm, devRoot, path_to_main_apk, proxyFile, errorFile): > + sys.exit(1) In-fact I see a large mix of sys.exit() vs return throughout this file... can we only use sys.exit() when we are __name__ == "__main__" if we're doing this? ::: sut_tools/sut_lib.py @@ +451,4 @@ > try: > dm._runCmds([{'cmd': 'settime %s' % s}]) > return True > + except devicemanager.AgentError, e: I'll take your word that this is right ;-) @@ +475,5 @@ > + if dm._sock: > + dm._sock.close() > + dm._sock = None > + dm.deviceRoot = None > + dm.retries = 0 ...this is the "abort retry faster"? or something else ::: sut_tools/verify.py @@ +269,1 @@ > return False NOTE: we don't yet ever try to fix the screen ;-)

@@ +475,5 @@ > + if dm._sock: > + dm._sock.close() > + dm._sock = None > + dm.deviceRoot = None > + dm.retries = 0 ...this is the "abort retry faster"? or something else ^ this code resets the retry limit so we actually attempt to reconnect after the first failure. Otherwise we sit in a loop try to get the deviceRoot without trying to connect. I can work on the sys.exit() and return values, good feedback!

updated to address return vs sys.exit.

Comment on attachment 666592 [details] [diff] [review] refactor many of the sut_tools (1.1) Review of attachment 666592 [details] [diff] [review]: ----------------------------------------------------------------- will add to staging later today, or early tomorrow

I suspect we'll need to adjust the smoketest script that joel has a review pending on me, but we noticed that jobs were failing in production (especially after try jobs get canceled) since we stopped checking for stalled xpcshell servers. This should fix it.

Comment on attachment 668715 [details] [diff] [review] [tools] regression fix - Actually checkStalled/watcherINI normally in prod That ought to behave more nicely.

So after I pushed the regression fix, it turned out we took out ALL tegras from the pool ::sad face:: 10/05/2012 20:03:03: INFO: /builds/tegra-173/error.flg Unable to identify if watcher.ini is current 10/05/2012 20:03:33: INFO: verifyDevice: failing to set watcher.ini program finished with exit code 1 elapsedTime=206.671093 ========= Finished Running verify.py failed (results: 5, elapsed: 3 mins, 26 secs) (at 2012-10-05 20:03:33.777404) ========= So I'm reverting the Boolean that sets it, and we'll work on fixing this code up no later than Monday. Since we now lost the ability to update watcherINI.

http://hg.mozilla.org/build/tools/rev/acdfb63e76da regression fixes: http://hg.mozilla.org/build/tools/rev/c2e4a0deeae3 http://hg.mozilla.org/build/tools/rev/8b5a1fcf46ac

Comment on attachment 668715 [details] [diff] [review] [tools] regression fix - Actually checkStalled/watcherINI normally in prod Review of attachment 668715 [details] [diff] [review]: ----------------------------------------------------------------- aye, my bad

Comment on attachment 668725 [details] [diff] [review] [tools] regression fix to regression fix - Don't update watcher.ini Review of attachment 668725 [details] [diff] [review]: ----------------------------------------------------------------- umm, you are flip flopping a lot, lets open a new bug to resolve this watcherini, on the panda board we can't look for it and I tried a few things to make it work.