Closed Bug 636451 Opened 11 years ago Closed 11 years ago

devicemanager should have the ability to terminate when it can't launch a second process

Categories

(Testing :: General, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

Details

(Whiteboard: [mobile_unittests])

Attachments

(1 file, 1 obsolete file)

currently when we are launching a process, we check if it is running first and spit out a warning.  The problem is we continue to run and timeout because our intended process never launched.
Whiteboard: [mobile_unittests]
this patch adds in an option to fail a launchProcess/fireProcess call if we detect the process is already running.  Clint pointed out that launchProcess will return None if we fail getDeviceRoot() or if we fail fireProcess().  I think in both of these situations we should expect things to fail miserably.  It is unfortunate that we don't have a error code system, but this seems acceptable to me.

In addition, while testing this on a windows box, I found that we had some backslash issues and a simple fix in pushFile does the trick to get us up and running.
Assignee: nobody → jmaher
Attachment #514795 - Flags: review?(mcote)
Attachment #514795 - Flags: feedback?(ctalbert)
Comment on attachment 514795 [details] [diff] [review]
fail if we are already running (1.0)

+    if (os.name == "nt"):
+      destname = destname.replace('//', '\')

Not sure what this is doing... should this not be .replace('\\', '/')?  It's translating from a Windows path to a Unix path, right?

-    if self.fireProcess(cmdline) is None:
+    if self.fireProcess(cmdline, failIfRunning) is None:

Btw, in general you should be careful with "is".  It verifies that the two objects are one and the same--not just that their *values* are the same.  In this case it's fine, but in general == is preferred unless you have a really good reason to be comparing object identities.

Anyway good aside from my question up top.
Attachment #514795 - Flags: review?(mcote) → review+
updated for the fix.  I had transposed these and didn't catch it before hand.  I did a quick cleanup of some rouge print statements as well.
Attachment #514795 - Attachment is obsolete: true
Attachment #514795 - Flags: feedback?(ctalbert)
Attachment #514807 - Flags: review+
Attachment #514807 - Flags: feedback?(ctalbert)
Comment on attachment 514807 [details] [diff] [review]
fail if we are already running (1.1)

Looks good, thanks!
Attachment #514807 - Flags: feedback?(ctalbert) → feedback+
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=0c11cf1fe594
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.