Merge lp:~mvo/software-center/lp917096 into lp:software-center

[Michael Vogt]

This fixes various issues around the debfile handling.

- it no longer crashes when viewing a debfile
- if a debfile is not installable it shows a error and hides the action button
- adds tests for this behavior

[Gary Lasker (gary-lasker)]

Looks great, thanks mvo!

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-02-15 18:32:24 +0000
+++ debian/changelog	2012-02-16 10:28:26 +0000
@@ -1,3 +1,10 @@
+software-center (5.1.10) UNRELEASED; urgency=low
+
+  * fix crash when opening a debfile and add regression test
+    (LP: #917096)
+
+ -- Michael Vogt <[e-mail]>  Thu, 16 Feb 2012 10:50:56 +0100
+
 software-center (5.1.9) precise; urgency=low
 
   [ Gabor Kelemen ]  

=== modified file 'softwarecenter/db/__init__.py'
--- softwarecenter/db/__init__.py	2011-08-11 09:25:56 +0000
+++ softwarecenter/db/__init__.py	2012-02-16 10:28:26 +0000
@@ -1,6 +1,8 @@
+import logging
 try:
     from debfile import DebFileApplication
     DebFileApplication # pyflakes
-except ImportError:
+except:
+    logging.exception("DebFileApplication import")
     class DebFileApplication(): 
         pass

=== modified file 'softwarecenter/db/application.py'
--- softwarecenter/db/application.py	2012-02-14 14:22:35 +0000
+++ softwarecenter/db/application.py	2012-02-16 10:28:26 +0000
@@ -32,7 +32,7 @@
 from softwarecenter.paths import (APP_INSTALL_CHANNELS_PATH,
                                   SOFTWARE_CENTER_ICON_CACHE_DIR,
                                   )
-from softwarecenter.utils import utf8, split_icon_ext, version_compare
+from softwarecenter.utils import utf8, split_icon_ext
 from softwarecenter.region import get_region_cached, REGIONTAG
 
 LOG = logging.getLogger(__name__)
@@ -613,6 +613,7 @@
         """ take a screenshot result dict from screenshots.debian.org 
             and sort it
         """
+        from softwarecenter.utils import version_compare
         my_version = self.version
         # discard screenshots which are more recent than the available version
         for item in screenshot_list[:]:

=== modified file 'softwarecenter/db/debfile.py'
--- softwarecenter/db/debfile.py	2012-01-05 08:44:43 +0000
+++ softwarecenter/db/debfile.py	2012-02-16 10:28:26 +0000
@@ -28,14 +28,16 @@
 from softwarecenter.utils import ExecutionTime, utf8
 
 class DebFileApplication(Application):
+
     def __init__(self, debfile):
-        # deb overrides this
-#        if not debfile.endswith(".deb") and not debfile.count('/') >= 2:
- #           raise ValueError("Need a deb file, got '%s'" % debfile)
+        # sanity check
+        if not debfile.endswith(".deb"):
+            raise ValueError("Need a deb file, got '%s'" % debfile)
+        # work out debname/appname
         debname = os.path.splitext(os.path.basename(debfile))[0]
-        self.appname = ""
-        self.pkgname = debname.split('_')[0].lower()
-        self.request = debfile
+        pkgname = debname.split('_')[0].lower()
+        # call the constructor
+        Application.__init__(self, pkgname=pkgname, request=debfile)
     def get_details(self, db):
         with ExecutionTime("get_details for DebFileApplication"):
             details = AppDetailsDebFile(db, application=self)
@@ -49,10 +51,10 @@
             raise ValueError("doc must be None for deb files")
 
         try:
-            # for some reason Cache() is much faster than "self._cache._cache"
-            # on startup
             with ExecutionTime("create DebPackage"):
-                self._deb = DebPackage(self._app.request, Cache())
+                # Cache() used to be faster here than self._cache._cache
+                # but that is no longer the case with the latest apt
+                self._deb = DebPackage(self._app.request, self._cache._cache)
         except:
             self._deb = None
             self._pkg = None
@@ -90,7 +92,7 @@
         # check deb and set failure state on error
         with ExecutionTime("AppDetailsDebFile._deb.check()"):
             if not self._deb.check():
-                self._error = self._deb._failure_string
+                self._error = self._deb._failure_string.strip()
 
     @property
     def description(self):

=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py	2012-01-19 16:39:15 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-02-16 10:28:26 +0000
@@ -59,8 +59,13 @@
     """
 
     class Pages():
-        # page names, useful for debuggin
-        NAMES = ('lobby', 'subcategory', 'list', 'details', 'purchase')
+        # page names, useful for debugging
+        NAMES = ('lobby', 
+                 'subcategory',
+                 'list',
+                 'details', 
+                 'purchase',
+                )
         # actual page id's
         (LOBBY,
          SUBCATEGORY,

=== modified file 'softwarecenter/ui/gtk3/views/appdetailsview.py'
--- softwarecenter/ui/gtk3/views/appdetailsview.py	2012-02-15 17:11:44 +0000
+++ softwarecenter/ui/gtk3/views/appdetailsview.py	2012-02-16 10:28:26 +0000
@@ -309,9 +309,7 @@
         elif state == PkgStates.NOT_FOUND:
             self.hide()
         elif state == PkgStates.ERROR:
-            self.progress.hide()
-            self.button.set_sensitive(False)
-            self.button.show()
+            # error details are set below
             self.show()
         else:
             # mvo: why do we override state here again?
@@ -432,8 +430,10 @@
         elif state == PkgStates.ERROR:
             # this is used when the pkg can not be installed
             # we display the error in the description field
-            self.set_button_label(_("Install"))
-            self.set_label("")
+            self.installed_icon.hide()
+            self.progress.hide()
+            self.button.hide()
+            self.set_label(_("Error"))
         elif state == PkgStates.NOT_FOUND:
             # this is used when the pkg is not in the cache and there is no
             # request we display the error in the summary field and hide the 
@@ -1394,10 +1394,7 @@
 
     def _update_app_description(self, app_details, appname):
         # format new app description
-        if app_details.pkg_state == PkgStates.ERROR:
-            description = app_details.error
-        else:
-            description = app_details.description
+        description = app_details.description
         if not description:
             description = " "
         self.desc.set_description(description, appname)
@@ -1452,6 +1449,11 @@
         return
 
     def _update_warning_bar(self, app_details):
+        # generic error wins over HW issue
+        if app_details.pkg_state == PkgStates.ERROR:
+            self.pkg_warningbar.show()
+            self.pkg_warningbar.label.set_text(app_details.error)
+            return
         if (app_details.hardware_requirements_satisfied and
             app_details.region_requirements_satisfied):
             self.pkg_warningbar.hide()

=== added file 'test/gtk3/test_debfile_view.py'
--- test/gtk3/test_debfile_view.py	1970-01-01 00:00:00 +0000
+++ test/gtk3/test_debfile_view.py	2012-02-16 10:28:26 +0000
@@ -0,0 +1,47 @@
+#!/usr/bin/python
+
+import pickle
+import os
+import subprocess
+import time
+import unittest
+
+from mock import Mock
+
+from testutils import setup_test_env, do_events
+setup_test_env()
+
+import softwarecenter.paths
+from softwarecenter.ui.gtk3.app import SoftwareCenterAppGtk3
+from softwarecenter.ui.gtk3.panes.availablepane import AvailablePane
+
+class DebFileOpenTestCase(unittest.TestCase):
+
+    def test_deb_file_view_error(self):
+        mock_options = Mock()
+        mock_options.display_navlog = False
+        mock_options.disable_apt_xapian_index = False
+        mock_options.disable_buy = False
+        xapianpath = softwarecenter.paths.XAPIAN_BASE_PATH
+        app = SoftwareCenterAppGtk3(
+            softwarecenter.paths.datadir, xapianpath, mock_options)
+        app.run(["./data/test_debs/gdebi-test1.deb"])
+        # give it time
+        for i in range(10):
+            time.sleep(0.1)
+            do_events()
+            # its ok to break out early
+            if (app.available_pane.app_details_view and
+                app.available_pane.app_details_view.get_property("visible")):
+                break
+        # check that we are on the right page
+        self.assertEqual(
+            app.available_pane.get_current_page(), AvailablePane.Pages.DETAILS)
+        # this is deb that is not installable
+        action_button = app.available_pane.app_details_view.pkg_statusbar.button
+        self.assertFalse(action_button.get_property("visible"))
+        
+
+
+if __name__ == "__main__":
+    unittest.main()

=== modified file 'test/test_debfileapplication.py'
--- test/test_debfileapplication.py	2012-01-16 14:42:49 +0000
+++ test/test_debfileapplication.py	2012-02-16 10:28:26 +0000
@@ -17,10 +17,10 @@
 DEBFILE_VERSION = '1.0'
 DEBFILE_WARNING = 'Only install this file if you trust the origin.'
 
-DEBFILE_REINSTALLABLE = './data/test_debs/gdebi-test1.deb'
 DEBFILE_PATH_NOTFOUND = './data/test_debs/notfound.deb'
 DEBFILE_PATH_NOTADEB = './data/notadeb.txt'
 DEBFILE_PATH_CORRUPT = './data/test_debs/corrupt.deb'
+DEBFILE_NOT_INSTALLABLE = './data/test_debs/gdebi-test1.deb'
 
 class TestDebFileApplication(unittest.TestCase):
     """ Test the class DebFileApplication """
@@ -46,13 +46,19 @@
 
         self.assertEquals(debfiledetails.pkg_state, PkgStates.UNINSTALLED)
 
-    # disabled until the fixme gets fixed
+    def test_get_pkg_state_not_installable(self):
+        debfileapplication = DebFileApplication(DEBFILE_NOT_INSTALLABLE)
+        debfiledetails = debfileapplication.get_details(self.db)
+
+        self.assertEquals(debfiledetails.pkg_state, PkgStates.ERROR)
+
     def disabled_for_now_test_get_pkg_state_reinstallable(self):
         # FIMXE: add hand crafted dpkg status file into the testdir so
         #        that gdebi-test1 is marked install for the MockAptCache
-        debfileapplication = DebFileApplication(DEBFILE_REINSTALLABLE)
-        debfiledetails = debfileapplication.get_details(self.db)
-        self.assertEquals(debfiledetails.pkg_state, PkgStates.REINSTALLABLE)
+        #debfileapplication = DebFileApplication(DEBFILE_REINSTALLABLE)
+        #debfiledetails = debfileapplication.get_details(self.db)
+        #self.assertEquals(debfiledetails.pkg_state, PkgStates.REINSTALLABLE)
+        pass
 
     def test_get_pkg_state_not_found(self):
         debfileapplication = DebFileApplication(DEBFILE_PATH_NOTFOUND)