Merge pull request #917 from efiop/master

improve test coverage

dvc/command/add.py

@@ -1,5 +1,5 @@
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdAdd(CmdBase):

dvc/command/checkout.py

@@ -1,5 +1,5 @@
 import os
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdCheckout(CmdBase):

dvc/command/common/common_error.py

@@ -1,6 +0,0 @@
-from dvc.exceptions import DvcException
-
-
-class CmdCommonError(DvcException):
-    def __init__(self, msg):
-        super(CmdCommonError, self).__init__('{}'.format(msg))

dvc/command/config.py

@@ -1,7 +1,7 @@
 import os
 import configobj
 
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 from dvc.logger import Logger
 from dvc.config import Config
 

dvc/command/data_sync.py

@@ -1,4 +1,4 @@
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdDataBase(CmdBase):

dvc/command/destroy.py

@@ -1,5 +1,6 @@
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 from dvc.prompt import prompt
+from dvc.exceptions import DvcException
 
 
 class CmdDestroy(CmdBase):
@@ -10,10 +11,9 @@ class CmdDestroy(CmdBase):
                   u'Are you sure you want to continue?'
 
             if not self.args.force and not prompt(msg, False):
-                err = u'Cannot destroy without a confirmation from the user. ' \
+                msg = u'Cannot destroy without a confirmation from the user. ' \
                       u'Use \'-f\' to force.'
-                self.project.logger.error(err)
-                return 1
+                raise DvcException(err)
 
             self.project.destroy()
         except Exception as exc:

dvc/command/gc.py

@@ -1,4 +1,4 @@
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdGC(CmdBase):

dvc/command/imp.py

@@ -1,5 +1,5 @@
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdImport(CmdBase):

dvc/command/install.py

@@ -1,5 +1,5 @@
 from dvc.logger import Logger
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdInstall(CmdBase):

dvc/command/lock.py

@@ -1,5 +1,5 @@
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdLockBase(CmdBase):

dvc/command/metrics.py

@@ -1,4 +1,4 @@
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 from dvc.exceptions import DvcException
 
 

dvc/command/move.py

@@ -1,5 +1,5 @@
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdMove(CmdBase):

dvc/command/pipeline.py

@@ -1,7 +1,7 @@
 import os
 
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdPipelineShow(CmdBase):

dvc/command/remove.py

@@ -1,5 +1,5 @@
 from dvc.exceptions import DvcException
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdRemove(CmdBase):

dvc/command/repro.py

@@ -1,6 +1,6 @@
 import os
 
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdRepro(CmdBase):

dvc/command/root.py

@@ -1,6 +1,6 @@
 import os
 
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 class CmdRoot(CmdBase):

dvc/command/run.py

@@ -1,6 +1,6 @@
 import os
 
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 from dvc.logger import Logger
 from dvc.exceptions import DvcException
 

dvc/dependency/local.py

@@ -59,7 +59,7 @@ class DependencyLOCAL(DependencyBase):
             raise self.DoesNotExistError(self.rel_path)
 
         if not os.path.isfile(self.path) and not os.path.isdir(self.path):
-            raise self.NotFileOrDirError(self.rel_path)
+            raise self.IsNotFileOrDirError(self.rel_path)
 
         if (os.path.isfile(self.path) and os.path.getsize(self.path) == 0) or \
            (os.path.isdir(self.path) and len(os.listdir(self.path)) == 0):

dvc/main.py

@@ -1,6 +1,6 @@
 from dvc.logger import Logger
 from dvc.cli import parse_args
-from dvc.command.common.base import CmdBase
+from dvc.command.base import CmdBase
 
 
 def main(argv=None):

dvc/output/local.py

@@ -82,7 +82,7 @@ class OutputLOCAL(DependencyLOCAL):
             raise self.DoesNotExistError(self.rel_path)
 
         if not os.path.isfile(self.path) and not os.path.isdir(self.path):
-            raise self.NotFileOrDirError(self.rel_path)
+            raise self.IsNotFileOrDirError(self.rel_path)
 
         if (os.path.isfile(self.path) and os.path.getsize(self.path) == 0) or \
            (os.path.isdir(self.path) and len(os.listdir(self.path)) == 0):

