fixed tests

Signed-off-by: Dhiren-Mhatre <kp064669@gmail.com>

Author: Dhiren-Mhatre

gen3/file.py

@@ -177,65 +177,64 @@ def _ensure_dirpath_exists(path: Path) -> Path:
 
         return out_path
 
-    def download_single(
-        self,
-        guid,
-        download_path=".",
-        filename_format="original",
-        protocol=None,
-        skip_completed=False,
-        rename=False,
-    ):
-        """Download a single file with enhanced options.
+    def download_single(self, object_id, path):
+        """
+        Download a single file using its GUID.
 
         Args:
-            guid (str): File GUID to download
-            download_path (str): Directory to save file
-            filename_format (str): Format for filename - 'original', 'guid', or 'combined'
-            protocol (str, optional): Protocol preference for download
-            skip_completed (bool): Skip if file already exists
-            rename (bool): Rename file if conflict exists
+            object_id (str): The file's unique ID
+            path (str): Path to store the downloaded file at
 
         Returns:
-            Dict: Download result with status and details
+            bool: True if download successful, False otherwise
         """
-        # Create a single-item manifest to reuse async logic
-        manifest_data = [{"guid": guid}]
-
-        # Use the async download logic with single process
-        loop = asyncio.new_event_loop()
-        asyncio.set_event_loop(loop)
-
         try:
-            result = loop.run_until_complete(
-                self.async_download_multiple(
-                    manifest_data=manifest_data,
-                    download_path=download_path,
-                    filename_format=filename_format,
-                    protocol=protocol,
-                    max_concurrent_requests=1,
-                    num_processes=1,
-                    queue_size=1,
-                    skip_completed=skip_completed,
-                    rename=rename,
-                    no_progress=True,
-                )
-            )
+            url = self.get_presigned_url(object_id)
+        except Exception as e:
+            logging.critical(f"Unable to get a presigned URL for download: {e}")
+            return False
 
-            # Extract the single result
-            if result["succeeded"]:
-                return {"status": "downloaded", "filepath": result["succeeded"][0]}
-            elif result["skipped"]:
-                return {"status": "skipped", "filepath": result["skipped"][0]}
-            elif result["failed"]:
-                return {"status": "failed", "error": result["failed"][0]}
+        response = requests.get(url["url"], stream=True)
+        if response.status_code != 200:
+            logging.error(f"Response code: {response.status_code}")
+            if response.status_code >= 500:
+                for _ in range(MAX_RETRIES):
+                    logging.info("Retrying now...")
+                    # NOTE could be updated with exponential backoff
+                    time.sleep(1)
+                    response = requests.get(url["url"], stream=True)
+                    if response.status_code == 200:
+                        break
+                if response.status_code != 200:
+                    logging.critical("Response status not 200, try again later")
+                    return False
             else:
-                return {"status": "failed", "error": "Unknown error"}
+                return False
 
-        except Exception as e:
-            return {"status": "failed", "error": f"Download failed: {e}"}
-        finally:
-            loop.close()
+        response.raise_for_status()
+
+        total_size_in_bytes = int(response.headers.get("content-length"))
+        total_downloaded = 0
+
+        index = Gen3Index(self._auth_provider)
+        record = index.get_record(object_id)
+
+        filename = record["file_name"]
+
+        out_path = Gen3File._ensure_dirpath_exists(Path(path))
+
+        with open(os.path.join(out_path, filename), "wb") as f:
+            for data in response.iter_content(4096):
+                total_downloaded += len(data)
+                f.write(data)
+
+        if total_size_in_bytes == total_downloaded:
+            logging.info(f"File {filename} downloaded successfully")
+        else:
+            logging.error(f"File {filename} not downloaded successfully")
+            return False
+
+        return True
 
     def upload_file_to_guid(
         self, guid, file_name, protocol=None, expires_in=None, bucket=None

tests/download_tests/test_async_download.py

@@ -1,4 +1,4 @@
-from unittest.mock import patch
+from unittest.mock import patch, MagicMock, AsyncMock
 import json
 import pytest
 from pathlib import Path
@@ -99,7 +99,6 @@ def test_load_manifest(self, mock_gen3_auth):
         data = json.load(f)
         assert len(data) == len(manifest_list)
 
-    @pytest.mark.skip(reason="download_single uses multiprocessing which is incompatible with mocking in tests")
     @patch("gen3.file.requests")
     @patch("gen3.index.Gen3Index.get_record")
     @pytest.mark.parametrize("download_dir_overwrite", [None, "sub/path"])
@@ -165,7 +164,6 @@ def test_download_single(
             if download_dir_overwrite and os.path.exists(download_path):
                 shutil.rmtree(download_path)
 
-    @pytest.mark.skip(reason="download_single uses multiprocessing which is incompatible with mocking in tests")
     @patch("gen3.file.requests")
     def test_download_single_no_auth(self, mock_get, download_dir, mock_gen3_auth):
 
@@ -196,7 +194,6 @@ def test_download_single_no_auth(self, mock_get, download_dir, mock_gen3_auth):
 
         assert result == False
 
-    @pytest.mark.skip(reason="download_single uses multiprocessing which is incompatible with mocking in tests")
     @patch("gen3.file.requests")
     def test_download_single_wrong_auth(self, mock_get, download_dir, mock_gen3_auth):
 
@@ -227,7 +224,6 @@ def test_download_single_wrong_auth(self, mock_get, download_dir, mock_gen3_auth
 
         assert result == False
 
-    @pytest.mark.skip(reason="download_single uses multiprocessing which is incompatible with mocking in tests")
     @patch("gen3.file.requests")
     def test_download_single_bad_id(self, mock_get, download_dir, mock_gen3_auth):
 
@@ -268,3 +264,30 @@ def test_load_manifest_bad_format(self):
 
         manifest_list = _load_manifest(Path(DIR, "resources/bad_format.json"))
         assert manifest_list == None
+
+    @pytest.mark.asyncio
+    async def test_async_download_multiple_empty_manifest(self, mock_gen3_auth):
+        """
+        Test async_download_multiple with an empty manifest.
+        Verifies it returns empty results without errors.
+        """
+        file_tool = Gen3File(mock_gen3_auth)
+        result = await file_tool.async_download_multiple(manifest_data=[])
+
+        assert result == {"succeeded": [], "failed": [], "skipped": []}
+
+    @pytest.mark.asyncio
+    async def test_async_download_multiple_invalid_guids(self, mock_gen3_auth):
+        """
+        Test async_download_multiple with invalid GUIDs.
+        Verifies it returns empty results for missing GUIDs.
+        """
+        file_tool = Gen3File(mock_gen3_auth)
+
+        # Manifest with missing guid/object_id fields
+        manifest_data = [{"file_name": "test.txt"}, {}]
+
+        result = await file_tool.async_download_multiple(manifest_data=manifest_data)
+
+        assert result == {"succeeded": [], "failed": [], "skipped": []}
+