Merge pull request #3073 from skshetry/refactor-remote-s3

test: merge _should_test_aws and _get_aws_url inside S3 test helper

Author: efiop

tests/func/test_data_cloud.py

@@ -31,20 +31,19 @@
 from tests.utils import spy
 
 from tests.remotes import (
-    _should_test_aws,
     _should_test_gcp,
     _should_test_hdfs,
     _should_test_ssh,
     Azure,
     GDrive,
+    S3,
     OSS,
     TEST_CONFIG,
     TEST_SECTION,
     TEST_GCP_CREDS_FILE,
     TEST_GDRIVE_CLIENT_ID,
     TEST_GDRIVE_CLIENT_SECRET,
     TEST_REMOTE,
-    get_aws_url,
     get_gcp_url,
     get_hdfs_url,
     get_local_url,
@@ -195,10 +194,10 @@ def test(self):
 
 class TestRemoteS3(TestDataCloudBase):
     def _should_test(self):
-        return _should_test_aws()
+        return S3.should_test()
 
     def _get_url(self):
-        return get_aws_url()
+        return S3.get_url()
 
     def _get_cloud_class(self):
         return RemoteS3
@@ -469,10 +468,10 @@ def _test(self):
 
 class TestRemoteS3CLI(TestDataCloudCLIBase):
     def _should_test(self):
-        return _should_test_aws()
+        return S3.should_test()
 
     def _test(self):
-        url = get_aws_url()
+        url = S3.get_url()
 
         self.main(["remote", "add", TEST_REMOTE, url])
 

tests/func/test_repro.py

@@ -34,11 +34,11 @@
 from dvc.utils.stage import load_stage_file
 from tests.basic_env import TestDvc
 from tests.remotes import (
-    _should_test_aws,
     _should_test_gcp,
     _should_test_hdfs,
     _should_test_ssh,
     get_ssh_url,
+    S3,
     TEST_AWS_REPO_BUCKET,
     TEST_GCP_REPO_BUCKET,
 )
@@ -954,7 +954,7 @@ def test(self, mock_prompt):
 @pytest.mark.skipif(os.name == "nt", reason="temporarily disabled on windows")
 class TestReproExternalS3(TestReproExternalBase):
     def should_test(self):
-        return _should_test_aws()
+        return S3.should_test()
 
     @property
     def scheme(self):

tests/func/test_s3.py

@@ -5,7 +5,7 @@
 from moto import mock_s3
 
 from dvc.remote.s3 import RemoteS3
-from tests.remotes import get_aws_url
+from tests.remotes import S3
 
 
 # from https://github.com/spulec/moto/blob/v1.3.5/tests/test_s3/test_s3.py#L40
@@ -30,7 +30,7 @@ def wrapped(*args, **kwargs):
 
 
 def _get_src_dst():
-    base_info = RemoteS3.path_cls(get_aws_url())
+    base_info = RemoteS3.path_cls(S3.get_url())
     return base_info / "from", base_info / "to"
 
 

tests/remotes.py

@@ -43,17 +43,6 @@
 TEST_GDRIVE_CLIENT_SECRET = "2fy_HyzSwkxkGzEken7hThXb"
 
 
-def _should_test_aws():
-    do_test = env2bool("DVC_TEST_AWS", undefined=None)
-    if do_test is not None:
-        return do_test
-
-    if os.getenv("AWS_ACCESS_KEY_ID") and os.getenv("AWS_SECRET_ACCESS_KEY"):
-        return True
-
-    return False
-
-
 def _should_test_gcp():
     do_test = env2bool("DVC_TEST_GCP", undefined=None)
     if do_test is not None:
@@ -156,14 +145,6 @@ def get_hdfs_url():
     )
 
 
-def get_aws_storagepath():
-    return TEST_AWS_REPO_BUCKET + "/" + str(uuid.uuid4())
-
-
-def get_aws_url():
-    return "s3://" + get_aws_storagepath()
-
-
 def get_gcp_storagepath():
     return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4())
 
@@ -179,13 +160,30 @@ class Local:
 
 
 class S3:
-    should_test = staticmethod(_should_test_aws)
-    get_url = staticmethod(get_aws_url)
+    @staticmethod
+    def should_test():
+        do_test = env2bool("DVC_TEST_AWS", undefined=None)
+        if do_test is not None:
+            return do_test
+
+        if os.getenv("AWS_ACCESS_KEY_ID") and os.getenv(
+            "AWS_SECRET_ACCESS_KEY"
+        ):
+            return True
+
+        return False
+
+    @staticmethod
+    def get_storagepath():
+        return TEST_AWS_REPO_BUCKET + "/" + str(uuid.uuid4())
+
+    @staticmethod
+    def get_url():
+        return "s3://" + S3.get_storagepath()
 
 
-class S3Mocked:
+class S3Mocked(S3):
     should_test = staticmethod(lambda: True)
-    get_url = staticmethod(get_aws_url)
 
     @classmethod
     @contextmanager