From 7f2d9ed39b03bb38bbe46c0a1724f9ca2a66c872 Mon Sep 17 00:00:00 2001
From: Bongjin Koo <b.misc@icloud.com>
Date: Mon, 16 May 2022 15:21:17 -0700
Subject: [PATCH] Issue #74: Testing test_datasets.py. Using subprocess instead
 of os.system().

---
 ioSPI/datasets.py      | 21 ++++++++++++---------
 tests/test_datasets.py | 16 +++++++++++-----
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/ioSPI/datasets.py b/ioSPI/datasets.py
index 92ee021..54ec0de 100644
--- a/ioSPI/datasets.py
+++ b/ioSPI/datasets.py
@@ -1,6 +1,7 @@
 """Module to house methods related to datasets (micrographs, meta-data, etc.)."""
 
 import os
+import subprocess
 
 
 class OSFProject:
@@ -56,7 +57,7 @@ def ls(self):
         """List all files in the project."""
         print(f"Listing files from OSF project: {self.project_id}...")
         # os.system("osf ls")
-        os.system(f"/usr/share/miniconda/envs/ioSPI/bin/osf ls")
+        subprocess.run(["osf", "ls"], text=True, capture_output=True)
 
     def download(self, remote_path: str, local_path: str):
         """Download a file from an OSF project and save it locally.
@@ -81,9 +82,10 @@ def download(self, remote_path: str, local_path: str):
         full_remote_path = self.storage + "/" + remote_path
         print(f"Downloading {full_remote_path} to {local_path}...")
         # os.system(f"osf fetch {full_remote_path} {local_path}")
-        os.system(
-            f"/usr/share/miniconda/envs/ioSPI/bin/osf fetch "
-            f"{full_remote_path} {local_path}"
+        subprocess.run(
+            ["osf", "fetch", f"{full_remote_path}, f{local_path}"],
+            text=True,
+            capture_output=True,
         )
         print("Done!")
 
@@ -114,9 +116,10 @@ def upload(self, local_path: str, remote_path: str):
         full_remote_path = self.storage + "/" + remote_path
         print(f"Uploading {local_path} to {full_remote_path}...")
         # os.system(f"osf upload {local_path} {full_remote_path}")
-        os.system(
-            f"/usr/share/miniconda/envs/ioSPI/bin/osf upload "
-            f"{local_path} {full_remote_path}"
+        subprocess.run(
+            ["osf", "upload", f"{local_path}", f"{full_remote_path}"],
+            text=True,
+            capture_output=True,
         )
         print("Done!")
 
@@ -138,7 +141,7 @@ def remove(self, remote_path: str):
         full_remote_path = self.storage + "/" + remote_path
         print(f"Removing {full_remote_path} in the project...")
         # os.system(f"osf remove {full_remote_path}")
-        os.system(
-            f"/usr/share/miniconda/envs/ioSPI/bin/osf remove " f"{full_remote_path}"
+        subprocess.run(
+            ["osf", "remove", f"{full_remote_path}"], text=True, capture_output=True
         )
         print("Done!")
diff --git a/tests/test_datasets.py b/tests/test_datasets.py
index b247cbf..5051a0b 100644
--- a/tests/test_datasets.py
+++ b/tests/test_datasets.py
@@ -1,6 +1,7 @@
 """Unit tests for listing/uploading/downloading datasets on an OSF project."""
-
+import io
 import os
+import subprocess
 
 import pytest
 
@@ -57,9 +58,11 @@ def test_upload_valid(setup, set_file_path):
     setup.upload(set_file_path[0] + set_file_path[1], set_file_path[1])
     file_exists = False
     # file_list = os.popen("osf ls")
-    file_list = os.popen("/usr/share/miniconda/envs/ioSPI/bin/osf ls")
+    file_list = subprocess.run(["osf", "ls"], text=True, capture_output=True).stdout
+    file_list = io.StringIO(file_list)
     line = file_list.readline()
     while line:
+        print(line)
         file_exists = set_file_path[1] == line.split("/")[1].strip()
         if file_exists:
             break
@@ -82,9 +85,11 @@ def test_upload_invalid_because_no_remote_path(setup):
 
 def test_download_valid(setup, set_file_path):
     """Test the download method."""
-    os.system(f"rm {set_file_path[0] + set_file_path[1]}")
-    setup.download(set_file_path[1], set_file_path[0] + set_file_path[1])
+    setup.download(
+        set_file_path[1], set_file_path[0] + "downloaded_" + set_file_path[1]
+    )
     assert os.path.exists(set_file_path[0] + set_file_path[1])
+    os.system(f"rm {set_file_path[0]}downloaded_{set_file_path[1]}")
 
 
 def test_download_invalid_because_no_remote_path(setup):
@@ -104,7 +109,8 @@ def test_remove_valid(setup, set_file_path):
     setup.remove(set_file_path[1])
     file_exists = False
     # file_list = os.popen("osf ls")
-    file_list = os.popen("/usr/share/miniconda/envs/ioSPI/bin/osf ls")
+    file_list = subprocess.run(["osf", "ls"], text=True, capture_output=True).stdout
+    file_list = io.StringIO(file_list)
     line = file_list.readline()
     while line:
         file_exists = set_file_path[1] == line.split("/")[1].strip()