Skip to content

Commit a2cc085

Browse files
committed
[RFC] tests: make read_text() work with directories
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.
1 parent 6e66ddf commit a2cc085

File tree

4 files changed

+16
-24
lines changed

4 files changed

+16
-24
lines changed

tests/dir_helpers.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,17 @@ def branch(self, name, new=False):
221221
finally:
222222
self.scm.checkout(old)
223223

224+
def read_text(self, *args, **kwargs): # pylint: disable=signature-differs
225+
# NOTE: on windows we'll get PermissionError instead of
226+
# IsADirectoryError when we try to `open` a directory, so we can't
227+
# rely on exception flow control
228+
if self.is_dir():
229+
return {
230+
path.name: path.read_text(*args, **kwargs)
231+
for path in self.iterdir()
232+
}
233+
return super().read_text(*args, **kwargs)
234+
224235

225236
def _coerce_filenames(filenames):
226237
if isinstance(filenames, (str, bytes, pathlib.PurePath)):

tests/func/test_get.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from dvc.repo.get import GetDVCFileError
1111
from dvc.system import System
1212
from dvc.utils.fs import makedirs
13-
from tests.utils import trees_equal
1413

1514

1615
def test_get_repo_file(tmp_dir, erepo_dir):
@@ -29,8 +28,7 @@ def test_get_repo_dir(tmp_dir, erepo_dir):
2928

3029
Repo.get(os.fspath(erepo_dir), "dir", "dir_imported")
3130

32-
assert os.path.isdir("dir_imported")
33-
trees_equal(erepo_dir / "dir", "dir_imported")
31+
assert (tmp_dir / "dir_imported").read_text() == {"file": "contents"}
3432

3533

3634
@pytest.mark.parametrize(
@@ -44,7 +42,6 @@ def test_get_git_file(tmp_dir, erepo):
4442

4543
Repo.get(os.fspath(erepo), src, dst)
4644

47-
assert (tmp_dir / dst).is_file()
4845
assert (tmp_dir / dst).read_text() == "hello"
4946

5047

@@ -61,8 +58,7 @@ def test_get_git_dir(tmp_dir, erepo):
6158

6259
Repo.get(os.fspath(erepo), src, dst)
6360

64-
assert (tmp_dir / dst).is_dir()
65-
trees_equal(erepo / src, tmp_dir / dst)
61+
assert (tmp_dir / dst).read_text() == {"dir": {"file.txt": "hello"}}
6662

6763

6864
def test_cache_type_is_properly_overridden(tmp_dir, erepo_dir):

tests/func/test_import.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from dvc.exceptions import DownloadError, PathMissingError
1212
from dvc.system import System
1313
from dvc.utils.fs import makedirs, remove
14-
from tests.utils import trees_equal
1514

1615

1716
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):
7372

7473
stage = dvc.imp(os.fspath(git_dir), "src", "dst")
7574

76-
assert (tmp_dir / "dst").is_dir()
77-
trees_equal(git_dir / "src", tmp_dir / "dst")
75+
assert (tmp_dir / "dst").read_text() == {"file.txt": "hello"}
7876
assert tmp_dir.scm.repo.git.check_ignore(os.fspath(tmp_dir / "dst"))
7977
assert stage.deps[0].def_repo == {
8078
"url": os.fspath(git_dir),
@@ -88,8 +86,7 @@ def test_import_dir(tmp_dir, scm, dvc, erepo_dir):
8886

8987
stage = dvc.imp(os.fspath(erepo_dir), "dir", "dir_imported")
9088

91-
assert os.path.isdir("dir_imported")
92-
trees_equal(erepo_dir / "dir", "dir_imported")
89+
assert (tmp_dir / "dir_imported").read_text() == {"foo": "foo content"}
9390
assert scm.repo.git.check_ignore("dir_imported")
9491
assert stage.deps[0].def_repo == {
9592
"url": os.fspath(erepo_dir),
@@ -222,8 +219,7 @@ def test_pull_imported_directory_stage(tmp_dir, dvc, erepo_dir):
222219

223220
dvc.pull(["dir_imported.dvc"])
224221

225-
assert os.path.isdir("dir_imported")
226-
trees_equal(erepo_dir / "dir", "dir_imported")
222+
assert (tmp_dir / "dir_imported").read_text() == {"foo": "foo content"}
227223

228224

229225
def test_download_error_pulling_imported_stage(tmp_dir, dvc, erepo_dir):

tests/utils/__init__.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import os
22
from contextlib import contextmanager
3-
from filecmp import dircmp
43

54
from dvc.scm import Git
65

@@ -20,15 +19,5 @@ def cd(newdir):
2019
os.chdir(prevdir)
2120

2221

23-
def trees_equal(dir_path_1, dir_path_2):
24-
25-
comparison = dircmp(dir_path_1, dir_path_2)
26-
27-
assert set(comparison.left_only) == set(comparison.right_only) == set()
28-
29-
for d in comparison.common_dirs:
30-
trees_equal(os.path.join(dir_path_1, d), os.path.join(dir_path_2, d))
31-
32-
3322
def to_posixpath(path):
3423
return path.replace("\\", "/")

0 commit comments

Comments
 (0)