tests/test_cli.py

@@ -12,6 +12,8 @@ from dvc.command.remove import CmdRemove
 from dvc.command.gc import CmdGC
 from dvc.command.config import CmdConfig
 from dvc.command.checkout import CmdCheckout
+from dvc.exceptions import DvcException
+from dvc.command.base import CmdBase
 
 from tests.basic_env import TestDvc
 
@@ -161,3 +163,16 @@ class TestStartupTime(TestDvc):
         t = timeit.default_timer() - start
 
         self.assertTrue(t < 0.3)
+
+
+class TestFindRoot(TestDvc):
+    def test(self):
+        os.chdir("..")
+
+        class A(object):
+            quiet = False
+            verbose = True
+
+        args = A()
+        with self.assertRaises(DvcException):
+            CmdBase(args)

tests/test_config.py

@@ -1,12 +1,18 @@
 import configobj
 
 from dvc.main import main
+from dvc.command.config import CmdConfig
 from tests.basic_env import TestDvc
 
 
 class TestConfigCLI(TestDvc):
-    def _contains(self, section, field, value):
-        config = configobj.ConfigObj(self.dvc.config.config_file)
+    def _contains(self, section, field, value, local=False):
+        if local:
+            fname = self.dvc.config.config_local_file
+        else:
+            fname = self.dvc.config.config_file
+
+        config = configobj.ConfigObj(fname)
         if section not in config.keys():
             return False
 
@@ -22,28 +28,38 @@ class TestConfigCLI(TestDvc):
         ret = main(['root'])
         self.assertEqual(ret, 0)
 
-    def test(self):
+    def _do_test(self, local=False):
         section = 'setsection'
         field = 'setfield'
         section_field = '{}.{}'.format(section, field)
         value = 'setvalue'
         newvalue = 'setnewvalue'
 
-        ret = main(['config', section_field, value])
-        self.assertEqual(ret, 0)
-        self.assertTrue(self._contains(section, field, value))
+        base = ['config']
+        if local:
+            base.append('--local')
 
-        ret = main(['config', section_field])
+        ret = main(base + [section_field, value])
+        self.assertEqual(ret, 0)
+        self.assertTrue(self._contains(section, field, value, local))
+
+        ret = main(base + [section_field])
         self.assertEqual(ret, 0)
 
-        ret = main(['config', section_field, newvalue])
+        ret = main(base + [section_field, newvalue])
         self.assertEqual(ret, 0)
-        self.assertTrue(self._contains(section, field, newvalue))
-        self.assertFalse(self._contains(section, field, value))
+        self.assertTrue(self._contains(section, field, newvalue, local))
+        self.assertFalse(self._contains(section, field, value, local))
 
-        ret = main(['config', section_field, '--unset'])
+        ret = main(base + [section_field, '--unset'])
         self.assertEqual(ret, 0)
-        self.assertFalse(self._contains(section, field, value))
+        self.assertFalse(self._contains(section, field, value, local))
+
+    def test(self):
+        self._do_test(False)
+
+    def test_local(self):
+        self._do_test(True)
 
     def test_non_existing(self):
         #FIXME check stdout/err
@@ -59,3 +75,23 @@ class TestConfigCLI(TestDvc):
 
         ret = main(['config', 'global.non_existing_field', '-u'])
         self.assertEqual(ret, 1)
+
+        ret = main(['config', 'core.remote', 'myremote'])
+        self.assertEqual(ret, 0)
+
+        ret = main(['config', 'core.non_existing_field', '-u'])
+        self.assertEqual(ret, 1)
+
+    def test_failed_write(self):
+        class A(object):
+            local = False
+            name = 'core.remote'
+            value = 'myremote'
+            unset = False
+
+        args = A()
+        cmd = CmdConfig(args)
+
+        cmd.configobj.write = None
+        ret = cmd.save()
+        self.assertNotEqual(ret, 0)

tests/test_data_cloud.py

