mirror of
https://github.com/Stone-Red-Code/dvc.git
synced 2026-11-09 23:10:24 +01:00
[RFC] tests: make read_text() work with directories (#4155)
This PR is just a showcase to see how we like it. I've converted a few
tests to it, but there will be much more to come.
```
assert (tmp_dir / "file") == "contents"
assert (tmp_dir / "dir").read_text() == {"subdir": {"file": "hello"}}
```
This approach is nicer because it allows us to use a declaratory
asserts instead of relying on fs files. The problem becomes much more
apparent once you get into remote workspaces, where we `trees_equal`
doesn't work.
This commit is contained in:
@@ -221,6 +221,17 @@ class TmpDir(pathlib.Path):
|
||||
finally:
|
||||
self.scm.checkout(old)
|
||||
|
||||
def read_text(self, *args, **kwargs): # pylint: disable=signature-differs
|
||||
# NOTE: on windows we'll get PermissionError instead of
|
||||
# IsADirectoryError when we try to `open` a directory, so we can't
|
||||
# rely on exception flow control
|
||||
if self.is_dir():
|
||||
return {
|
||||
path.name: path.read_text(*args, **kwargs)
|
||||
for path in self.iterdir()
|
||||
}
|
||||
return super().read_text(*args, **kwargs)
|
||||
|
||||
|
||||
def _coerce_filenames(filenames):
|
||||
if isinstance(filenames, (str, bytes, pathlib.PurePath)):
|
||||
|
||||
@@ -10,7 +10,6 @@ from dvc.repo import Repo
|
||||
from dvc.repo.get import GetDVCFileError
|
||||
from dvc.system import System
|
||||
from dvc.utils.fs import makedirs
|
||||
from tests.utils import trees_equal
|
||||
|
||||
|
||||
def test_get_repo_file(tmp_dir, erepo_dir):
|
||||
@@ -29,8 +28,7 @@ def test_get_repo_dir(tmp_dir, erepo_dir):
|
||||
|
||||
Repo.get(os.fspath(erepo_dir), "dir", "dir_imported")
|
||||
|
||||
assert os.path.isdir("dir_imported")
|
||||
trees_equal(erepo_dir / "dir", "dir_imported")
|
||||
assert (tmp_dir / "dir_imported").read_text() == {"file": "contents"}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -44,7 +42,6 @@ def test_get_git_file(tmp_dir, erepo):
|
||||
|
||||
Repo.get(os.fspath(erepo), src, dst)
|
||||
|
||||
assert (tmp_dir / dst).is_file()
|
||||
assert (tmp_dir / dst).read_text() == "hello"
|
||||
|
||||
|
||||
@@ -61,8 +58,7 @@ def test_get_git_dir(tmp_dir, erepo):
|
||||
|
||||
Repo.get(os.fspath(erepo), src, dst)
|
||||
|
||||
assert (tmp_dir / dst).is_dir()
|
||||
trees_equal(erepo / src, tmp_dir / dst)
|
||||
assert (tmp_dir / dst).read_text() == {"dir": {"file.txt": "hello"}}
|
||||
|
||||
|
||||
def test_cache_type_is_properly_overridden(tmp_dir, erepo_dir):
|
||||
|
||||
@@ -11,7 +11,6 @@ from dvc.dvcfile import Dvcfile
|
||||
from dvc.exceptions import DownloadError, PathMissingError
|
||||
from dvc.system import System
|
||||
from dvc.utils.fs import makedirs, remove
|
||||
from tests.utils import trees_equal
|
||||
|
||||
|
||||
def test_import(tmp_dir, scm, dvc, erepo_dir):
|
||||
@@ -73,8 +72,7 @@ def test_import_git_dir(tmp_dir, scm, dvc, git_dir, src_is_dvc):
|
||||
|
||||
stage = dvc.imp(os.fspath(git_dir), "src", "dst")
|
||||
|
||||
assert (tmp_dir / "dst").is_dir()
|
||||
trees_equal(git_dir / "src", tmp_dir / "dst")
|
||||
assert (tmp_dir / "dst").read_text() == {"file.txt": "hello"}
|
||||
assert tmp_dir.scm.repo.git.check_ignore(os.fspath(tmp_dir / "dst"))
|
||||
assert stage.deps[0].def_repo == {
|
||||
"url": os.fspath(git_dir),
|
||||
@@ -88,8 +86,7 @@ def test_import_dir(tmp_dir, scm, dvc, erepo_dir):
|
||||
|
||||
stage = dvc.imp(os.fspath(erepo_dir), "dir", "dir_imported")
|
||||
|
||||
assert os.path.isdir("dir_imported")
|
||||
trees_equal(erepo_dir / "dir", "dir_imported")
|
||||
assert (tmp_dir / "dir_imported").read_text() == {"foo": "foo content"}
|
||||
assert scm.repo.git.check_ignore("dir_imported")
|
||||
assert stage.deps[0].def_repo == {
|
||||
"url": os.fspath(erepo_dir),
|
||||
@@ -222,8 +219,7 @@ def test_pull_imported_directory_stage(tmp_dir, dvc, erepo_dir):
|
||||
|
||||
dvc.pull(["dir_imported.dvc"])
|
||||
|
||||
assert os.path.isdir("dir_imported")
|
||||
trees_equal(erepo_dir / "dir", "dir_imported")
|
||||
assert (tmp_dir / "dir_imported").read_text() == {"foo": "foo content"}
|
||||
|
||||
|
||||
def test_download_error_pulling_imported_stage(tmp_dir, dvc, erepo_dir):
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import os
|
||||
from contextlib import contextmanager
|
||||
from filecmp import dircmp
|
||||
|
||||
from dvc.scm import Git
|
||||
|
||||
@@ -20,15 +19,5 @@ def cd(newdir):
|
||||
os.chdir(prevdir)
|
||||
|
||||
|
||||
def trees_equal(dir_path_1, dir_path_2):
|
||||
|
||||
comparison = dircmp(dir_path_1, dir_path_2)
|
||||
|
||||
assert set(comparison.left_only) == set(comparison.right_only) == set()
|
||||
|
||||
for d in comparison.common_dirs:
|
||||
trees_equal(os.path.join(dir_path_1, d), os.path.join(dir_path_2, d))
|
||||
|
||||
|
||||
def to_posixpath(path):
|
||||
return path.replace("\\", "/")
|
||||
|
||||
Reference in New Issue
Block a user