If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

add uninstallApp method to devicemanager which does not do a reboot

RESOLVED FIXED

Status

Testing
Mozbase
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Trunk
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
in order to support panda boards, we need to uninstall applications without rebooting via sutagent, only via power cycle.  We have this support already in the agent via 'uninstall', we just need to call it from devicemanager.
(Assignee)

Comment 1

5 years ago
Created attachment 665043 [details] [diff] [review]
add in uninstallApp to devicemanager (1.0)
Assignee: nobody → jmaher
Attachment #665043 - Flags: review?(wlachance)
Comment on attachment 665043 [details] [diff] [review]
add in uninstallApp to devicemanager (1.0)

This looks ok to me. My sole problem is that we don't have an analogous method in devicemanagerADB, which might cause compatibility problems. Could you add such a method? Should be a straightforward invocation of "adb uninstall"
Attachment #665043 - Flags: review?(wlachance) → review-
(In reply to William Lachance (:wlach) from comment #2)
> Comment on attachment 665043 [details] [diff] [review]
> add in uninstallApp to devicemanager (1.0)
> 
> This looks ok to me. My sole problem is that we don't have an analogous
> method in devicemanagerADB, which might cause compatibility problems. Could
> you add such a method? Should be a straightforward invocation of "adb
> uninstall"

One other thought: Maybe we should make uninstall throw a DMError if it's unsuccessful), since we're on the road to doing that for everything anyway. I can do that in a followup if you don't want to bother with that now though.
(Assignee)

Comment 4

5 years ago
Created attachment 665398 [details] [diff] [review]
added support for ADB uninstall (2.0)

this adds adb support for uninstallApp and uninstallAppAndReboot as well as throws an exception in the failure case.
Attachment #665043 - Attachment is obsolete: true
Attachment #665398 - Flags: review?(wlachance)
(Assignee)

Comment 5

5 years ago
Created attachment 665436 [details] [diff] [review]
support uninstallApp in devicemanager (3.0)

this latest patch adds the exceptions in the SUT version as well.
Attachment #665398 - Attachment is obsolete: true
Attachment #665398 - Flags: review?(wlachance)
Attachment #665436 - Flags: review?(wlachance)
Comment on attachment 665436 [details] [diff] [review]
support uninstallApp in devicemanager (3.0)

Looks good, though there are some minor issues:

* I think these functions should now just return "None" (the default return value in python), since we're no longer supposed to check it.
* You missed a spot where we should throw an exception
* A debugging statement snuck in.

If all these fixed, r+!

>diff --git a/mozdevice/mozdevice/devicemanager.py b/mozdevice/mozdevice/devicemanager.py
>index 08c0dde..8d94458 100644
>--- a/mozdevice/mozdevice/devicemanager.py
>+++ b/mozdevice/mozdevice/devicemanager.py
>@@ -496,6 +496,20 @@ class DeviceManager:
>         """
> 
>     @abstractmethod
>+    def uninstallApp(self, appName, installPath=None):
>+        """
>+        Uninstalls the named application from device and DOES NOT cause a reboot
>+        appName - the name of the application (e.g org.mozilla.fennec)
>+        installPath - the path to where the application was installed (optional)
>+
>+        Returns True if SUTAgent successfully remove the application

Remove this line, we describe return values below

>+
>+        returns:
>+          success: True

success: None

>+          failure: DMError exception thrown
>+        """
>+
>+    @abstractmethod
>     def uninstallAppAndReboot(self, appName, installPath=None):
>         """
>         Uninstalls the named application from device and causes a reboot
>@@ -507,7 +521,7 @@ class DeviceManager:
> 
>         returns:
>           success: True
>-          failure: None
>+          failure: DMError exception thrown
>         """

success: None

>     @abstractmethod
>diff --git a/mozdevice/mozdevice/devicemanagerADB.py b/mozdevice/mozdevice/devicemanagerADB.py
>index 060acd2..52db820 100644
>--- a/mozdevice/mozdevice/devicemanagerADB.py
>+++ b/mozdevice/mozdevice/devicemanagerADB.py
>@@ -806,6 +806,41 @@ class DeviceManagerADB(DeviceManager):
>         print ret
>         return ret
> 
>+    def uninstallApp(self, appName, installPath=None):
>+        """
>+        Uninstalls the named application from device and DOES NOT cause a reboot
>+        appName - the name of the application (e.g org.mozilla.fennec)
>+        installPath - ignored, but used for compatibility with SUTAgent
>+
>+        Returns True if SUTAgent successfully remove the application

Remove this line, we describe return values below

>+        returns:
>+          success: True

success: None


>+          failure: DMError exception thrown
>+        """
>+        data = self._runCmd(["uninstall", appName]).stdout.read().strip()
>+        status = data.split('\n')[0].strip()
>+        if status == 'Success':
>+            return True

Just "return"

>+        raise DMError("uninstall failed for %s" % appName)
>+
>+    def uninstallAppAndReboot(self, appName, installPath=None):
>+        """
>+        Uninstalls the named application from device and causes a reboot
>+        appName - the name of the application (e.g org.mozilla.fennec)
>+        installPath - ignored, but used for compatibility with SUTAgent
>+
>+        Returns True, but it doesn't mean anything other than the command was sent,
>+        the reboot happens and we don't know if this succeeds or not

Remove this line, we describe return values below

>+        returns:
>+          success: True

success: None

>+          failure: DMError exception thrown
>+        """
>+        results = self.uninstallApp(appName)
>+        self.reboot()
>+        return True
>+
>     def _runCmd(self, args):
>         """
>         Runs a command using adb
>@@ -940,8 +975,7 @@ class DeviceManagerADB(DeviceManager):
>             if deviceStatus == None:
>                 raise DMError("device not found: %s" % self.deviceSerial)
>             elif deviceStatus != "device":
>-                raise DMError("bad status for device %s: %s" % (self.deviceSerial,
>-                                                                                                                deviceStatus))
>+                raise DMError("bad status for device %s: %s" % (self.deviceSerial, deviceStatus))
> 
>         # Check to see if we can connect to device and run a simple command
>         try:
>diff --git a/mozdevice/mozdevice/devicemanagerSUT.py b/mozdevice/mozdevice/devicemanagerSUT.py
>index 5f9d7fd..9b3fe55 100644
>--- a/mozdevice/mozdevice/devicemanagerSUT.py
>+++ b/mozdevice/mozdevice/devicemanagerSUT.py
>@@ -12,7 +12,7 @@ import posixpath
> import subprocess
> from threading import Thread
> import StringIO
>-from devicemanager import DeviceManager, FileError, NetworkTools, _pop_last_line
>+from devicemanager import DeviceManager, FileError, DMError, NetworkTools, _pop_last_line
> import errno
> from distutils.version import StrictVersion
> 
>@@ -1149,6 +1149,34 @@ class DeviceManagerSUT(DeviceManager):
>                 return line
>         return None
> 
>+    def uninstallApp(self, appName, installPath=None):
>+        """
>+        Uninstalls the named application from device and DOES NOT cause a reboot
>+        appName - the name of the application (e.g org.mozilla.fennec)
>+        installPath - the path to where the application was installed (optional)
>+
>+        Returns True if SUTAgent successfully remove the application

Remove this line, we describe return values below

>+
>+        returns:
>+          success: True

success: None

>+          failure: DMError exception thrown
>+        """
>+        cmd = 'uninstall ' + appName
>+        if installPath:
>+            cmd += ' ' + installPath
>+        try:
>+            data = self._runCmds([{ 'cmd': cmd }])
>+        except AgentError, err:
>+            print "Remote Device Error: Error uninstalling app: %s" % err
>+            return None

Let's raise a DMError here instead of returning None

>+        status = data.split('\n')[0].strip()
>+        if self.debug > 3:
>+            print "uninstallApp: '%s'" % status
>+        if status == 'Success':
>+            return True

Just return

>+        raise DMError("uninstall failed for %s" % appName)
>+
>     def uninstallAppAndReboot(self, appName, installPath=None):
>         """
>         Uninstalls the named application from device and causes a reboot
>@@ -1160,7 +1188,7 @@ class DeviceManagerSUT(DeviceManager):
> 
>         returns:
>           success: True

success: None

>-          failure: None
>+          failure: DMError exception thrown
>         """
>         cmd = 'uninst ' + appName
>         if installPath:
>@@ -1168,8 +1196,9 @@ class DeviceManagerSUT(DeviceManager):
>         try:
>             data = self._runCmds([{ 'cmd': cmd }])
>         except AgentError:
>-            return None
>+            raise DMError("uninstall failed for %s" % appName)
> 
>+        print data.split('\n')

This looks like a debugging statement that snuck in?
Attachment #665436 - Flags: review?(wlachance) → review+
(Assignee)

Comment 7

5 years ago
https://github.com/mozilla/mozbase/commit/9f222faa01411d9d565bbe12d30dc08b06a26db0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.