From 9724bd94aacc0de949540ff463b25bb017c2998c Mon Sep 17 00:00:00 2001
From: Clemens Wolff <clewolff@microsoft.com>
Date: Tue, 10 Jul 2018 15:19:16 -0400
Subject: [PATCH] Replace os.system with subprocess.check_output

It is generally recommended to use the subprocess module instead of the
os module to run subshell commands given since the former makes it
easier to comunicate with the process, has an improved security model,
etc. More information is available in [1].

Note that this change uses `check_output` instead of `check_call` to
prevent the command output from being printed to stdout.

This change also introduces two additional enhancements:
- Skip the hadoop test if the hadoop binary is not available.
- Skip the ssh test if the ssh binary is not available.

[1] https://docs.python.org/3/library/subprocess.html#replacing-os-system
---
 tests/test_data_cloud.py | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py
index ee73a499c..a675e2a3a 100644
--- a/tests/test_data_cloud.py
+++ b/tests/test_data_cloud.py
@@ -1,6 +1,7 @@
+from subprocess import CalledProcessError
+from subprocess import check_output
 import os
 import time
-import stat
 import uuid
 import shutil
 import getpass
@@ -46,8 +47,11 @@ def _should_test_gcp():
         if os.path.exists(creds):
             os.unlink(creds)
         shutil.copyfile(TestDvc.GCP_CREDS_FILE, creds)
-        ret = os.system('gcloud auth activate-service-account --key-file={}'.format(creds))
-        assert ret == 0
+        try:
+            check_output(['gcloud', 'auth', 'activate-service-account',
+                          '--key-file', creds])
+        except CalledProcessError:
+            return False
         return True
 
     return False
@@ -67,17 +71,28 @@ def _should_test_ssh():
     if os.getenv("DVC_TEST_SSH") == "true":
         return True
 
-    #FIXME: enable on windows
+    # FIXME: enable on windows
     if os.name == 'nt':
         return False
 
-    assert os.system('ssh 127.0.0.1 ls &> /dev/null') == 0
+    try:
+        check_output(['ssh', '127.0.0.1', 'ls'])
+    except (CalledProcessError, FileNotFoundError):
+        return False
 
     return True
 
 
 def _should_test_hdfs():
-    return platform.system() == 'Linux'
+    if platform.system() != 'Linux':
+        return False
+
+    try:
+        check_output(['hadoop', 'version'])
+    except (CalledProcessError, FileNotFoundError):
+        return False
+
+    return True
 
 
 def get_local_storagepath():