@@ -381,9 +381,17 @@ class TestDataCloudCLIBase(TestDvc):
         sleep()
         self.main(['status', '-c'] + args)
 
+        self.main(['fetch'] + args)
+        self.assertTrue(os.path.exists(cache))
+        self.assertTrue(os.path.isfile(cache))
+        self.assertTrue(os.path.isfile(cache_dir))
+
         self.main(['pull'] + args)
         self.assertTrue(os.path.exists(cache))
         self.assertTrue(os.path.isfile(cache))
+        self.assertTrue(os.path.isfile(cache_dir))
+        self.assertTrue(os.path.isfile(self.FOO))
+        self.assertTrue(os.path.isdir(self.DATA_DIR))
 
         sleep()
         with open(cache, 'r') as fd:

tests/test_logger.py

@@ -1,6 +1,7 @@
 import colorama
 import logging
 
+from dvc.command.base import CmdBase
 from dvc.config import Config
 from dvc.logger import Logger
 
@@ -32,3 +33,19 @@ class TestLogger(TestDvc):
             msg = Logger.colorize(name, name)
             # This is not a tty, so it should not colorize anything
             self.assertEqual(msg, name)
+
+
+class TestLoggerCLI(TestDvc):
+    def test(self):
+        class A(object):
+            quiet = True
+            verbose = False
+
+        args = A()
+        CmdBase._set_loglevel(args)
+        self.assertEqual(Logger.logger().getEffectiveLevel(), logging.CRITICAL)
+
+        args.quiet = False
+        args.verbose = True
+        CmdBase._set_loglevel(args)
+        self.assertEqual(Logger.logger().getEffectiveLevel(), logging.DEBUG)

tests/test_metrics.py

@@ -91,6 +91,12 @@ class TestMetricsReproCLI(TestDvc):
         ret = main(['repro', '-f', '-m', stage.path])
         self.assertNotEqual(ret, 0)
 
+        ret = main(['metrics', 'add', 'metrics'])
+        self.assertEqual(ret, 0)
+
+        ret = main(['repro', '-f', '-m', stage.path])
+        self.assertEqual(ret, 0)
+
     def test_dir(self):
         os.mkdir('metrics_dir')
 

tests/test_pipeline.py

@@ -12,6 +12,10 @@ class TestPipelineShow(TestRepro):
         ret = main(['pipeline', 'show', self.file1_stage, '--commands'])
         self.assertEqual(ret, 0)        
 
+    def test_outs(self):
+        ret = main(['pipeline', 'show', self.file1_stage, '--outs'])
+        self.assertEqual(ret, 0)        
+
     def test_not_dvc_file(self):
         ret = main(['pipeline', 'show', self.file1])
         self.assertNotEqual(ret, 0)

tests/test_remote.py

@@ -1,4 +1,5 @@
 from dvc.main import main
+from dvc.command.remote import CmdRemoteAdd
 
 from tests.basic_env import TestDvc
 
@@ -19,3 +20,17 @@ class TestRemote(TestDvc):
         self.assertEqual(main(['remote', 'modify', remotes[0], 'option', 'value']), 0)
 
         self.assertEqual(main(['remote', 'list']), 0)
+
+    def test_failed_write(self):   
+        class A(object):           
+            local = False          
+            name = 'myremote'   
+            url = 's3://remote'     
+            unset = False          
+                                   
+        args = A()                 
+        cmd = CmdRemoteAdd(args)      
+                                   
+        cmd.configobj.write = None 
+        ret = cmd.run()           
+        self.assertNotEqual(ret, 0)

tests/test_repro.py

@@ -40,6 +40,14 @@ class TestRepro(TestDvc):
                      cmd='python {} {} {}'.format(self.CODE, self.FOO, self.file1))
 
 
+class TestReproFail(TestRepro):
+    def test(self):
+        os.unlink(self.CODE)
+
+        ret = main(['repro', self.file1_stage])
+        self.assertNotEqual(ret, 0)
+
+
 class TestReproDepUnderDir(TestDvc):
     def test(self):
         self.dir_stage = self.dvc.add(self.DATA_DIR)