diff --git a/dvc/remote/base.py b/dvc/remote/base.py index f2f0edbb4..011b0af4a 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -312,7 +312,7 @@ class RemoteBASE(object): return checksum if self.isdir(path_info): - checksum = self.get_dir_checksum(path_info / "") + checksum = self.get_dir_checksum(path_info) else: checksum = self.get_file_checksum(path_info) diff --git a/dvc/remote/gs.py b/dvc/remote/gs.py index bfbd51010..f89654461 100644 --- a/dvc/remote/gs.py +++ b/dvc/remote/gs.py @@ -136,18 +136,17 @@ class RemoteGS(RemoteBASE): blob.delete() - def _list_paths(self, bucket, prefix, max_items=None): - for blob in self.gs.bucket(bucket).list_blobs( - prefix=prefix, max_results=max_items + def _list_paths(self, path_info, max_items=None): + for blob in self.gs.bucket(path_info.bucket).list_blobs( + prefix=path_info.path, max_results=max_items ): yield blob.name def list_cache_paths(self): - for cache in self.walk_files(self.path_info): - yield cache.path + return self._list_paths(self.path_info) def walk_files(self, path_info): - for fname in self._list_paths(path_info.bucket, path_info.path): + for fname in self._list_paths(path_info): # skip nested empty directories if fname.endswith("/"): continue @@ -160,17 +159,11 @@ class RemoteGS(RemoteBASE): def isdir(self, path_info): dir_path = path_info / "" - return bool( - list( - self._list_paths(path_info.bucket, dir_path.path, max_items=1) - ) - ) + return bool(list(self._list_paths(dir_path, max_items=1))) def exists(self, path_info): dir_path = path_info / "" - fname = next( - self._list_paths(path_info.bucket, path_info.path, max_items=1), "" - ) + fname = next(self._list_paths(path_info, max_items=1), "") return path_info.path == fname or fname.startswith(dir_path.path) def _upload(self, from_file, to_info, name=None, no_progress_bar=True): diff --git a/tests/func/remotes.py b/tests/func/remotes.py new file mode 100644 index 000000000..cee9cc963 --- /dev/null +++ b/tests/func/remotes.py @@ -0,0 +1,138 @@ +from .test_data_cloud import _should_test_aws +from .test_data_cloud import _should_test_azure +from .test_data_cloud import _should_test_gcp +from .test_data_cloud import _should_test_hdfs +from .test_data_cloud import _should_test_oss +from .test_data_cloud import _should_test_ssh +from .test_data_cloud import get_aws_url +from .test_data_cloud import get_azure_url +from .test_data_cloud import get_gcp_url +from .test_data_cloud import get_hdfs_url +from .test_data_cloud import get_local_url +from .test_data_cloud import get_oss_url +from .test_data_cloud import get_ssh_url + +from dvc.remote.gs import RemoteGS +from dvc.remote.s3 import RemoteS3 + +from contextlib import contextmanager + +from moto.s3 import mock_s3 + + +# NOTE: staticmethod is only needed in Python 2 +class Local: + should_test = staticmethod(lambda: True) + get_url = staticmethod(get_local_url) + + +class S3: + should_test = staticmethod(_should_test_aws) + get_url = staticmethod(get_aws_url) + + +class S3Mocked: + should_test = staticmethod(lambda: True) + get_url = staticmethod(get_aws_url) + + @classmethod + def remote(cls): + @contextmanager + def inner(): + with mock_s3(): + remote = RemoteS3(None, {"url": cls.get_url()}) + yield remote + + return inner() + + @classmethod + def put_objects(cls, remote, objects): + @contextmanager + def inner(): + s3 = cls.get_client(remote) + s3.create_bucket(Bucket="dvc-test") + for key, body in objects.items(): + cls.put_object(remote, key, body) + yield + + return inner() + + @staticmethod + def get_client(remote): + return remote.s3 + + @classmethod + def put_object(cls, remote, key, body): + s3 = cls.get_client(remote) + bucket = remote.path_info.bucket + + s3.put_object( + Bucket=bucket, Key=remote.path_info.path + "/" + key, Body=body + ) + + +class GCP: + should_test = staticmethod(_should_test_gcp) + get_url = staticmethod(get_gcp_url) + + @classmethod + def remote(cls): + @contextmanager + def inner(): + remote = RemoteGS(None, {"url": cls.get_url()}) + yield remote + + return inner() + + @classmethod + def put_objects(cls, remote, objects): + @contextmanager + def inner(): + for key, body in objects.items(): + cls.put_object(remote, key, body) + yield + cls.remove(remote, objects.keys()) + + return inner() + + @classmethod + def put_object(cls, remote, key, body): + client = cls.get_client(remote) + bucket = remote.path_info.bucket + + bucket = client.get_bucket(bucket) + blob = bucket.blob(remote.path_info.path + "/" + key) + blob.upload_from_string(body) + + @staticmethod + def get_client(remote): + return remote.gs + + @staticmethod + def remove(remote, files): + for fname in files: + remote.remove(remote.path_info / fname) + + +class Azure: + should_test = staticmethod(_should_test_azure) + get_url = staticmethod(get_azure_url) + + +class OSS: + should_test = staticmethod(_should_test_oss) + get_url = staticmethod(get_oss_url) + + +class SSH: + should_test = staticmethod(_should_test_ssh) + get_url = staticmethod(get_ssh_url) + + +class HDFS: + should_test = staticmethod(_should_test_hdfs) + get_url = staticmethod(get_hdfs_url) + + +remote_params = [S3, GCP, Azure, OSS, SSH, HDFS] +all_remote_params = [Local] + remote_params diff --git a/tests/func/test_api.py b/tests/func/test_api.py index 3ba71ea46..a8f293853 100644 --- a/tests/func/test_api.py +++ b/tests/func/test_api.py @@ -3,64 +3,12 @@ import shutil import pytest -from .test_data_cloud import _should_test_aws -from .test_data_cloud import _should_test_azure -from .test_data_cloud import _should_test_gcp -from .test_data_cloud import _should_test_hdfs -from .test_data_cloud import _should_test_oss -from .test_data_cloud import _should_test_ssh -from .test_data_cloud import get_aws_url -from .test_data_cloud import get_azure_url -from .test_data_cloud import get_gcp_url -from .test_data_cloud import get_hdfs_url -from .test_data_cloud import get_local_url -from .test_data_cloud import get_oss_url -from .test_data_cloud import get_ssh_url from dvc import api from dvc.exceptions import FileMissingError from dvc.main import main from dvc.path_info import URLInfo from dvc.remote.config import RemoteConfig - - -# NOTE: staticmethod is only needed in Python 2 -class Local: - should_test = staticmethod(lambda: True) - get_url = staticmethod(get_local_url) - - -class S3: - should_test = staticmethod(_should_test_aws) - get_url = staticmethod(get_aws_url) - - -class GCP: - should_test = staticmethod(_should_test_gcp) - get_url = staticmethod(get_gcp_url) - - -class Azure: - should_test = staticmethod(_should_test_azure) - get_url = staticmethod(get_azure_url) - - -class OSS: - should_test = staticmethod(_should_test_oss) - get_url = staticmethod(get_oss_url) - - -class SSH: - should_test = staticmethod(_should_test_ssh) - get_url = staticmethod(get_ssh_url) - - -class HDFS: - should_test = staticmethod(_should_test_hdfs) - get_url = staticmethod(get_hdfs_url) - - -remote_params = [S3, GCP, Azure, OSS, SSH, HDFS] -all_remote_params = [Local] + remote_params +from tests.func.remotes import remote_params, all_remote_params @pytest.fixture diff --git a/tests/func/test_remote_dir.py b/tests/func/test_remote_dir.py deleted file mode 100644 index b78f80041..000000000 --- a/tests/func/test_remote_dir.py +++ /dev/null @@ -1,168 +0,0 @@ -# -*- coding: utf-8 -*- -import pytest -import uuid - -from moto import mock_s3 - -from dvc.remote.gs import RemoteGS -from dvc.remote.s3 import RemoteS3 - -from tests.func.test_data_cloud import _should_test_gcp - -test_gs = pytest.mark.skipif(not _should_test_gcp(), reason="Skipping on gs.") - - -def create_object_gs(client, bucket, key, body): - bucket = client.get_bucket(bucket) - blob = bucket.blob(key) - blob.upload_from_string(body) - - -@pytest.fixture -def remote_s3(): - """Returns a RemoteS3 connected to a bucket with the following structure: - - bucket - ├── data - │ ├── alice - │ ├── alpha - │ └── subdir - │ ├── 1 - │ ├── 2 - │ └── 3 - ├── empty_dir - ├── empty_file - └── foo - """ - with mock_s3(): - remote = RemoteS3(None, {"url": "s3://bucket", "region": "us-east-1"}) - s3 = remote.s3 - - s3.create_bucket(Bucket="bucket") - s3.put_object(Bucket="bucket", Key="empty_dir/") - s3.put_object(Bucket="bucket", Key="empty_file", Body=b"") - s3.put_object(Bucket="bucket", Key="foo", Body=b"foo") - s3.put_object(Bucket="bucket", Key="data/alice", Body=b"alice") - s3.put_object(Bucket="bucket", Key="data/alpha", Body=b"alpha") - s3.put_object(Bucket="bucket", Key="data/subdir/1", Body=b"1") - s3.put_object(Bucket="bucket", Key="data/subdir/2", Body=b"2") - s3.put_object(Bucket="bucket", Key="data/subdir/3", Body=b"3") - - yield remote - - -@pytest.fixture -def remote_gs(): - """Returns a RemoteGS connected to a bucket with the following structure: - bucket - ├── data - │ ├── alice - │ ├── alpha - │ └── subdir - │ ├── 1 - │ ├── 2 - │ └── 3 - ├── empty_dir - ├── empty_file - └── foo - """ - prefix = str(uuid.uuid4()) - REMOTE_URL = "gs://dvc-test/" + prefix - remote = RemoteGS(None, {"url": REMOTE_URL}) - teardowns = [] - - def put_object(file, content): - create_object_gs(remote.gs, "dvc-test", prefix + "/" + file, content) - teardowns.append(lambda: remote.remove(remote.path_info / file)) - - put_object("empty_dir/", "") - put_object("empty_file", "") - put_object("foo", "foo") - put_object("data/alice", "alice") - put_object("data/alpha", "alpha") - put_object("data/subdir/1", "1") - put_object("data/subdir/2", "2") - put_object("data/subdir/3", "3") - put_object("data/subdir/4/", "") - - yield remote - - for teardown in teardowns: - teardown() - - -remote_parameterized = pytest.mark.parametrize( - "remote_name", [pytest.param("remote_gs", marks=test_gs), "remote_s3"] -) - - -@remote_parameterized -def test_isdir(request, remote_name): - remote = request.getfixturevalue(remote_name) - - test_cases = [ - (True, "data"), - (True, "data/"), - (True, "data/subdir"), - (True, "empty_dir"), - (False, "foo"), - (False, "data/alice"), - (False, "data/al"), - (False, "data/subdir/1"), - ] - - for expected, path in test_cases: - assert remote.isdir(remote.path_info / path) == expected - - -@remote_parameterized -def test_exists(request, remote_name): - remote = request.getfixturevalue(remote_name) - - test_cases = [ - (True, "data"), - (True, "data/"), - (True, "data/subdir"), - (True, "empty_dir"), - (True, "empty_file"), - (True, "foo"), - (True, "data/alice"), - (True, "data/subdir/1"), - (False, "data/al"), - (False, "foo/"), - ] - - for expected, path in test_cases: - assert remote.exists(remote.path_info / path) == expected - - -@remote_parameterized -def test_walk_files(request, remote_name): - remote = request.getfixturevalue(remote_name) - - files = [ - remote.path_info / "data/alice", - remote.path_info / "data/alpha", - remote.path_info / "data/subdir/1", - remote.path_info / "data/subdir/2", - remote.path_info / "data/subdir/3", - ] - - assert list(remote.walk_files(remote.path_info / "data")) == files - - -def test_copy_preserve_etag_across_buckets(remote_s3): - s3 = remote_s3.s3 - s3.create_bucket(Bucket="another") - - another = RemoteS3(None, {"url": "s3://another", "region": "us-east-1"}) - - from_info = remote_s3.path_info / "foo" - to_info = another.path_info / "foo" - - remote_s3.copy(from_info, to_info) - - from_etag = RemoteS3.get_etag(s3, "bucket", "foo") - to_etag = RemoteS3.get_etag(s3, "another", "foo") - - assert from_etag == to_etag diff --git a/tests/unit/remote/test_remote_dir.py b/tests/unit/remote/test_remote_dir.py new file mode 100644 index 000000000..827fcab0e --- /dev/null +++ b/tests/unit/remote/test_remote_dir.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +import pytest + +from dvc.remote.s3 import RemoteS3 + +from tests.func.remotes import GCP, S3Mocked + +remotes = [GCP, S3Mocked] + +FILE_WITH_CONTENTS = { + "empty_dir/": "", + "empty_file": "", + "foo": "foo", + "data/alice": "alice", + "data/alpha": "alpha", + "data/subdir/1": "1", + "data/subdir/2": "2", + "data/subdir/3": "3", + "data/subdir/empty_dir/": "", + "data/subdir/empty_file": "", +} + + +@pytest.fixture +def remote(request): + if not request.param.should_test(): + raise pytest.skip() + with request.param.remote() as remote: + with request.param.put_objects(remote, FILE_WITH_CONTENTS): + yield remote + + +@pytest.mark.parametrize("remote", remotes, indirect=True) +def test_isdir(remote): + test_cases = [ + (True, "data"), + (True, "data/"), + (True, "data/subdir"), + (True, "empty_dir"), + (False, "foo"), + (False, "data/alice"), + (False, "data/al"), + (False, "data/subdir/1"), + ] + + for expected, path in test_cases: + assert remote.isdir(remote.path_info / path) == expected + + +@pytest.mark.parametrize("remote", remotes, indirect=True) +def test_exists(remote): + test_cases = [ + (True, "data"), + (True, "data/"), + (True, "data/subdir"), + (True, "empty_dir"), + (True, "empty_file"), + (True, "foo"), + (True, "data/alice"), + (True, "data/subdir/1"), + (False, "data/al"), + (False, "foo/"), + ] + + for expected, path in test_cases: + assert remote.exists(remote.path_info / path) == expected + + +@pytest.mark.parametrize("remote", remotes, indirect=True) +def test_walk_files(remote): + files = [ + remote.path_info / "data/alice", + remote.path_info / "data/alpha", + remote.path_info / "data/subdir/1", + remote.path_info / "data/subdir/2", + remote.path_info / "data/subdir/3", + remote.path_info / "data/subdir/empty_file", + ] + + assert list(remote.walk_files(remote.path_info / "data")) == files + + +@pytest.mark.parametrize("remote", [S3Mocked], indirect=True) +def test_copy_preserve_etag_across_buckets(remote): + s3 = remote.s3 + s3.create_bucket(Bucket="another") + + another = RemoteS3(None, {"url": "s3://another", "region": "us-east-1"}) + + from_info = remote.path_info / "foo" + to_info = another.path_info / "foo" + + remote.copy(from_info, to_info) + + from_etag = RemoteS3.get_etag(s3, "bucket", "foo") + to_etag = RemoteS3.get_etag(s3, "another", "foo") + + assert from_etag == to_etag