diff --git a/dvc/command/run.py b/dvc/command/run.py index 6a2f6ac93..63bed48e1 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -20,8 +20,8 @@ class RunError(DvcException): class CmdRun(CmdBase): - def __init__(self): - CmdBase.__init__(self) + def __init__(self, parse_config=True, git_obj=None, config_obj=None): + super(CmdRun, self).__init__(parse_config, git_obj, config_obj) pass def define_args(self, parser): @@ -40,7 +40,7 @@ class CmdRun(CmdBase): @property def code_dependencies(self): - return self.args.code + return self.args.code or [] @cached_property def declaration_input_data_items(self): @@ -48,7 +48,7 @@ class CmdRun(CmdBase): @cached_property def declaration_output_data_items(self): - return self._data_items_from_params(self.args.stdout, 'Output') + return self._data_items_from_params(self.args.output, 'Output') def run(self): lock = fasteners.InterProcessLock(self.git.lock_file) @@ -61,33 +61,32 @@ class CmdRun(CmdBase): if not self.skip_git_actions and not self.git.is_ready_to_go(): return 1 - if not self.run_command(self._args_unkn, - self._data_items_from_args(self._args_unkn), - self.args.stdout, - self.args.stderr): - return 1 - - self.commit_if_needed('DVC run: {}'.format(' '.join(sys.argv))) + self.run_command(self._args_unkn, + self._data_items_from_args(self._args_unkn), + self.args.stdout, + self.args.stderr) + return self.commit_if_needed('DVC run: {}'.format(' '.join(sys.argv))) finally: lock.release() - return 0 + return 1 def run_command(self, argv, data_items_from_args, stdout=None, stderr=None): repo_change = RepositoryChange(argv, stdout, stderr, self.git, self.config, self.path_factory) - if not self.skip_git_actions and not self.validate_file_states(repo_change): + if not self.skip_git_actions and not self._validate_file_states(repo_change): self.remove_new_files(repo_change) - return False + raise RunError('Errors occurred.') output_set = set(self.declaration_output_data_items + repo_change.changed_data_items) - output_files_dvc = map(lambda x: x.data.dvc, output_set) + output_files_dvc = [x.data.dvc for x in output_set] input_set = set(data_items_from_args + self.declaration_input_data_items) - input_files_dvc = map(lambda x: x.data.dvc, input_set) + input_files_dvc = [x.data.dvc for x in input_set] code_dependencies_dvc = self.git.abs_paths_to_dvc(self.code_dependencies) + result = [] for data_item in repo_change.changed_data_items: Logger.debug('Move output file "{}" to cache dir "{}" and create a symlink'.format( data_item.data.relative, data_item.cache.relative)) @@ -100,9 +99,9 @@ class CmdRun(CmdBase): code_dependencies_dvc, argv=argv) state_file.save() - pass + result.append(state_file) - return True + return result @staticmethod def remove_new_files(repo_change): @@ -111,7 +110,8 @@ class CmdRun(CmdBase): os.remove(data_item.data.relative) pass - def validate_file_states(self, repo_change): + @staticmethod + def _validate_file_states(repo_change): error = False for data_item in repo_change.removed_data_items: Logger.error('Error: file "{}" was removed'.format(data_item.data.relative)) @@ -121,13 +121,7 @@ class CmdRun(CmdBase): Logger.error('Error: file "{}" was created outside of the data directory'.format(file)) error = True - if error: - Logger.error('Errors occurred. ' + \ - 'Reproducible commands allow only file creation only in data directory "{}".'. - format(self.config.data_dir)) - return False - - return True + return not error def _data_items_from_args(self, argv): result = [] diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index aa6f80d00..56d335bbf 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -52,6 +52,13 @@ class GitWrapperI(object): result.append((status, file)) return result + def abs_paths_to_dvc(self, files): + result = [] + for file in files: + result.append(os.path.relpath(os.path.abspath(file), self.git_dir_abs)) + + return result + class GitWrapper(GitWrapperI): def __init__(self): @@ -124,13 +131,6 @@ class GitWrapper(GitWrapperI): return result - def abs_paths_to_dvc(self, files): - result = [] - for file in files: - result.append(os.path.relpath(os.path.abspath(file), self.git_dir_abs)) - - return result - def dvc_paths_to_abs(self, files): results = [] diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py new file mode 100644 index 000000000..8a565cf0b --- /dev/null +++ b/tests/test_cmd_run.py @@ -0,0 +1,96 @@ +import os +import shutil +import subprocess +import tempfile +from unittest import TestCase + +from dvc.command.run import CmdRun, RunError +from dvc.config import ConfigI +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 + + +class RunBasicTest(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._devnull = open(os.devnull, 'w') + subprocess.Popen(['git', 'init'], stdout=self._devnull, stderr=None).wait() + + 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 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) + pass + + +class RunTwoFilesBase(RunBasicTest): + def setUp(self): + super(RunTwoFilesBase, self).setUp() + + cmd_run = CmdRun(config_obj=self.config, git_obj=self.git) + self.input_param_file = 'data/extra_input' + cmd_run.args.input = [self.input_param_file] + + self.extra_output_file = 'data/extra_output' + cmd_run.args.output = [self.extra_output_file] + + self.file_name1 = 'data/file1' + self.file_name2 = 'data/file2' + self.state_objs = cmd_run.run_command(['touch', self.file_name1, self.file_name2], []) + + self.state_file_name1 = 'state/file1' + DataItem.STATE_FILE_SUFFIX + self.state_file_name2 = 'state/file2' + DataItem.STATE_FILE_SUFFIX + + self.state_file1 = None + self.state_file2 = None + for s in self.state_objs: + if s.file == self.state_file_name1: + self.state_file1 = s + elif s.file == self.state_file_name2: + self.state_file2 = s + pass + + +class TestRunStateFiles(RunTwoFilesBase): + def test(self): + names = [x.file for x in self.state_objs] + self.assertEqual({self.state_file_name1, self.state_file_name2}, set(names)) + + +class TestRunExtraInExtraOutFiles(RunTwoFilesBase): + def test(self): + self.assertIsNotNone(self.state_file1) + self.assertEqual(self.state_file1.input_files, [self.input_param_file]) + + output_set = {self.file_name1, self.file_name2, self.extra_output_file} + self.assertEqual(set(self.state_file1.output_files), output_set)