From c3c2116220fdaa54b1812d464e07fad3104eee69 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Wed, 5 Apr 2017 20:41:35 -0700 Subject: [PATCH] Close #1 - repro cmd and close #2 repro unit tese --- bin/dvc-data-remove | 2 +- dvc/command/base.py | 28 ++++++--- dvc/command/data_import.py | 12 ++-- dvc/command/data_sync.py | 22 +++---- dvc/command/init.py | 12 ++-- dvc/command/remove.py | 46 +++++++++------ dvc/command/repro.py | 14 ++--- dvc/command/run.py | 20 +++---- dvc/executor.py | 6 +- dvc/git_wrapper.py | 46 ++++++++++----- tests/test_cmd_data_remove.py | 52 ++++++++--------- tests/test_cmd_run.py | 7 +-- tests/test_repro.py | 106 ++++++++++------------------------ 13 files changed, 182 insertions(+), 191 deletions(-) diff --git a/bin/dvc-data-remove b/bin/dvc-data-remove index 2536f9b4b..52db1467b 100755 --- a/bin/dvc-data-remove +++ b/bin/dvc-data-remove @@ -1,3 +1,3 @@ #!/bin/bash -PYTHONPATH=$DVC_HOME python $DVC_HOME/dvc/command/data_remove.py $@ +PYTHONPATH=$DVC_HOME python $DVC_HOME/dvc/command/remove.py $@ diff --git a/dvc/command/base.py b/dvc/command/base.py index 179b3b858..2f6773798 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -1,5 +1,6 @@ import argparse import os +import sys from dvc.git_wrapper import GitWrapper from dvc.config import Config, ConfigError @@ -24,12 +25,17 @@ class CmdBase(object): if parse_config: self._config = Config(os.path.realpath(os.path.join(self.git.git_dir, self.CONFIG))) - self._args = None + if args is None: + self._args = sys.argv[1:] + else: + self._args = args + + self._parsed_args = None self._dvc_home = None parser = argparse.ArgumentParser() self.define_args(parser) - self._args, self._args_unkn = parser.parse_known_args(args=args) + self._parsed_args, self._command_args = parser.parse_known_args(args=self._args) self._dvc_home = os.environ.get('DVC_HOME') @@ -39,6 +45,18 @@ class CmdBase(object): raise ConfigError("DVC_HOME directory doesn't exists") pass + @property + def args(self): + return self._args + + @property + def parsed_args(self): + return self._parsed_args + + @property + def command_args(self): + return self._command_args + @cached_property def path_factory(self): return PathFactory(self.git, self.config) @@ -54,10 +72,6 @@ class CmdBase(object): def lnx_home(self): return self._dvc_home - @property - def args(self): - return self._args - @property def git(self): return self._git @@ -71,7 +85,7 @@ class CmdBase(object): @property def skip_git_actions(self): - return self.args.skip_git_actions + return self.parsed_args.skip_git_actions def commit_if_needed(self, message, error=False): if error or self.skip_git_actions: diff --git a/dvc/command/data_import.py b/dvc/command/data_import.py index 228def2d4..f72b49401 100644 --- a/dvc/command/data_import.py +++ b/dvc/command/data_import.py @@ -40,18 +40,18 @@ class CmdDataImport(CmdBase): lock = fasteners.InterProcessLock(self.git.lock_file) gotten = lock.acquire(timeout=5) if not gotten: - Logger.printing('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') + Logger.info('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') return 1 try: if not self.skip_git_actions and not self.git.is_ready_to_go(): return 1 - output = self.args.output - for file in self.args.input: - self.import_file(file, output, self.args.is_reproducible) + output = self.parsed_args.output + for file in self.parsed_args.input: + self.import_file(file, output, self.parsed_args.is_reproducible) - message = 'DVC data import: {} {}'.format(' '.join(self.args.input), self.args.output) + message = 'DVC data import: {} {}'.format(' '.join(self.parsed_args.input), self.parsed_args.output) return self.commit_if_needed(message) finally: lock.release() @@ -95,7 +95,7 @@ class CmdDataImport(CmdBase): state_file = StateFile(data_item.state.relative, self.git, - [input], + [], [output], [], is_reproducible) diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 8c7009e6e..41f0f4f38 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -44,7 +44,7 @@ class CmdDataSync(CmdBase): self._bucket = conn.lookup(bucket_name) if not self._bucket: self._bucket = conn.create_bucket(bucket_name) - Logger.printing('S3 bucket "{}" was created'.format(bucket_name)) + Logger.info('S3 bucket "{}" was created'.format(bucket_name)) pass def define_args(self, parser): @@ -52,14 +52,14 @@ class CmdDataSync(CmdBase): pass def run(self): - if os.path.islink(self.args.target): - data_item = self.path_factory.existing_data_item(self.args.target) + if os.path.islink(self.parsed_args.target): + data_item = self.path_factory.existing_data_item(self.parsed_args.target) return self.sync_symlink(data_item) - if os.path.isdir(self.args.target): - return self.sync_dir(self.args.target) + if os.path.isdir(self.parsed_args.target): + return self.sync_dir(self.parsed_args.target) - raise DataSyncError('File "{}" does not exit'.format(target)) + raise DataSyncError('File "{}" does not exit'.format(self.parsed_args.target)) def sync_dir(self, dir): for f in os.listdir(dir): @@ -85,10 +85,10 @@ class CmdDataSync(CmdBase): if not key: raise DataSyncError('File "{}" does not exist in the cloud'.format(aws_key)) - Logger.printing('Downloading cache file from S3 "{}/{}"'.format(self._bucket.name, + Logger.info('Downloading cache file from S3 "{}/{}"'.format(self._bucket.name, aws_key)) key.get_contents_to_filename(item.cache.relative, cb=percent_cb) - Logger.printing('Downloading completed') + Logger.info('Downloading completed') pass def sync_to_cloud(self, data_item): @@ -103,13 +103,13 @@ class CmdDataSync(CmdBase): Logger.debug('File checksum matches. No uploading is needed.') return - Logger.printing('Checksum miss-match. Re-uploading is required.') + Logger.debug('Checksum miss-match. Re-uploading is required.') - Logger.printing('Uploading cache file "{}" to S3 "{}"'.format(data_item.cache.relative, + Logger.info('Uploading cache file "{}" to S3 "{}"'.format(data_item.cache.relative, aws_key)) key = self._bucket.new_key(aws_key) key.set_contents_from_filename(data_item.cache.relative, cb=percent_cb) - Logger.printing('Uploading completed') + Logger.info('Uploading completed') pass diff --git a/dvc/command/init.py b/dvc/command/init.py index f6e72eae9..893f09352 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -69,16 +69,16 @@ SecurityGroup = dvc-group''' return file_name def run(self): - data_dir_path = self.get_not_existing_dir(self.args.data_dir) - cache_dir_path = self.get_not_existing_dir(self.args.cache_dir) - state_dir_path = self.get_not_existing_dir(self.args.state_dir) + data_dir_path = self.get_not_existing_dir(self.parsed_args.data_dir) + cache_dir_path = self.get_not_existing_dir(self.parsed_args.cache_dir) + state_dir_path = self.get_not_existing_dir(self.parsed_args.state_dir) conf_file_name = self.get_not_existing_conf_file_name() data_dir_path.mkdir() cache_dir_path.mkdir() state_dir_path.mkdir() - Logger.printing('Directories {}, {} and {} were created'.format( + Logger.info('Directories {}, {} and {} were created'.format( data_dir_path.name, cache_dir_path.name, state_dir_path.name)) @@ -102,12 +102,12 @@ SecurityGroup = dvc-group''' gitignore_file = os.path.join(self.git.git_dir, '.gitignore') if not os.path.exists(gitignore_file): open(gitignore_file, 'a').close() - Logger.printing('File .gitignore was created') + Logger.info('File .gitignore was created') with open(gitignore_file, 'a') as fd: fd.write('\n{}'.format(cache_dir_name)) fd.write('\n{}'.format(os.path.basename(self.git.lock_file))) - Logger.printing('Directory {} was added to .gitignore file'.format(cache_dir_name)) + Logger.info('Directory {} was added to .gitignore file'.format(cache_dir_name)) if __name__ == '__main__': run(CmdInit()) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index f72876422..1908ac49b 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -34,7 +34,7 @@ class CmdDataRemove(CmdBase): lock = fasteners.InterProcessLock(self.git.lock_file) gotten = lock.acquire(timeout=5) if not gotten: - Logger.info('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') + Logger.info('[Cmd-Remove] Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') return 1 try: @@ -47,11 +47,11 @@ class CmdDataRemove(CmdBase): return 1 error = False - for target in self.args.target: + for target in self.parsed_args.target: if not self.remove_target(target): error = True - message = 'DVC data remove: {}'.format(' '.join(self.args.target)) + message = 'DVC data remove: {}'.format(' '.join(self.parsed_args.target)) self.commit_if_needed(message, error) return 0 if error == 0 else 1 @@ -61,19 +61,19 @@ class CmdDataRemove(CmdBase): if os.path.isdir(target): self.remove_dir(target) else: - self.remove_data_instance(target) + self.remove_file(target) return True except DvcException as ex: - Logger.error('Unable to remove data file "{}": {}'.format(target, ex)) + Logger.error('[Cmd-Remove] Unable to remove data file "{}": {}'.format(target, ex)) return False def remove_dir(self, target): - if not self.args.recursive: - raise DataRemoveError('Directory "%s" cannot be removed. Use --recurcive flag.' % target) + if not self.parsed_args.recursive: + raise DataRemoveError('[Cmd-Remove] Directory "%s" cannot be removed. Use --recurcive flag.' % target) data_item = self.path_factory.data_item(target) if data_item.data_dvc_short == '': - raise DataRemoveError('Data directory "%s" cannot be removed' % target) + raise DataRemoveError('[Cmd-Remove] Data directory "%s" cannot be removed' % target) return self.remove_dir_file_by_file(target) @@ -81,11 +81,14 @@ class CmdDataRemove(CmdBase): def remove_dir_if_empty(file): dir = os.path.dirname(file) if dir != '' and not os.listdir(dir): + Logger.debug(u'[Cmd-Remove] Empty directory was removed {}.'.format(dir)) os.rmdir(dir) pass - def remove_data_instance(self, target): + def remove_file(self, target): # it raises exception if not a symlink is provided + Logger.debug(u'[Cmd-Remove] Remove file {}.'.format(target)) + data_item = self.path_factory.existing_data_item(target) self._remove_cache_file(data_item) @@ -93,40 +96,49 @@ class CmdDataRemove(CmdBase): self._remove_cloud_cache(data_item) os.remove(data_item.data.relative) + Logger.debug(u'[Cmd-Remove] Remove data item {}. Success.'.format(data_item.data.relative)) pass def _remove_cloud_cache(self, data_item): - if not self.args.keep_in_cloud: + if not self.parsed_args.keep_in_cloud: aws_key = self.cache_file_aws_key(data_item.cache.dvc) self.remove_from_cloud(aws_key) def _remove_state_file(self, data_item): if os.path.isfile(data_item.state.relative): + Logger.debug(u'[Cmd-Remove] Remove state {}.'.format(data_item.state.relative)) os.remove(data_item.state.relative) self.remove_dir_if_empty(data_item.state.relative) + Logger.debug(u'[Cmd-Remove] Remove state. Success.') else: - Logger.warn(u'State file {} for data instance {} does not exist'.format( + Logger.warn(u'[Cmd-Remove] State file {} for data instance {} does not exist'.format( data_item.state.relative, data_item.data.relative)) def _remove_cache_file(self, data_item): - if not self.args.keep_in_cache and os.path.isfile(data_item.cache.relative): + if not self.parsed_args.keep_in_cache and os.path.isfile(data_item.cache.relative): + Logger.debug(u'[Cmd-Remove] Remove cache {}.'.format(data_item.cache.relative)) + os.remove(data_item.cache.relative) self.remove_dir_if_empty(data_item.cache.relative) + Logger.debug(u'[Cmd-Remove] Remove cache. Success.') else: - if not self.args.keep_in_cache: - Logger.warn(u'Unable to find cache file for data instance %s' % data_item.data.relative) + if not self.parsed_args.keep_in_cache: + Logger.warn(u'[Cmd-Remove] Unable to find cache file for data item %s' % data_item.data.relative) + pass def remove_from_cloud(self, aws_file_name): + Logger.debug(u'[Cmd-Remove] Remove from cloud {}.'.format(aws_file_name)) + conn = S3Connection(self.config.aws_access_key_id, self.config.aws_secret_access_key) bucket_name = self.config.aws_storage_bucket bucket = conn.lookup(bucket_name) if bucket: key = bucket.get_key(aws_file_name) if not key: - Logger.warn('S3 remove warning: file "{}" does not exist in S3'.format(aws_file_name)) + Logger.warn('[Cmd-Remove] S3 remove warning: file "{}" does not exist in S3'.format(aws_file_name)) else: key.delete() - Logger.info('File "{}" was removed from S3'.format(aws_file_name)) + Logger.info('[Cmd-Remove] File "{}" was removed from S3'.format(aws_file_name)) pass def remove_dir_file_by_file(self, target): @@ -135,7 +147,7 @@ class CmdDataRemove(CmdBase): if os.path.isdir(file): self.remove_dir_file_by_file(file) else: - self.remove_data_instance(file) + self.remove_file(file) os.rmdir(target) pass diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 91e5ac6a3..1034aecc9 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -1,5 +1,4 @@ import os - import fasteners from dvc.command.base import CmdBase @@ -57,14 +56,14 @@ class CmdRepro(CmdRun): if not self.skip_git_actions and not self.git.is_ready_to_go(): return 1 - data_item_list, external_files_names = self.path_factory.to_data_items(self.args.target) + data_item_list, external_files_names = self.path_factory.to_data_items(self.parsed_args.target) if external_files_names: Logger.error('Files from outside of the data directory "{}" could not be reproduced: {}'. format(self.config.data_dir, ' '.join(external_files_names))) return 1 if self.repro_data_items(data_item_list): - return self.commit_if_needed('DVC repro: {}'.format(' '.join(self.args.target))) + return self.commit_if_needed('DVC repro: {}'.format(' '.join(self.parsed_args.target))) finally: lock.release() @@ -107,16 +106,16 @@ 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 + cmd_obj._code = self.state.code_dependencies - argv = self._state.norm_argv + argv = self.state.norm_argv if not argv: raise ReproError('Error: parameter {} is nor defined in state file "{}"'. format(StateFile.PARAM_NORM_ARGV, data_item.state.relative)) if len(argv) < 2: raise ReproError('Error: reproducible cmd in state file "{}" is too short'. - format(self._state.file)) + format(self.state.file)) self._repro_argv = argv pass @@ -160,7 +159,8 @@ class ReproChange(object): were_input_files_changed = True was_source_code_changed = self.git.were_files_changed(self._data_item.data.relative, - self.state.code_dependencies) + self.state.code_dependencies + self.state.input_files, + self.cmd_obj.path_factory) if was_source_code_changed: Logger.debug('Source code was changed for "{}"'.format(self._data_item.data.dvc)) diff --git a/dvc/command/run.py b/dvc/command/run.py index acce034fb..e1cfe095f 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -1,6 +1,4 @@ import os -import sys - import fasteners from dvc.command.base import CmdBase @@ -43,19 +41,19 @@ class CmdRun(CmdBase): @property def is_reproducible(self): - return not self.args.not_repro + return not self.parsed_args.not_repro @property def code_dependencies(self): - return self.args.code or [] + return self.parsed_args.code or [] @cached_property def declaration_input_data_items(self): - return self._data_items_from_params(self.args.input, 'Input') + return self._data_items_from_params(self.parsed_args.input, 'Input') @cached_property def declaration_output_data_items(self): - return self._data_items_from_params(self.args.output, 'Output') + return self._data_items_from_params(self.parsed_args.output, 'Output') def run(self): lock = fasteners.InterProcessLock(self.git.lock_file) @@ -68,11 +66,11 @@ class CmdRun(CmdBase): if not self.skip_git_actions and not self.git.is_ready_to_go(): return 1 - 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))) + self.run_command(self.command_args, + self.data_items_from_args(self.command_args), + self.parsed_args.stdout, + self.parsed_args.stderr) + return self.commit_if_needed('DVC run: {}'.format(' '.join(self.args))) finally: lock.release() diff --git a/dvc/executor.py b/dvc/executor.py index 911d20069..d48257a75 100644 --- a/dvc/executor.py +++ b/dvc/executor.py @@ -49,9 +49,5 @@ class Executor: def exec_cmd_only_success(cmd, stdout_file=None, stderr_file=None, cwd=None): code, out, err = Executor.exec_cmd(cmd, stdout_file=stdout_file, stderr_file=stderr_file, cwd=cwd) if code != 0: - raise Executor._build_executor_error(cmd, err) + raise ExecutorError('Git command error ({}):\n{}'.format(' '.join(cmd), err)) return out - - @staticmethod - def _build_executor_error(cmd, out): - return ExecutorError('Git command error ({}):\n{}'.format(' '.join(cmd), out)) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 56d335bbf..5293dae58 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -35,9 +35,11 @@ class GitWrapperI(object): @staticmethod def git_file_statuses(): + Logger.debug('[Git] Getting file statuses. Command: git status --porcelain') code, out, err = Executor.exec_cmd(['git', 'status', '--porcelain']) if code != 0: - raise ExecutorError('Git command error - {}'.format(err)) + raise ExecutorError('[Git] File status command error - {}'.format(err)) + Logger.debug('[Git] Getting file statuses. Success.') return GitWrapper.parse_porcelain_files(out) @@ -67,8 +69,8 @@ class GitWrapper(GitWrapperI): def is_ready_to_go(self): statuses = self.git_file_statuses() if len(statuses) > 0: - Logger.error('Commit all changed files before running reproducible command.') - Logger.error('Changed files:') + Logger.error('[Git] Commit all changed files before running reproducible command.') + Logger.error('[Git] Changed files:') for status, file in statuses: Logger.error("{} {}".format(status, file)) return False @@ -85,10 +87,12 @@ class GitWrapper(GitWrapperI): return self._git_dir try: + Logger.debug('[Git] Getting git directory. Command: git rev-parse --show-toplevel') code, out, err = Executor.exec_cmd(['git', 'rev-parse', '--show-toplevel']) if code != 0: - raise ExecutorError('Git command error - {}'.format(err)) + raise ExecutorError('[Git] Git directory command error - {}'.format(err)) + Logger.debug('[Git] Getting git directory. Success.') self._git_dir = out return self._git_dir @@ -100,25 +104,32 @@ class GitWrapper(GitWrapperI): @property def curr_commit(self): - if self._commit is None: - code, out, err = Executor.exec_cmd(['git', 'rev-parse', '--short', 'HEAD']) - if code != 0: - raise ExecutorError('Git command error - {}'.format(err)) - self._commit = out - return self._commit + Logger.debug('[Git] Getting current git commit. Command: git rev-parse --short HEAD') + + code, out, err = Executor.exec_cmd(['git', 'rev-parse', '--short', 'HEAD']) + if code != 0: + raise ExecutorError('[Git] Commit command error - {}'.format(err)) + Logger.debug('[Git] Getting current git commit. Success.') + return out @staticmethod def commit_all_changes(message): + Logger.debug('[Git] Commit all changes. Commands: {} && {} && {}'.format( + 'git add --all', 'git status --porcelain', 'git commit -m')) + Executor.exec_cmd_only_success(['git', 'add', '--all']) out_status = Executor.exec_cmd_only_success(['git', 'status', '--porcelain']) Executor.exec_cmd_only_success(['git', 'commit', '-m', message]) + Logger.debug('[Git] Commit all changes. Success.') + return GitWrapper.parse_porcelain_files(out_status) def commit_all_changes_and_log_status(self, message): statuses = self.commit_all_changes(message) - Logger.printing('A new commit {} was made in the current branch. Added files:'.format(self.curr_commit)) + Logger.debug('[Git] A new commit {} was made in the current branch. Added files:'.format( + self.curr_commit)) for status, file in statuses: - Logger.printing('\t{} {}'.format(status, file)) + Logger.debug('[Git]\t{} {}'.format(status, file)) pass @staticmethod @@ -139,18 +150,21 @@ class GitWrapper(GitWrapperI): return results - def were_files_changed(self, file, code_dependencies): + def were_files_changed(self, file, code_dependencies, path_factory): + Logger.debug('[Git] Identify changes. Commands: {} {} && {}'.format( + 'git log -1 --pretty=format:"%h"', file, 'git diff --name-only HEAD *')) + commit = Executor.exec_cmd_only_success(['git', 'log', '-1', '--pretty=format:"%h"', file]) commit = commit.strip('"') + Logger.debug('[Git] Identify changes. Success.') + changed_files = Executor.exec_cmd_only_success(['git', 'diff', '--name-only', 'HEAD', commit]) changed_files = changed_files.strip('"') code_files, code_dirs = self.separate_dependency_files_and_dirs(code_dependencies) - - code_files_set = set(code_files) + code_files_set = set([path_factory.path(x).dvc for x in code_files]) for changed_file in changed_files.split('\n'): - changed_file = os.path.realpath(changed_file) if changed_file in code_files_set: return True diff --git a/tests/test_cmd_data_remove.py b/tests/test_cmd_data_remove.py index d17460e2b..ca55e94cf 100644 --- a/tests/test_cmd_data_remove.py +++ b/tests/test_cmd_data_remove.py @@ -11,7 +11,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): def test_file_by_file_removal(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True dir1_dvc_name = os.path.join('data', self.dir1) self.assertTrue(os.path.exists(dir1_dvc_name)) @@ -21,8 +21,8 @@ class TestCmdDataRemove(DirHierarchyEnvironment): def test_remove_deep_dir(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True - cmd.args.recursive = True + cmd.parsed_args.keep_in_cloud = True + cmd.parsed_args.recursive = True dir1_dvc_name = os.path.join('data', self.dir1) self.assertTrue(os.path.exists(dir1_dvc_name)) @@ -32,8 +32,8 @@ class TestCmdDataRemove(DirHierarchyEnvironment): def test_not_recursive_removal(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True - cmd.args.recursive = False + cmd.parsed_args.keep_in_cloud = True + cmd.parsed_args.recursive = False dir1_dvc_name = os.path.join('data', self.dir1) self.assertTrue(os.path.exists(dir1_dvc_name)) @@ -44,8 +44,8 @@ class TestCmdDataRemove(DirHierarchyEnvironment): def test_data_dir_removal(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True - cmd.args.recursive = True + cmd.parsed_args.keep_in_cloud = True + cmd.parsed_args.recursive = True data_dir = 'data' self.assertTrue(os.path.exists(data_dir)) @@ -61,41 +61,41 @@ class TestRemoveDataItem(DirHierarchyEnvironment): def test_remove_data_instance(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True self.assertTrue(os.path.isfile(self.file1)) self.assertTrue(os.path.isfile(self.cache1)) self.assertTrue(os.path.isfile(self.state1)) - cmd.remove_data_instance(self.file1) + cmd.remove_file(self.file1) self.assertFalse(os.path.exists(self.file1)) self.assertFalse(os.path.exists(self.cache1)) self.assertFalse(os.path.exists(self.state1)) def test_keep_cache(self): 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 + cmd.parsed_args.keep_in_cloud = True + cmd.parsed_args.keep_in_cache = True self.assertTrue(os.path.isfile(self.file1)) self.assertTrue(os.path.isfile(self.cache1)) self.assertTrue(os.path.isfile(self.state1)) - cmd.remove_data_instance(self.file1) + cmd.remove_file(self.file1) self.assertFalse(os.path.exists(self.file1)) self.assertTrue(os.path.exists(self.cache1)) self.assertFalse(os.path.exists(self.state1)) def test_remove_data_instance_without_cache(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True self.assertTrue(os.path.isfile(self.file6)) self.assertIsNone(self.cache6) self.assertTrue(os.path.isfile(self.state6)) with self.assertRaises(DataItemError): - cmd.remove_data_instance(self.file6) + cmd.remove_file(self.file6) class TestRemoveEndToEnd(DirHierarchyEnvironment): @@ -104,16 +104,16 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): def test_end_to_end_with_an_error(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) dir2_full = os.path.join('data', self.dir2) file6_full = os.path.join('data', self.file6) - cmd.args.target = [dir11_full, self.file5, file6_full] - cmd.args.recursive = True + cmd.parsed_args.target = [dir11_full, self.file5, file6_full] + cmd.parsed_args.recursive = True - cmd.args.skip_git_actions = True + cmd.parsed_args.skip_git_actions = True self.assertTrue(os.path.exists(dir11_full)) self.assertTrue(os.path.exists(self.file5)) @@ -131,15 +131,15 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): def test_end_to_end_with_no_error(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) dir2_full = os.path.join('data', self.dir2) - cmd.args.target = [dir11_full, self.file5] - cmd.args.recursive = True + cmd.parsed_args.target = [dir11_full, self.file5] + cmd.parsed_args.recursive = True - cmd.args.skip_git_actions = True + cmd.parsed_args.skip_git_actions = True self.assertTrue(os.path.exists(dir11_full)) self.assertTrue(os.path.exists(self.file5)) @@ -155,15 +155,15 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): def test_run(self): cmd = CmdDataRemove(parse_config=False, git_obj=self._git, config_obj=self._config) - cmd.args.keep_in_cloud = True + cmd.parsed_args.keep_in_cloud = True dir11_full = os.path.join('data', self.dir11) dir2_full = os.path.join('data', self.dir2) - cmd.args.target = [dir11_full, self.file5] - cmd.args.recursive = True + cmd.parsed_args.target = [dir11_full, self.file5] + cmd.parsed_args.recursive = True - cmd.args.skip_git_actions = True + cmd.parsed_args.skip_git_actions = True self.assertTrue(os.path.exists(dir11_full)) self.assertTrue(os.path.exists(self.file5)) diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 6c1df6b20..b183279e5 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -1,6 +1,5 @@ import os import shutil -import subprocess import tempfile from unittest import TestCase @@ -9,7 +8,7 @@ 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.git_wrapper import GitWrapper class RunBasicTest(TestCase): @@ -62,10 +61,10 @@ class RunTwoFilesBase(RunBasicTest): 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] + cmd_run.parsed_args.input = [self.input_param_file] self.extra_output_file = 'data/extra_output' - cmd_run.args.output = [self.extra_output_file] + cmd_run.parsed_args.output = [self.extra_output_file] self.file_name1 = 'data/file1' self.file_name2 = 'data/file2' diff --git a/tests/test_repro.py b/tests/test_repro.py index dd63447ce..d9c1a1965 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -1,62 +1,10 @@ -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() @@ -130,7 +78,6 @@ class ReproBasicEnv(RunBasicTest): class ReproCodeDependencyTest(ReproBasicEnv): - def test(self): self.modify_file_and_commit(self.file_res_code_file) @@ -142,24 +89,35 @@ class ReproCodeDependencyTest(ReproBasicEnv): 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') +class ReproChangedDependency(ReproBasicEnv): + def test(self): + self.recreate_file1() + + CmdRepro(args=[self.file_name11], + config_obj=self.config, + git_obj=self.git + ).run() + + self.assertEqual(open(self.file_name11).read(), 'Goodbye\n') + + def recreate_file1(self): + CmdDataRemove(args=[self.file_name1, '--keep-in-cloud'], # ??? + config_obj=self.config, + git_obj=self.git + ).run() + CmdRun(args=['printf', 'Goodbye\nJack', '--stdout', self.file_name1], + config_obj=self.config, + git_obj=self.git + ).run() + + +class ReproChangedDeepDependency(ReproChangedDependency): + def test(self): + self.recreate_file1() + + 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')