From 721ddfa3912dd27d21ae1f6c20174b3bcd974c2e Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sun, 26 Mar 2017 21:43:12 -0700 Subject: [PATCH] Add RepositoryChange unit test --- dvc/executor.py | 2 +- dvc/git_wrapper.py | 40 +++++++++---------- dvc/repository_change.py | 8 +++- tests/test_dvc_path.py | 7 +++- tests/test_executor.py | 2 +- tests/test_repository_change.py | 68 +++++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 25 deletions(-) create mode 100644 tests/test_repository_change.py diff --git a/dvc/executor.py b/dvc/executor.py index 100a65c1f..911d20069 100644 --- a/dvc/executor.py +++ b/dvc/executor.py @@ -1,4 +1,3 @@ -import sys import subprocess from dvc.exceptions import DvcException @@ -20,6 +19,7 @@ class Executor: cwd=cwd, stdout=stdout, stderr=stderr) + p.wait() out, err = map(lambda s: s.decode().strip('\n\r') if s else '', p.communicate()) return p.returncode, out, err diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 6a20d95db..aa6f80d00 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -33,13 +33,32 @@ class GitWrapperI(object): def is_ready_to_go(self): return True + @staticmethod + def git_file_statuses(): + code, out, err = Executor.exec_cmd(['git', 'status', '--porcelain']) + if code != 0: + raise ExecutorError('Git command error - {}'.format(err)) + + return GitWrapper.parse_porcelain_files(out) + + @staticmethod + def parse_porcelain_files(out): + result = [] + if len(out) > 0: + lines = out.split('\n') + for line in lines: + status = line[:2] + file = line[3:] + result.append((status, file)) + return result + class GitWrapper(GitWrapperI): def __init__(self): super(GitWrapper, self).__init__() def is_ready_to_go(self): - statuses = self.status_files() + statuses = self.git_file_statuses() if len(statuses) > 0: Logger.error('Commit all changed files before running reproducible command.') Logger.error('Changed files:') @@ -72,25 +91,6 @@ class GitWrapper(GitWrapperI): raise ExecutorError('Unable to run git command: {}'.format(e)) pass - @staticmethod - def status_files(): - code, out, err = Executor.exec_cmd(['git', 'status', '--porcelain']) - if code != 0: - raise ExecutorError('Git command error - {}'.format(err)) - - return GitWrapper.parse_porcelain_files(out) - - @staticmethod - def parse_porcelain_files(out): - result = [] - if len(out) > 0: - lines = out.split('\n') - for line in lines: - status = line[:2] - file = line[3:] - result.append((status, file)) - return result - @property def curr_commit(self): if self._commit is None: diff --git a/dvc/repository_change.py b/dvc/repository_change.py index 393d3e15c..7287fa922 100644 --- a/dvc/repository_change.py +++ b/dvc/repository_change.py @@ -40,6 +40,9 @@ class FileModificationState(object): def is_unusual(self): return self.is_new or self.is_modified or self.is_removed + def __repr__(self): + return u'({}, {})'.format(self.state, self.file) + class RepositoryChange(object): """Pre-condition: git repository has no changes""" @@ -83,12 +86,13 @@ class RepositoryChange(object): return self.get_filenames(filter(lambda x: not x.is_unusual, self._file_states)) def _get_file_states(self): - statuses = GitWrapper.status_files() + statuses = GitWrapper.git_file_statuses() result = [] for status, file in statuses: file_path = os.path.join(self.git.git_dir_abs, file) - if os.path.isfile(file_path): + + if not os.path.isdir(file_path): result.append(FileModificationState(status, file_path)) else: files = [] diff --git a/tests/test_dvc_path.py b/tests/test_dvc_path.py index 0fe3b7bb8..760eaa0b8 100644 --- a/tests/test_dvc_path.py +++ b/tests/test_dvc_path.py @@ -8,14 +8,19 @@ from dvc.git_wrapper import GitWrapperI class TestDvcPathTest(TestCase): def setUp(self): + self.curr_dir = os.path.realpath('.') self.test_dir = os.path.realpath('/tmp/ntx_unit_test/dvc_path') - shutil.rmtree(self.test_dir, ignore_errors=True) + self.tearDown() os.makedirs(os.path.join(self.test_dir, 'data')) os.makedirs(os.path.join(self.test_dir, 'code', 'lib')) os.makedirs(os.path.join(self.test_dir, 'd1', 'd2', 'dir3', 'd4', 'dir5')) self._git = GitWrapperI(self.test_dir) + def tearDown(self): + os.chdir(self.curr_dir) + shutil.rmtree(self.test_dir, ignore_errors=True) + def _validate_dvc_path(self, path, dvc_file_name, relative_file_name): self.assertEqual(path.dvc, dvc_file_name) self.assertEqual(path.filename, os.path.basename(dvc_file_name)) diff --git a/tests/test_executor.py b/tests/test_executor.py index 5248a6183..2754f9376 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -32,7 +32,7 @@ class TestExecutor(TestCase): output = Executor.exec_cmd_only_success(['ls', self.test_dir], '-', '-') self.assertEqual(output, '') - def test_custom_files_output(self): + def test_custom_file_outputs(self): stdout_file = os.path.join(self.test_dir, 'stdout.txt') stderr_file = os.path.join(self.test_dir, 'stderr.txt') output = Executor.exec_cmd_only_success(['ls', self.test_dir], stdout_file, stderr_file) diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py new file mode 100644 index 000000000..a5dd7769e --- /dev/null +++ b/tests/test_repository_change.py @@ -0,0 +1,68 @@ +import os +import shutil +import subprocess +from unittest import TestCase + +from dvc.config import ConfigI +from dvc.path.factory import PathFactory +from dvc.git_wrapper import GitWrapperI +from dvc.repository_change import RepositoryChange + + +class TestRepositoryChange(TestCase): + def setUp(self): + self.test_dir = os.path.realpath(os.path.join('/', 'tmp', 'dvc_unit_test_repo_change')) + + self.tearDown() + os.mkdir(self.test_dir) + os.chdir(self.test_dir) + + devnull = open(os.devnull, 'w') + subprocess.Popen(['git', 'init'], stdout=devnull, stderr=None).wait() + + self.file_to_modify = 'file1.txt' + self.file_created_before_run = 'file2.txt' + self.file_created_by_run = 'out.txt' + self.file_to_remove = 'file3.txt' + + self.create_file(self.file_to_modify) + self.create_file(self.file_created_before_run) + self.create_file(self.file_to_remove) + + subprocess.Popen(['git', 'add', self.file_to_modify, self.file_to_remove], + stdout=devnull, stderr=None).wait() + subprocess.Popen(['git', 'commit', '-m', '"Adding one file3"'], + stdout=devnull, stderr=None).wait() + os.remove(self.file_to_remove) + + self.git = GitWrapperI(self.test_dir) + self.config = ConfigI('data', 'cache', 'state') + self.path_factory = PathFactory(self.git, self.config) + + def tearDown(self): + shutil.rmtree(self.test_dir, ignore_errors=True) + pass + + @staticmethod + def create_file(file2): + fd = open(file2, 'w+') + fd.write('random text') + fd.close() + + def test_df(self): + change = RepositoryChange(['ls', '-la'], + self.file_created_by_run, + self.file_to_modify, + self.git, + self.config, + self.path_factory) + + expected_new = [self.file_created_by_run, self.file_created_before_run] + self.assertEqual(set(change.new_files), set(map(os.path.realpath, expected_new))) + + self.assertEqual(change.removed_files, [os.path.realpath(self.file_to_remove)]) + self.assertEqual(change.modified_files, [os.path.realpath(self.file_to_modify)]) + + expected_to_change = expected_new + [self.file_to_modify] + self.assertEqual(set(change.changed_files), set(map(os.path.realpath, expected_to_change))) + pass