From d55427dbd9630969bf47ab278642354fe0931f51 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Mon, 13 Jan 2020 16:30:15 +0545 Subject: [PATCH 1/2] test: refactor hdfs related functions in HDFS test helpers --- tests/func/test_data_cloud.py | 11 +++---- tests/func/test_repro.py | 4 +-- tests/remotes.py | 61 +++++++++++++++++------------------ 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 10fee7da3..b0fb40abd 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -32,9 +32,9 @@ from tests.utils import spy from tests.remotes import ( _should_test_gcp, - _should_test_hdfs, Azure, GDrive, + HDFS, S3, SSHMocked, OSS, @@ -45,7 +45,6 @@ from tests.remotes import ( TEST_GDRIVE_CLIENT_SECRET, TEST_REMOTE, get_gcp_url, - get_hdfs_url, get_local_url, ) @@ -332,10 +331,10 @@ class TestRemoteSSHMocked(TestDataCloudBase): class TestRemoteHDFS(TestDataCloudBase): def _should_test(self): - return _should_test_hdfs() + return HDFS.should_test() def _get_url(self): - return get_hdfs_url() + return HDFS.get_url() def _get_cloud_class(self): return RemoteHDFS @@ -431,10 +430,10 @@ class TestRemoteLOCALCLI(TestDataCloudCLIBase): class TestRemoteHDFSCLI(TestDataCloudCLIBase): def _should_test(self): - return _should_test_hdfs() + return HDFS.should_test() def _test(self): - url = get_hdfs_url() + url = HDFS.get_url() self.main(["remote", "add", TEST_REMOTE, url]) diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 4a8435c5b..4c3f6a1b5 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -36,7 +36,7 @@ from dvc.utils.stage import load_stage_file from tests.basic_env import TestDvc from tests.remotes import ( _should_test_gcp, - _should_test_hdfs, + HDFS, S3, SSH, SSHMocked, @@ -996,7 +996,7 @@ class TestReproExternalGS(TestReproExternalBase): class TestReproExternalHDFS(TestReproExternalBase): def should_test(self): - return _should_test_hdfs() + return HDFS.should_test() @property def scheme(self): diff --git a/tests/remotes.py b/tests/remotes.py index d4c61ebc6..daa206a17 100644 --- a/tests/remotes.py +++ b/tests/remotes.py @@ -66,29 +66,6 @@ def _should_test_gcp(): return True -def _should_test_hdfs(): - if platform.system() != "Linux": - return False - - try: - check_output( - ["hadoop", "version"], shell=True, executable=os.getenv("SHELL") - ) - except (CalledProcessError, IOError): - return False - - p = Popen( - "hadoop fs -ls hdfs://127.0.0.1/", - shell=True, - executable=os.getenv("SHELL"), - ) - p.communicate() - if p.returncode != 0: - return False - - return True - - def get_local_storagepath(): return TestDvc.mkdtemp() @@ -97,12 +74,6 @@ def get_local_url(): return get_local_storagepath() -def get_hdfs_url(): - return "hdfs://{}@127.0.0.1{}".format( - getpass.getuser(), get_local_storagepath() - ) - - def get_gcp_storagepath(): return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4()) @@ -277,5 +248,33 @@ class SSHMocked: class HDFS: - should_test = _should_test_hdfs - get_url = get_hdfs_url + @staticmethod + def should_test(): + if platform.system() != "Linux": + return False + + try: + check_output( + ["hadoop", "version"], + shell=True, + executable=os.getenv("SHELL"), + ) + except (CalledProcessError, IOError): + return False + + p = Popen( + "hadoop fs -ls hdfs://127.0.0.1/", + shell=True, + executable=os.getenv("SHELL"), + ) + p.communicate() + if p.returncode != 0: + return False + + return True + + @staticmethod + def get_url(): + return "hdfs://{}@127.0.0.1{}".format( + getpass.getuser(), get_local_storagepath() + ) From 26d85f1a528a4ccc4dc51640e7019ab06ead4b15 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Mon, 13 Jan 2020 23:49:22 +0545 Subject: [PATCH 2/2] test: merge gcp related functions to GCP helper class --- tests/func/test_data_cloud.py | 11 +++--- tests/func/test_repro.py | 9 +++-- tests/remotes.py | 63 +++++++++++++++++------------------ 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index b0fb40abd..7b2e349c8 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -31,8 +31,8 @@ from tests.basic_env import TestDvc from tests.utils import spy from tests.remotes import ( - _should_test_gcp, Azure, + GCP, GDrive, HDFS, S3, @@ -44,7 +44,6 @@ from tests.remotes import ( TEST_GDRIVE_CLIENT_ID, TEST_GDRIVE_CLIENT_SECRET, TEST_REMOTE, - get_gcp_url, get_local_url, ) @@ -231,7 +230,7 @@ class TestRemoteGDrive(TestDataCloudBase): class TestRemoteGS(TestDataCloudBase): def _should_test(self): - return _should_test_gcp() + return GCP.should_test() def _setup_cloud(self): self._ensure_should_run() @@ -249,7 +248,7 @@ class TestRemoteGS(TestDataCloudBase): self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class()) def _get_url(self): - return get_gcp_url() + return GCP.get_url() def _get_cloud_class(self): return RemoteGS @@ -484,10 +483,10 @@ class TestRemoteGDriveCLI(TestDataCloudCLIBase): class TestRemoteGSCLI(TestDataCloudCLIBase): def _should_test(self): - return _should_test_gcp() + return GCP.should_test() def _test(self): - url = get_gcp_url() + url = GCP.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self.main( diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 4c3f6a1b5..3e3be2349 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -9,6 +9,7 @@ from pathlib import Path from subprocess import PIPE from subprocess import Popen from urllib.parse import urljoin +from unittest import SkipTest import boto3 import paramiko @@ -35,7 +36,7 @@ from dvc.utils.stage import dump_stage_file from dvc.utils.stage import load_stage_file from tests.basic_env import TestDvc from tests.remotes import ( - _should_test_gcp, + GCP, HDFS, S3, SSH, @@ -857,7 +858,9 @@ class TestReproExternalBase(TestDvc): @patch("dvc.prompt.confirm", return_value=True) def test(self, mock_prompt): if not self.should_test(): - return + raise SkipTest( + "Test {} is disabled".format(self.__class__.__name__) + ) cache = ( self.scheme @@ -975,7 +978,7 @@ class TestReproExternalS3(TestReproExternalBase): class TestReproExternalGS(TestReproExternalBase): def should_test(self): - return _should_test_gcp() + return GCP.should_test() @property def scheme(self): diff --git a/tests/remotes.py b/tests/remotes.py index daa206a17..4623c7606 100644 --- a/tests/remotes.py +++ b/tests/remotes.py @@ -43,29 +43,6 @@ TEST_GDRIVE_CLIENT_ID = ( TEST_GDRIVE_CLIENT_SECRET = "2fy_HyzSwkxkGzEken7hThXb" -def _should_test_gcp(): - do_test = env2bool("DVC_TEST_GCP", undefined=None) - if do_test is not None: - return do_test - - if not os.path.exists(TEST_GCP_CREDS_FILE): - return False - - try: - check_output( - [ - "gcloud", - "auth", - "activate-service-account", - "--key-file", - TEST_GCP_CREDS_FILE, - ] - ) - except (CalledProcessError, OSError): - return False - return True - - def get_local_storagepath(): return TestDvc.mkdtemp() @@ -74,14 +51,6 @@ def get_local_url(): return get_local_storagepath() -def get_gcp_storagepath(): - return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4()) - - -def get_gcp_url(): - return "gs://" + get_gcp_storagepath() - - class Local: should_test = lambda: True # noqa: E731 get_url = get_local_url @@ -131,8 +100,36 @@ class S3Mocked(S3): class GCP: - should_test = _should_test_gcp - get_url = get_gcp_url + @staticmethod + def should_test(): + do_test = env2bool("DVC_TEST_GCP", undefined=None) + if do_test is not None: + return do_test + + if not os.path.exists(TEST_GCP_CREDS_FILE): + return False + + try: + check_output( + [ + "gcloud", + "auth", + "activate-service-account", + "--key-file", + TEST_GCP_CREDS_FILE, + ] + ) + except (CalledProcessError, OSError): + return False + return True + + @staticmethod + def get_storagepath(): + return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4()) + + @staticmethod + def get_url(): + return "gs://" + GCP.get_storagepath() @classmethod @contextmanager