From b9a17752e3e989aff0ec4312af270ed49754e680 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 31 Oct 2019 00:23:18 +0530 Subject: [PATCH 1/4] Test acceptable path types of `get_mtime_and_size` --- tests/unit/utils/test_fs.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/utils/test_fs.py b/tests/unit/utils/test_fs.py index 97c1ad29ba..5c8849903a 100644 --- a/tests/unit/utils/test_fs.py +++ b/tests/unit/utils/test_fs.py @@ -39,6 +39,15 @@ def test(self): self.assertIs(type(dir_size), str) self.assertEqual(dir_size, str(actual_dir_size)) + def test_path_object_and_str_are_valid_types(self): + dvcignore = DvcIgnoreFilter(self.root_dir) + file_time, file_size = get_mtime_and_size(self.DATA, dvcignore) + file_object_time, file_object_size = get_mtime_and_size( + PathInfo(self.DATA), dvcignore + ) + self.assertEqual(file_time, file_object_time) + self.assertEqual(file_size, file_object_size) + class TestContainsLink(TestCase): def test_should_raise_exception_on_base_path_not_in_path(self): From f4f9f3aac1dd4b4d111fdf8b21f329595bee80d5 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 31 Oct 2019 00:27:09 +0530 Subject: [PATCH 2/4] Replace `path` with `path_info` --- dvc/state.py | 7 +++---- dvc/utils/fs.py | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/dvc/state.py b/dvc/state.py index 61f82549d9..1d8e3623d1 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -370,13 +370,12 @@ def save(self, path_info, checksum): assert path_info.scheme == "local" assert checksum is not None - path = fspath_py35(path_info) - assert os.path.exists(path) + assert os.path.exists(fspath_py35(path_info)) actual_mtime, actual_size = get_mtime_and_size( - path, self.repo.dvcignore + path_info, self.repo.dvcignore ) - actual_inode = get_inode(path) + actual_inode = get_inode(path_info) existing_record = self.get_state_record_for_inode(actual_inode) if not existing_record: diff --git a/dvc/utils/fs.py b/dvc/utils/fs.py index 811b4195a9..edbcec3c71 100644 --- a/dvc/utils/fs.py +++ b/dvc/utils/fs.py @@ -7,7 +7,7 @@ from dvc.exceptions import DvcException from dvc.system import System -from dvc.utils import dict_md5, walk_files +from dvc.utils import dict_md5, walk_files, fspath_py35 from dvc.utils.compat import str @@ -21,12 +21,12 @@ def get_inode(path): def get_mtime_and_size(path, dvcignore): - if os.path.isdir(path): + if os.path.isdir(fspath_py35(path)): size = 0 files_mtimes = {} for file_path in walk_files(path, dvcignore): try: - stat = os.stat(file_path) + stat = os.stat(fspath_py35(file_path)) except OSError as exc: # NOTE: broken symlink case. if exc.errno != errno.ENOENT: @@ -39,7 +39,7 @@ def get_mtime_and_size(path, dvcignore): # max(mtime(f) for f in non_ignored_files) mtime = dict_md5(files_mtimes) else: - base_stat = os.stat(path) + base_stat = os.stat(fspath_py35(path)) size = base_stat.st_size mtime = base_stat.st_mtime mtime = int(nanotime.timestamp(mtime)) From d454cb8637697e4aa779aa461105217a4efec4c0 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Sun, 3 Nov 2019 02:21:11 +0530 Subject: [PATCH 3/4] Modify test to check for directory as well --- tests/unit/utils/test_fs.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/unit/utils/test_fs.py b/tests/unit/utils/test_fs.py index 5c8849903a..d479b10d99 100644 --- a/tests/unit/utils/test_fs.py +++ b/tests/unit/utils/test_fs.py @@ -39,15 +39,6 @@ def test(self): self.assertIs(type(dir_size), str) self.assertEqual(dir_size, str(actual_dir_size)) - def test_path_object_and_str_are_valid_types(self): - dvcignore = DvcIgnoreFilter(self.root_dir) - file_time, file_size = get_mtime_and_size(self.DATA, dvcignore) - file_object_time, file_object_size = get_mtime_and_size( - PathInfo(self.DATA), dvcignore - ) - self.assertEqual(file_time, file_object_time) - self.assertEqual(file_size, file_object_size) - class TestContainsLink(TestCase): def test_should_raise_exception_on_base_path_not_in_path(self): @@ -165,3 +156,14 @@ def test_get_inode(repo_dir): path = repo_dir.FOO path_info = PathInfo(path) assert get_inode(path) == get_inode(path_info) + + +@pytest.mark.parametrize("path", [TestDir.DATA, TestDir.DATA_DIR]) +def test_path_object_and_str_are_valid_types_get_mtime_and_size( + path, repo_dir +): + dvcignore = DvcIgnoreFilter(repo_dir.root_dir) + time, size = get_mtime_and_size(path, dvcignore) + object_time, object_size = get_mtime_and_size(PathInfo(path), dvcignore) + assert time == object_time + assert size == object_size From cffc9c8ed9ca9c97fd18b3c09bfe3dc9eaec7abc Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Sun, 3 Nov 2019 02:38:50 +0530 Subject: [PATCH 4/4] Remove unnecessary wrapping of path --- .gitignore | 1 + dvc/state.py | 1 - dvc/utils/fs.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index d2a206fe3d..33be5cb20c 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ neatlynx/__pycache__ cache *.pyc .env/ +.env2.7/ cache .dvc.conf.lock diff --git a/dvc/state.py b/dvc/state.py index 1d8e3623d1..dacc07693c 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -369,7 +369,6 @@ def save(self, path_info, checksum): """ assert path_info.scheme == "local" assert checksum is not None - assert os.path.exists(fspath_py35(path_info)) actual_mtime, actual_size = get_mtime_and_size( diff --git a/dvc/utils/fs.py b/dvc/utils/fs.py index edbcec3c71..6ded4cefdb 100644 --- a/dvc/utils/fs.py +++ b/dvc/utils/fs.py @@ -26,7 +26,7 @@ def get_mtime_and_size(path, dvcignore): files_mtimes = {} for file_path in walk_files(path, dvcignore): try: - stat = os.stat(fspath_py35(file_path)) + stat = os.stat(file_path) except OSError as exc: # NOTE: broken symlink case. if exc.errno != errno.ENOENT: