From dccc15c0d636bd2361254a3f232d3b5ffca20772 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 3 Apr 2017 03:56:08 -0700 Subject: [PATCH] Repro cmd first unttest --- dvc/command/base.py | 4 +- dvc/command/remove.py | 6 +- dvc/command/repro.py | 113 +++++++++++++++------------- dvc/command/run.py | 17 ++++- tests/test_cmd_data_remove.py | 20 ++--- tests/test_cmd_run.py | 31 ++++---- tests/test_repro.py | 134 ++++++++++++++++++++++++++++++++++ 7 files changed, 242 insertions(+), 83 deletions(-) diff --git a/dvc/command/base.py b/dvc/command/base.py index 14f3b8fc6..eb66dc57c 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -11,7 +11,7 @@ from dvc.utils import cached_property class CmdBase(object): CONFIG = 'dvc.conf' - def __init__(self, parse_config=True, git_obj=None, config_obj=None): + def __init__(self, args=None, parse_config=True, git_obj=None, config_obj=None): if git_obj: self._git = git_obj else: @@ -29,7 +29,7 @@ class CmdBase(object): parser = argparse.ArgumentParser() self.define_args(parser) - self._args, self._args_unkn = parser.parse_known_args() + self._args, self._args_unkn = parser.parse_known_args(args=args) self._dvc_home = os.environ.get('DVC_HOME') diff --git a/dvc/command/remove.py b/dvc/command/remove.py index cb29341dc..f72876422 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -15,8 +15,8 @@ class DataRemoveError(DvcException): class CmdDataRemove(CmdBase): - def __init__(self, parse_config=True, git_obj=None, config_obj=None): - super(CmdDataRemove, self).__init__(parse_config, git_obj, config_obj) + def __init__(self, args=None, parse_config=True, git_obj=None, config_obj=None): + super(CmdDataRemove, self).__init__(args, parse_config, git_obj, config_obj) pass def define_args(self, parser): @@ -24,7 +24,7 @@ class CmdDataRemove(CmdBase): parser.add_argument('target', metavar='', help='Target to remove - file or directory', nargs='*') parser.add_argument('-r', '--recursive', action='store_true', help='Remove directory recursively') - parser.add_argument('-l', '--keep-in-cloud', action='store_false', default=False, + parser.add_argument('-l', '--keep-in-cloud', action='store_true', default=False, help='Do not remove data from cloud') parser.add_argument('-c', '--keep-in-cache', action='store_false', default=False, help='Do not remove data from cache') diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 99175d4f8..04fa3692d 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -17,10 +17,10 @@ class ReproError(DvcException): class CmdRepro(CmdRun): - def __init__(self): - CmdBase.__init__(self) + def __init__(self, args=None,parse_config=True, git_obj=None, config_obj=None): + CmdBase.__init__(self, args, parse_config, git_obj, config_obj) - self._code =[] + self._code = [] pass @property @@ -30,11 +30,13 @@ class CmdRepro(CmdRun): def define_args(self, parser): self.set_skip_git_actions(parser) - # self.add_string_arg(parser, 'target', 'Reproduce data file') parser.add_argument('target', metavar='', help='Data file to reproduce', nargs='*') pass - # Overridden methods: + + @property + def is_reproducible(self): + return True @property def declaration_input_data_items(self): @@ -61,37 +63,41 @@ class CmdRepro(CmdRun): format(self.config.data_dir, ' '.join(external_files_names))) return 1 - error = False - changed = False - for data_item in data_item_list: - try: - repro_change = ReproChange(data_item, self) - if repro_change.reproduce(): - changed = True - Logger.info(u'Data file "{}" was reproduced.'.format( - data_item.data.relative - )) - else: - Logger.info(u'Reproduction is not required for data file "{}".'.format( - data_item.data.relative - )) - except ReproError as err: - Logger.error('Error in reproducing data file {}: {}'.format( - data_item.data.relative, str(err) - )) - error = True - break - - if error and not self.skip_git_actions: - Logger.error('Errors occurred. One or more repro cmd was not successful.') - self.not_committed_changes_warning() - return 1 - - if changed: - self.commit_if_needed('DVC repro: {}'.format(' '.join(self.args.target))) + if self.repro_data_items(data_item_list): + return self.commit_if_needed('DVC repro: {}'.format(' '.join(self.args.target))) finally: lock.release() - return 0 + + return 1 + + def repro_data_items(self, data_item_list): + error = False + changed = False + + for data_item in data_item_list: + try: + repro_change = ReproChange(data_item, self) + if repro_change.reproduce(): + changed = True + Logger.info(u'Data file "{}" was reproduced.'.format( + data_item.data.relative + )) + else: + Logger.info(u'Reproduction is not required for data file "{}".'.format( + data_item.data.relative + )) + except ReproError as err: + Logger.error('Error in reproducing data file {}: {}'.format( + data_item.data.relative, str(err) + )) + error = True + break + + if error and not self.skip_git_actions: + Logger.error('Errors occurred. One or more repro cmd was not successful.') + self.not_committed_changes_warning() + + return changed and not error class ReproChange(object): @@ -101,7 +107,7 @@ class ReproChange(object): self._cmd_obj = cmd_obj self._state = StateFile.load(data_item.state.relative, self.git) - cmd_obj._code = self._state.code_dependencies # HACK!!! + cmd_obj._code = self._state.code_dependencies argv = self._state.norm_argv @@ -112,16 +118,9 @@ class ReproChange(object): raise ReproError('Error: reproducible cmd in state file "{}" is too short'. format(self._state.file)) - # if argv[0][-3:] != '.py': - # raise ReproError('Error: reproducible cmd format error in state file "{}"'. - # format(self._state.file)) - self._repro_argv = argv pass - def were_direct_dependencies_changed(self): - return True - def reproduce_data_file(self): Logger.debug('Reproducing data file "{}". Removing the file...'.format( self._data_item.data.relative)) @@ -134,21 +133,16 @@ class ReproChange(object): return self._cmd_obj.run_command(self._repro_argv, data_items_from_args) def reproduce(self, force=False): + print('+++++++++++++++ reproduce', self._data_item.data.dvc) were_input_files_changed = False if not self._state.is_reproducible: Logger.debug('Data file "{}" is not reproducible'.format(self._data_item.data.relative)) return False - for input_file in self._state.input_files: - try: - data_item = self._cmd_obj.path_factory.data_item(input_file) - except NotInDataDirError: - raise ReproError(u'The dependency files "{}" is not a data file'.format(input_file)) - except Exception as ex: - raise ReproError(u'The dependency files "{}" can not be reproduced: {}'.format( - input_file, ex)) + print('+++++++++++++++ reproduce dependencies: '.format(self.get_dependencies())) + for data_item in self.get_dependencies(): change = ReproChange(data_item, self._cmd_obj) if change.reproduce(force): were_input_files_changed = True @@ -163,5 +157,24 @@ class ReproChange(object): return self.reproduce_data_file() + def get_dependencies(self): + + print('+++++++++++++== INPUT FILES: {}'.format(self._state.input_files)) + + dependency_data_items = [] + for input_file in self._state.input_files: + try: + data_item = self._cmd_obj.path_factory.data_item(input_file) + except NotInDataDirError: + raise ReproError(u'The dependency file "{}" is not a data item'.format(input_file)) + except Exception as ex: + raise ReproError(u'Unable to reproduced the dependency file "{}": {}'.format( + input_file, ex)) + + dependency_data_items.append(data_item) + + return dependency_data_items + + if __name__ == '__main__': run(CmdRepro()) diff --git a/dvc/command/run.py b/dvc/command/run.py index 63bed48e1..bce131f5e 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -20,14 +20,17 @@ class RunError(DvcException): class CmdRun(CmdBase): - def __init__(self, parse_config=True, git_obj=None, config_obj=None): - super(CmdRun, self).__init__(parse_config, git_obj, config_obj) + def __init__(self, args=None, parse_config=True, git_obj=None, config_obj=None): + super(CmdRun, self).__init__(args, parse_config, git_obj, config_obj) pass def define_args(self, parser): self.set_skip_git_actions(parser) - parser.add_argument('--random', help='not reproducible, output is random', action='store_true') + parser.add_argument('--not-repro', + help='Not reproducible', + action='store_false', + default=False) parser.add_argument('--stdout', help='output std output to a file') parser.add_argument('--stderr', help='output std error to a file') parser.add_argument('--input', '-i', action='append', @@ -38,6 +41,10 @@ class CmdRun(CmdBase): help='Code dependencies which produce the output') pass + @property + def is_reproducible(self): + return not self.args.not_repro + @property def code_dependencies(self): return self.args.code or [] @@ -93,11 +100,13 @@ class CmdRun(CmdBase): data_item.move_data_to_cache() Logger.debug('Create state file "{}"'.format(data_item.state.relative)) + state_file = StateFile(data_item.state.relative, self.git, input_files_dvc, output_files_dvc, code_dependencies_dvc, - argv=argv) + argv=argv, + is_reproducible=self.is_reproducible) state_file.save() result.append(state_file) diff --git a/tests/test_cmd_data_remove.py b/tests/test_cmd_data_remove.py index 8b99ebe7c..d17460e2b 100644 --- a/tests/test_cmd_data_remove.py +++ b/tests/test_cmd_data_remove.py @@ -10,7 +10,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): DirHierarchyEnvironment.init_environment(self) def test_file_by_file_removal(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True dir1_dvc_name = os.path.join('data', self.dir1) @@ -20,7 +20,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): self.assertFalse(os.path.exists(dir1_dvc_name)) def test_remove_deep_dir(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True cmd.args.recursive = True @@ -31,7 +31,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): self.assertFalse(os.path.exists(dir1_dvc_name)) def test_not_recursive_removal(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True cmd.args.recursive = False @@ -43,7 +43,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): pass def test_data_dir_removal(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True cmd.args.recursive = True @@ -60,7 +60,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment): DirHierarchyEnvironment.init_environment(self) def test_remove_data_instance(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True self.assertTrue(os.path.isfile(self.file1)) @@ -73,7 +73,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment): self.assertFalse(os.path.exists(self.state1)) def test_keep_cache(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True cmd.args.keep_in_cache = True @@ -87,7 +87,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment): self.assertFalse(os.path.exists(self.state1)) def test_remove_data_instance_without_cache(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True self.assertTrue(os.path.isfile(self.file6)) @@ -103,7 +103,7 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): DirHierarchyEnvironment.init_environment(self) def test_end_to_end_with_an_error(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) @@ -130,7 +130,7 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): self.assertTrue(os.path.exists(self.file6)) def test_end_to_end_with_no_error(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) @@ -154,7 +154,7 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): self.assertTrue(os.path.exists(self.file1)) def test_run(self): - cmd = CmdDataRemove(False, git_obj=self._git, config_obj=self._config) + cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) cmd.args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 8a565cf0b..6c1df6b20 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -6,10 +6,10 @@ from unittest import TestCase from dvc.command.run import CmdRun, RunError from dvc.config import ConfigI +from dvc.executor import Executor from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory -from dvc.git_wrapper import GitWrapperI -from dvc.repository_change import RepositoryChange +from dvc.git_wrapper import GitWrapperI, GitWrapper class RunBasicTest(TestCase): @@ -24,32 +24,35 @@ class RunBasicTest(TestCase): os.mkdir('cache') os.mkdir('state') - self._devnull = open(os.devnull, 'w') - subprocess.Popen(['git', 'init'], stdout=self._devnull, stderr=None).wait() + self.init_git_repo() + self.git = GitWrapper() - self.commit = 'abc1234' - self.git = GitWrapperI(self.test_dir, self.commit) self.config = ConfigI('data', 'cache', 'state') self.path_factory = PathFactory(self.git, self.config) pass + def init_git_repo(self): + Executor.exec_cmd_only_success(['git', 'init']) + self.create_file('.gitignore', 'cache\n.dvc.conf.lock') + Executor.exec_cmd_only_success(['git', 'add', '.gitignore']) + Executor.exec_cmd_only_success(['git', 'commit', '-m', '"Init test repo"']) + + @staticmethod + def create_file(file2, content='random text'): + fd = open(file2, 'w+') + fd.write(content) + fd.close() + def tearDown(self): shutil.rmtree(self.test_dir, ignore_errors=True) os.chdir(self._old_curr_dir_abs) - @staticmethod - def create_file(file2): - fd = open(file2, 'w+') - fd.write('random text') - fd.close() - class TestRunOutsideData(RunBasicTest): def test(self): cmd_run = CmdRun(config_obj=self.config) with self.assertRaises(RunError): - state = cmd_run.run_command(['touch', 'file1', 'file2'], []) - next(state) + cmd_run.run_command(['touch', 'file1', 'file2'], []) pass diff --git a/tests/test_repro.py b/tests/test_repro.py index e69de29bb..65986d53a 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -0,0 +1,134 @@ +import os +import shutil +import subprocess +import tempfile +from unittest import TestCase + +import sys + +from dvc.command.remove import CmdDataRemove +from dvc.command.repro import ReproChange, CmdRepro +from dvc.command.run import CmdRun, RunError +from dvc.config import ConfigI +from dvc.executor import Executor +from dvc.path.data_item import DataItem +from dvc.path.factory import PathFactory +from dvc.git_wrapper import GitWrapperI, GitWrapper +from dvc.repository_change import RepositoryChange +from dvc.state_file import StateFile +from tests.test_cmd_run import RunBasicTest + + +# class ReproBasicEnv(TestCase): +# def setUp(self): +# self.test_dir = tempfile.mkdtemp() +# self._old_curr_dir_abs = os.path.realpath(os.curdir) +# +# self.tearDown() +# os.mkdir(self.test_dir) +# os.chdir(self.test_dir) +# os.mkdir('data') +# os.mkdir('cache') +# os.mkdir('state') +# +# self.init_git_repo() +# self.git = GitWrapper() +# +# self.config = ConfigI('data', 'cache', 'state') +# self.path_factory = PathFactory(self.git, self.config) +# pass +# +# def init_git_repo(self): +# Executor.exec_cmd_only_success(['git', 'init']) +# self.create_file('.gitignore', 'cache\n.dvc.conf.lock') +# Executor.exec_cmd_only_success(['git', 'add', '.gitignore']) +# Executor.exec_cmd_only_success(['git', 'commit', '-m', '"Init test repo"']) +# +# def tearDown(self): +# shutil.rmtree(self.test_dir, ignore_errors=True) +# os.chdir(self._old_curr_dir_abs) +# +# @staticmethod +# def create_file(file2, content='random text'): +# fd = open(file2, 'w+') +# fd.write(content) +# fd.close() + + +# class RunEndToEndTest() + +class ReproBasicEnv(RunBasicTest): + def setUp(self): + super(ReproBasicEnv, self).setUp() + self.file_name1 = 'data/file1' + CmdRun(args=['printf', 'Hello\nWorld!', '--not-repro', '--stdout', self.file_name1], + parse_config=False, + config_obj=self.config, + git_obj=self.git + ).run() + + self.file_name11 = 'data/file11' + CmdRun(args=['head', '-n', '1', self.file_name1, '--stdout', self.file_name11], + parse_config=False, + config_obj=self.config, + git_obj=self.git + ).run() + + self.file_name2 = 'data/file2' + CmdRun(args=['printf', 'Bobby', '--not-repro', '--stdout', self.file_name2], + parse_config=False, + config_obj=self.config, + git_obj=self.git + ).run() + + self.file_name_res = 'data/file_res' + CmdRun(args=['cat', self.file_name11, self.file_name2, '--stdout', self.file_name_res], + parse_config=False, + config_obj=self.config, + git_obj=self.git + ).run() + + self.assertEqual(open(self.file_name_res).read(), 'Hello\nBobby') + +# +# class ReproRemovedFileTest(ReproBasicEnv): +# def test(self): +# # CmdDataRemove(args=[self.file_name_res, '--keep-in-cloud'], +# # config_obj=self.config, +# # git_obj=self.git +# # ).run() +# +# # CmdRun(args=['printf', 'Goodbye\nWorld!', '--stdout', self.file_name1], +# # config_obj=self.config, +# # git_obj=self.git +# # ).run() +# +# CmdRepro(args=[self.file_name_res], +# config_obj=self.config, +# git_obj=self.git +# ).run() +# +# self.assertEqual(open(self.file_name_res).read(), 'Hello\nBobby') + + +class ReproChangedDependency(ReproBasicEnv): + def test(self): + + self.assertEqual(open(self.file_name_res).read(), 'Hello\nBobby') + + CmdDataRemove(args=[self.file_name1, '--keep-in-cloud'], + config_obj=self.config, + git_obj=self.git + ).run() + + CmdRun(args=['printf', 'Goodbye\nWorld!', '--stdout', self.file_name1], + config_obj=self.config, + git_obj=self.git + ).run() + + CmdRepro(args=[self.file_name_res], + config_obj=self.config, + git_obj=self.git + ).run() + + # self.assertEqual(open(self.file_name_res).read(), 'Goodbye\nBobby')