This is a bug I found out. I don't know if this is the right way of handling it. I don't even know why we run self.getDeviceRoot() at the beginning of all.

Comment on attachment 605546 [details] [diff] [review] raise exception when DeviceManager fails to initialize Review of attachment 605546 [details] [diff] [review]: ----------------------------------------------------------------- this should be in the m-c/build/mobile/devicemanagerSUT.py script though.

I still have to write an email to try to figure out the way forward since we're running for tegras the old version that lived on talos back in November. This patch is for m-c. I will carry forward your review on my next bug change and run it through the try server.

Comment on attachment 605859 [details] [diff] [review] [checked-in] [m-c] raise exception when DeviceManager fails to initialize Whatever. I thought I could keep your name from switching from "?" to "+".

Autoland Patchset: Patches: 605859 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=aa4da7f6d0d2 Try run started, revision aa4da7f6d0d2. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=aa4da7f6d0d2

Try run for aa4da7f6d0d2 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=aa4da7f6d0d2 Results (out of 26 total builds): success: 23 warnings: 2 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-aa4da7f6d0d2

Comment on attachment 605546 [details] [diff] [review] raise exception when DeviceManager fails to initialize >diff --git a/talos/devicemanagerSUT.py b/talos/devicemanagerSUT.py >--- a/talos/devicemanagerSUT.py >+++ b/talos/devicemanagerSUT.py >@@ -69,17 +69,18 @@ class DeviceManagerSUT(DeviceManager): > # pushFile() and other file-related commands could set filesystem errors, etc. > > def __init__(self, host, port = 20701, retrylimit = 5): > self.host = host > self.port = port > self.retrylimit = retrylimit > self.retries = 0 > self._sock = None >- self.getDeviceRoot() >+ if self.getDeviceRoot() == None: >+ raise BaseException("Failed to connect to SUT Agent and retrieve the device root.") Not marked as a reviewer, but I'm a bit concerned about this change. I though DeviceManager* wasn't supposed to emit exceptions? And regardless, BaseException seems like a strange exception to throw: if we were going to throw anything, I'd throw DMError.

I am fine using DMError. jmaher? I decided to use an exception to somehow let the user know that something went wrong. If you can think of an alternative approach I am all ears :)

Nothing on the try push show that anything broke with this. Once you let me know if you would like me to change the patch please let me know. Otherwise, I can land anytime.

(In reply to Armen Zambrano G. [:armenzg] - Release Engineer from comment #7) > I am fine using DMError. jmaher? > > I decided to use an exception to somehow let the user know that something > went wrong. > If you can think of an alternative approach I am all ears :) I actually think raising an exception is the best approach. I was chatting with jmaher a bit on irc, and we think this can go ahead, but we will need to modify instantiations of devicemanager to handle the exception. m-c: testing/mochitest/runtestsremote.py layout/tools/reftest/remotereftests.py talos: talos/run_tests.py talos/remotePerfConfigurator.py I'm not sure exactly how involved this is.

IIUC those files would have failed by the time they called dm.sendCMD(). With this patch we would fail a little earlier and with an informative traceback, I think.

Comment on attachment 605859 [details] [diff] [review] [checked-in] [m-c] raise exception when DeviceManager fails to initialize http://hg.mozilla.org/integration/mozilla-inbound