fixes

- improve unit test coverage & fix bugs exposed thereby
- lint

salt/spm/__init__.py

@@ -109,6 +109,8 @@ class SPMClient(object):
                 self._list_files(args)
             elif command == 'info':
                 self._info(args)
+            else:
+                raise SPMInvocationError('Invalid command \'{0}\''.format(command))
         except SPMException as exc:
             self.ui.error(str(exc))
 
@@ -393,9 +395,9 @@ class SPMClient(object):
         if not self.opts['assume_yes']:
             self.ui.confirm(msg)
 
-        ui.status('... removing')
+        self.ui.status('... removing')
 
-        if not self.pkgfiles['{0}.db_exists'.format(self.db_prov)](self.opts['spm_db']):
+        if not self.pkgdb['{0}.db_exists'.format(self.db_prov)](self.opts['spm_db']):
             raise SPMDatabaseError('No database at {0}, cannot remove {1}'.format(self.opts['spm_db'], package))
 
         # Look at local repo index
@@ -442,6 +444,9 @@ class SPMClient(object):
 
         pkg_file = args[1]
 
+        if not os.path.exists(pkg_file):
+            raise SPMInvocationError('Package file {0} not found'.format(pkg_file))
+
         comps = pkg_file.split('-')
         comps = '-'.join(comps[:-2]).split('/')
         name = comps[-1]

tests/unit/spm.py

@@ -4,12 +4,11 @@
 from __future__ import absolute_import
 import os
 import shutil
-import tarfile
 import tempfile
 
 # Import Salt Testing libs
 from salttesting import TestCase
-from salttesting.helpers import ensure_in_syspath
+from salttesting.helpers import ensure_in_syspath, destructiveTest
 
 import salt.config
 import salt.spm
@@ -36,6 +35,7 @@ __opts__ = {
     'file_roots': {'base': [os.path.join(_TMP_SPM, 'salt')]},
     'pillar_roots': {'base': [os.path.join(_TMP_SPM, 'pillar')]},
     'assume_yes': True,
+    'force': False,
 }
 
 _F1 = {
@@ -61,6 +61,7 @@ _F1['contents'] = (
 )
 
 
+@destructiveTest
 class SPMTestUserInterface(salt.spm.SPMUserInterface):
     '''
     Unit test user interface to SPMClient
@@ -68,6 +69,7 @@ class SPMTestUserInterface(salt.spm.SPMUserInterface):
     def __init__(self):
         self._status = []
         self._confirm = []
+        self._error = []
 
     def status(self, msg):
         self._status.append(msg)
@@ -75,6 +77,9 @@ class SPMTestUserInterface(salt.spm.SPMUserInterface):
     def confirm(self, action):
         self._confirm.append(action)
 
+    def error(self, msg):
+        self._error.append(msg)
+
 
 class SPMTest(TestCase):
     def setUp(self):
@@ -121,6 +126,48 @@ class SPMTest(TestCase):
                 ('release', 'Release: {0}'),
                 ('summary', 'Summary: {0}')):
             assert line.format(_F1['definition'][key]) in lines
+        # Reinstall with force=False, should fail
+        self.ui._error.clear()
+        self.client.run(['local', 'install', pkgpath])
+        assert len(self.ui._error) > 0
+        # Reinstall with force=True, should succeed
+        __opts__['force'] = True
+        self.ui._error.clear()
+        self.client.run(['local', 'install', pkgpath])
+        assert len(self.ui._error) == 0
+        __opts__['force'] = False
+
+    def test_failure_paths(self):
+        fail_args = (
+            ['bogus', 'command'],
+            ['create_repo'],
+            ['build'],
+            ['build', '/nonexistent/path'],
+            ['info'],
+            ['info', 'not_installed'],
+            ['files'],
+            ['files', 'not_installed'],
+            ['install'],
+            ['install', 'nonexistent.spm'],
+            ['remove'],
+            ['remove', 'not_installed'],
+            ['local', 'bogus', 'command'],
+            ['local', 'info'],
+            ['local', 'info', '/nonexistent/path/junk.spm'],
+            ['local', 'files'],
+            ['local', 'files', '/nonexistent/path/junk.spm'],
+            ['local', 'install'],
+            ['local', 'install', '/nonexistent/path/junk.spm'],
+            ['local', 'list'],
+            ['local', 'list', '/nonexistent/path/junk.spm'],
+            # XXX install failure due to missing deps
+            # XXX install failure due to missing field
+        )
+
+        for args in fail_args:
+            self.ui._error.clear()
+            self.client.run(args)
+            assert len(self.ui._error) > 0
 
 
 if __name__ == '__main__':