Add RepositoryChange unit test

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

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:

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 = []

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))

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)

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