From 2cab8e03be21435e39c5869cd59f0fcaa16fd744 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:00:39 +0300 Subject: [PATCH 01/13] command: remove unused code Signed-off-by: Ruslan Kuprieiev --- dvc/command/add.py | 2 +- dvc/command/{common => }/base.py | 0 dvc/command/checkout.py | 2 +- dvc/command/common/__init__.py | 0 dvc/command/common/common_error.py | 6 ------ dvc/command/config.py | 2 +- dvc/command/data_sync.py | 2 +- dvc/command/destroy.py | 2 +- dvc/command/gc.py | 2 +- dvc/command/imp.py | 2 +- dvc/command/install.py | 2 +- dvc/command/lock.py | 2 +- dvc/command/metrics.py | 2 +- dvc/command/move.py | 2 +- dvc/command/pipeline.py | 2 +- dvc/command/remove.py | 2 +- dvc/command/repro.py | 2 +- dvc/command/root.py | 2 +- dvc/command/run.py | 2 +- dvc/main.py | 2 +- 20 files changed, 17 insertions(+), 23 deletions(-) rename dvc/command/{common => }/base.py (100%) delete mode 100644 dvc/command/common/__init__.py delete mode 100644 dvc/command/common/common_error.py diff --git a/dvc/command/add.py b/dvc/command/add.py index 5fcaa9560..813a497eb 100644 --- a/dvc/command/add.py +++ b/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): diff --git a/dvc/command/common/base.py b/dvc/command/base.py similarity index 100% rename from dvc/command/common/base.py rename to dvc/command/base.py diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index 32f77c178..50e519f9a 100644 --- a/dvc/command/checkout.py +++ b/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): diff --git a/dvc/command/common/__init__.py b/dvc/command/common/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/dvc/command/common/common_error.py b/dvc/command/common/common_error.py deleted file mode 100644 index 3013a73bb..000000000 --- a/dvc/command/common/common_error.py +++ /dev/null @@ -1,6 +0,0 @@ -from dvc.exceptions import DvcException - - -class CmdCommonError(DvcException): - def __init__(self, msg): - super(CmdCommonError, self).__init__('{}'.format(msg)) diff --git a/dvc/command/config.py b/dvc/command/config.py index c49d81f68..731ba5c8a 100644 --- a/dvc/command/config.py +++ b/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 diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 3db3d16d4..f3606630f 100644 --- a/dvc/command/data_sync.py +++ b/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): diff --git a/dvc/command/destroy.py b/dvc/command/destroy.py index 5308239bc..5619f022b 100644 --- a/dvc/command/destroy.py +++ b/dvc/command/destroy.py @@ -1,4 +1,4 @@ -from dvc.command.common.base import CmdBase +from dvc.command.base import CmdBase from dvc.prompt import prompt diff --git a/dvc/command/gc.py b/dvc/command/gc.py index 19c4c0036..2bd9fcaff 100644 --- a/dvc/command/gc.py +++ b/dvc/command/gc.py @@ -1,4 +1,4 @@ -from dvc.command.common.base import CmdBase +from dvc.command.base import CmdBase class CmdGC(CmdBase): diff --git a/dvc/command/imp.py b/dvc/command/imp.py index abe497838..8ed3c0950 100644 --- a/dvc/command/imp.py +++ b/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): diff --git a/dvc/command/install.py b/dvc/command/install.py index d91ee6cd5..55d6341de 100644 --- a/dvc/command/install.py +++ b/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): diff --git a/dvc/command/lock.py b/dvc/command/lock.py index 6e0d5850a..24a1c42cf 100644 --- a/dvc/command/lock.py +++ b/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): diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index 8d3e746f7..7014826c6 100644 --- a/dvc/command/metrics.py +++ b/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 diff --git a/dvc/command/move.py b/dvc/command/move.py index bef31165b..7f2ac6fb7 100644 --- a/dvc/command/move.py +++ b/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): diff --git a/dvc/command/pipeline.py b/dvc/command/pipeline.py index 5a7a7237c..8098d0be4 100644 --- a/dvc/command/pipeline.py +++ b/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): diff --git a/dvc/command/remove.py b/dvc/command/remove.py index e241fe32e..3fa0260c2 100644 --- a/dvc/command/remove.py +++ b/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): diff --git a/dvc/command/repro.py b/dvc/command/repro.py index e6c07658f..433702ff8 100644 --- a/dvc/command/repro.py +++ b/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): diff --git a/dvc/command/root.py b/dvc/command/root.py index 99890da4f..8d83f5c89 100644 --- a/dvc/command/root.py +++ b/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): diff --git a/dvc/command/run.py b/dvc/command/run.py index 95d7a636d..b279e47ce 100644 --- a/dvc/command/run.py +++ b/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 diff --git a/dvc/main.py b/dvc/main.py index 1f708d00f..1ebc3e625 100644 --- a/dvc/main.py +++ b/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): From ebd8468b1c44cd19c3425c7d35e983f67b5c8b58 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:05:28 +0300 Subject: [PATCH 02/13] tests: test CmdBase._set_loglevel Signed-off-by: Ruslan Kuprieiev --- tests/test_logger.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_logger.py b/tests/test_logger.py index 8959bc462..596313245 100644 --- a/tests/test_logger.py +++ b/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) From 9557c56a7eb1c9c16a855ce2aa99b6f2bdb4fdb4 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:10:11 +0300 Subject: [PATCH 03/13] tests: add _find_root test Signed-off-by: Ruslan Kuprieiev --- tests/test_cli.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index c34b6c464..ee438b4e3 100644 --- a/tests/test_cli.py +++ b/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) From ff51b1ae0423ac99b88b751fabc6c67ff0b7aaa6 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:17:27 +0300 Subject: [PATCH 04/13] test: add test for local config Signed-off-by: Ruslan Kuprieiev --- tests/test_config.py | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index d582c354b..c7ae5d432 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,8 +5,13 @@ 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 +27,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 From 0a3a1247563e3ae26e08164d2fba7f1f16ae8c5e Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:23:32 +0300 Subject: [PATCH 05/13] test: cover "Invalid option value" for config Signed-off-by: Ruslan Kuprieiev --- tests/test_config.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index c7ae5d432..a8690a82f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -74,3 +74,9 @@ 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) From 637fc29b795645afd02de92aaeec3cb02e6de391 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:29:39 +0300 Subject: [PATCH 06/13] test: config: add test for failed write Signed-off-by: Ruslan Kuprieiev --- tests/test_config.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index a8690a82f..5f9d9ff02 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,7 @@ import configobj from dvc.main import main +from dvc.command.config import CmdConfig from tests.basic_env import TestDvc @@ -80,3 +81,17 @@ class TestConfigCLI(TestDvc): 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) From bc4a5d78e375172183674092b933f3e60f7a82f3 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:34:21 +0300 Subject: [PATCH 07/13] tests: data cloud: don't forget to test fetch Signed-off-by: Ruslan Kuprieiev --- tests/test_data_cloud.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py index 0db488247..011213693 100644 --- a/tests/test_data_cloud.py +++ b/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: From e734ad1712498ac1e2b18d32fc16863b7afd46bc Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:41:42 +0300 Subject: [PATCH 08/13] destroy: raise exception Signed-off-by: Ruslan Kuprieiev --- dvc/command/destroy.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/command/destroy.py b/dvc/command/destroy.py index 5619f022b..996fb61a7 100644 --- a/dvc/command/destroy.py +++ b/dvc/command/destroy.py @@ -1,5 +1,6 @@ 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: From 94ae130b56bc3d6a4a3210a605d7c6a57f9f517f Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:44:48 +0300 Subject: [PATCH 09/13] dvc: fix exception name Signed-off-by: Ruslan Kuprieiev --- dvc/dependency/local.py | 2 +- dvc/output/local.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/dependency/local.py b/dvc/dependency/local.py index 20705f75d..278e6d9f5 100644 --- a/dvc/dependency/local.py +++ b/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): diff --git a/dvc/output/local.py b/dvc/output/local.py index 05a863521..8f7cbbe9b 100644 --- a/dvc/output/local.py +++ b/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): From 90781900e2bc0f96a56881dfded1f673c169d3f4 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:47:46 +0300 Subject: [PATCH 10/13] tests: test CLI `dvc metrics add` Signed-off-by: Ruslan Kuprieiev --- tests/test_metrics.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index e918002e7..46064ff25 100644 --- a/tests/test_metrics.py +++ b/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') From 41ed1bca94b5821df01b96f3820976a67f5c631a Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:48:41 +0300 Subject: [PATCH 11/13] tests: test `dvc pipeline show --outs` Signed-off-by: Ruslan Kuprieiev --- tests/test_pipeline.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 5ea44e1cc..287f17468 100644 --- a/tests/test_pipeline.py +++ b/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) From 3be898797a8e9370274b7482f91ffee605119460 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 07:59:24 +0300 Subject: [PATCH 12/13] test: remote: add test for failed write Signed-off-by: Ruslan Kuprieiev --- tests/test_remote.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_remote.py b/tests/test_remote.py index cdc74e5c7..d8c5c9219 100644 --- a/tests/test_remote.py +++ b/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) From e598b0b4c7202e325060e675b5e94d7b56e4973e Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 19 Jul 2018 08:03:18 +0300 Subject: [PATCH 13/13] test: repro: add test for failed stage in CLI Signed-off-by: Ruslan Kuprieiev --- tests/test_repro.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_repro.py b/tests/test_repro.py index 8fa6d84d3..d7112c82b 100644 --- a/tests/test_repro.py +++ b/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)